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

depending on "model.constructor.modelName" sets model.constructor to undefined #14014

Closed
spruce opened this issue Aug 3, 2016 · 13 comments
Closed

Comments

@spruce
Copy link

spruce commented Aug 3, 2016

In my app I use an computed property to state how many steps are left to do for creating the models. This depends on how many steps are already done and which kind of model it is currently creating. (We create different kind of coworkers)

In Ember 2.5 it did work. After upgrading or slightly after (not quite sure) it stopped working.

this is my cp

numberOfSubSteps: Ember.computed('model.constructor.modelName', function() {
    let valueBasedOnModel = 4;
    switch(this.get('model.constructor.modelName')) {
      case 'kurzfristig':
        valueBasedOnModel = 5;
        break;
      case 'geringfugig':
        valueBasedOnModel = 6;
        break;
...
    }
    return valueBasedOnModel;
  }),

I found the exact line where it changes the constructor. (It defines a new property which somehow is then returning undefined.)
https://github.com/emberjs/ember.js/blob/master/packages/ember-metal/lib/watch_key.js#L81

This is the callstack when it happens
screenshot 2016-08-03 um 17 05 44

might be somewhat related to #9387

@Serabe
Copy link
Member

Serabe commented Aug 4, 2016

Hello, @spruce.

Can you please reproduce the problem in an ember-twiddle, please?

I tried but couldn't.

Thank you!

@Serabe
Copy link
Member

Serabe commented Aug 4, 2016

I finally got two different reproductions. The first one by @cibernox can be found here and it raises the error as soon as the application renders.

The other example is a bit more convoluted but shows that a computed property can work if the model does not change. Instructions are found in the twiddle itself.

@cibernox
Copy link
Contributor

cibernox commented Aug 4, 2016

To clarify, the application fails as soon as the app renders because I'm passing the post model after it has been destroyed by observing his constructor to a component, and components perform an assertion to deperecate didInitAttrs that calls .toString() on that property.

The default toString attempts to call this.constructor.super, which now raises an error.

@cibernox
Copy link
Contributor

cibernox commented Aug 4, 2016

Another note. I understand that observing the constructor does seem like a weird pattern, given that the constructor can't change, so I'd accept as reasonable bugfix to add an assertion in development to instruct users to not do that.

@Sinled
Copy link

Sinled commented Aug 5, 2016

Also have problem with this.constructor undefined in didInitAttrs depreaction check

but in my case i only pass model.constructor.modelName as argument to component

@cibernox
Copy link
Contributor

cibernox commented Aug 5, 2016

@Sinled Using that in a template is implicitly creating an observer to watch for changes, so it's the same issue.

chrisseto added a commit to CenterForOpenScience/ember-osf that referenced this issue Aug 19, 2016
  Observers that rely on constructor.something set constructor to undefined.
  emberjs/ember.js#14014
  This only appeared when wrapping objects in a ProxyObject. ¯\_(ツ)_/¯
@ToMoCoop
Copy link

I've recreated this too in a current project that upgraded, and it was the same cause. I amended the computed property to use the modelName, but not actually listen for the property (as, as you said, it doesn't actually change) and this seems to resolve it for me.

@urbany
Copy link

urbany commented Nov 29, 2016

This just happened to me after upgrading to ember 2.10.0

replaced:

computed.oneWay('owner.constructor.modelName')

by:

computed('owner', function() {
  return this.get('owner.constructor.modelName');
})

and it's working now, thanks for the tips @cibernox

@ngouy
Copy link

ngouy commented Feb 13, 2017

how did u resolve it @Sinled ? I have the exact same issue

@stefanpenner
Copy link
Member

stefanpenner commented Feb 13, 2017

So modelName being written (or having been written in the past) to the constructor is polluting a global (the class, which may have more then one name...). This ends up being at odds with several performance efforts, such as: #14360

Maybe an alternative API can be provided?

@PowerP
Copy link

PowerP commented Feb 13, 2017

This works:

import DS from 'ember-data';
import Ember from 'ember';

export default {
  name: 'model-Name',
  initialize() {
    DS.Model.reopen({
      modelName: Ember.computed(function() {
        return this.get('constructor.modelName');
      })
    });
  }
};

Is this a proper workaround?

@pixelhandler
Copy link
Contributor

@PowerP I think that is a sufficient workaround.

@stefanpenner @cibernox is this still an issue, perhaps we should close; what do you think?

@pixelhandler
Copy link
Contributor

Per our triage policy I'll close this out for now, feel free to reopen if the issue persists on the current release.

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

No branches or pull requests

10 participants