Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Special kind of Ember object (a proxy) error thrown in ember 3.1 #16521

Closed
toranb opened this issue Apr 15, 2018 · 3 comments
Closed

Special kind of Ember object (a proxy) error thrown in ember 3.1 #16521

toranb opened this issue Apr 15, 2018 · 3 comments

Comments

@toranb
Copy link
Contributor

toranb commented Apr 15, 2018

I recently upgraded an ember addon from ember 3.0 => 3.1 and found a model class that is created via Proxy like so ... (note: RecordProxy is using ObjectProxy from '@ember/object/proxy')

return RecordProxy.create({
  _store: this,
  _type: type,
  _filter_value: actualId,
  _source: this._findAll(type),
  compute() {
    let filter_value = this.get("_filter_value");
    return this.get("_source").findBy(primaryKey, filter_value);
  }
});

And constructed it using a model object with a non computed property (ie: vanilla function)

import { Model } from 'ember-cli-simple-store/model';

export default Model.extend({
  change_role() {
  }
});

Is now throwing an error like so ...

You attempted to access the `change_role` property (of <(unknown mixin):ember244>). Since Ember 3.1, this is usually fine as you no longer need to use `.get()` to access computed properties. However, in this case, the object in question is a special kind of Ember object (a proxy). Therefore, it is still necessary to use `.get('change_role')` in this case.

I'm curious if this is a regression or future deprecation just waiting to happen ;)

If I understand the error ... as of ember 3.1 I would need to access this change_role function with a Ember.get ... but prior I could invoke it like a normal function (without get) but because it's a function (and not a computed/ or ES5 GETTER) I wouldn't anticipate having to use an explicit Ember.get to invoke it.

I had trouble creating this w/ ember twiddle but I was able to ember new using 3.1 to prove it's broken. Note: this app works fine if you were to flip the source dependency back to ember 3.0

toranb/simple-store-busted-ember-three-one@427e805

Any helpful pointers on how to move forward here? Thank you in advance!

@rwjblue
Copy link
Member

rwjblue commented Apr 18, 2018

The issue in this case with ember-cli-simple-store (which doesn't seem terribly simple to me 😉 ) is here. In Ember 3.1 doing that check will trigger the reported assertion because the unknownProperty of the proxy assertion will find change_role in the underlying model.

The quick fix workaround is to implement your own unknownProperty that always returns undefined during this "copy forward the methods on my underlying model" setup. Something like:

init() {
     // ...snip...
      this.isCopyingMethods = true;
      for(let method in model) {
            if(typeof model[method] === "function") {
                if(!this[method]) {
                    this.proxyMethod(method);
                }
            }
        }
        this.isCopyingMethods = false;
},

unknownProperty() {
  if (this.isCopyingMethods) { return; }

  return this._super(...arguments);
}

To fix this issue generically means either:

  1. Make the proxy assertion ignore when the type of the result is 'function'. This feels somewhat like "whack-a-mole" but may actually be a nice heuristic for the sorts of "check if this method exists before calling it" types of things (see list of exclusions which are nearly all functions).
  2. Change the assertion into a warning...

@toranb
Copy link
Contributor Author

toranb commented Apr 21, 2018

@rwjblue closing this out in favor of #16560

Thanks again for all the help Robert !!!

@toranb toranb closed this as completed Apr 21, 2018
@rwjblue
Copy link
Member

rwjblue commented Apr 21, 2018

No problem! Happy to help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants