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

MODEL_FACTORY_INJECTIONS + no double extend + polymorphic may have issues #4966

Closed
stefanpenner opened this issue May 3, 2017 · 3 comments
Closed
Assignees
Labels
🏷️ bug This PR primarily fixes a reported issue

Comments

@stefanpenner
Copy link
Member

stefanpenner commented May 3, 2017

With double extend off in new ember Ember.ENV.MODEL_FACTORY_INJECTIONS isn't accurate, and most likely is causing issues.

Most likely:

if (Ember.MODEL_FACTORY_INJECTIONS) {
modelClass = modelClass.superclass;
}

And likely others:
https://github.com/emberjs/data/search?utf8=%E2%9C%93&q=MODEL_FACTORY_INJECTIONS&type=

We should review and audit each occurrence, and address issues that arise.

Plan

  • add tests for assertPolymorphicType
    if (Ember.MODEL_FACTORY_INJECTIONS) {
    modelClass = modelClass.superclass;
    }
  • add tests with both double extend and no double extend
  • fix funky things discovered
@stefanpenner stefanpenner self-assigned this May 3, 2017
@stefanpenner
Copy link
Member Author

I believe ember (when double extend is disabled) needs to force MODEL_FACTORY_INJECTIONS to always be false: emberjs/ember.js#15204

@runspired
Copy link
Contributor

There is an open PR for this I should fix at some point soon: (#4886)

@runspired
Copy link
Contributor

runspired commented May 22, 2021

We were never able to fix this because there isn't a good way to deal with creating an extra extend just-in-time. That said the need for doing so has greatly diminished with time, and the lack of reports indicates that we've mostly eliminated the footguns. Internally I don't believe we use Model.modelName anywhere at this point. Even adapters and serializers are moving away from it as we now provide them a schema wrapper.

For polymorphism, we still do have a check that walks the prototype chain, but it uses instanceof now. (and we'll be deleting the check soon because we've shown it to be unnecessary).

Going to close this as I don't think there's any further action for us to take directly on this issue.

@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue and removed Bug labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

No branches or pull requests

2 participants