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.

Sign up for GitHub

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

[BUGFIX beta] only defProp the deprecated container once per prototype #15008

Merged
merged 1 commit into from
Mar 14, 2017
Merged

[BUGFIX beta] only defProp the deprecated container once per prototype #15008

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/container/lib/container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ function instantiate(factory, props, container, fullName) {
obj = factory.create(assign({}, injections, props));

// TODO - remove when Ember reaches v3.0.0
if (!Object.isFrozen(obj) && 'container' in obj) {
if (!Object.isFrozen(obj)) {
injectDeprecatedContainer(obj, container);
}
}
Expand All @@ -559,6 +559,7 @@ function factoryInjectionsFor(container, fullName) {

// TODO - remove when Ember reaches v3.0.0
function injectDeprecatedContainer(object, container) {
if ('container' in object) { return; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite right, but it may not matter at this point. In the case above (in instantiate) where we are creating an instance on a factory that does not have .extend we have to ensure that the .container property on the instance (which may be the container._fakeContainerToInject is replaced by a getter that issues a deprecation.

Now, considering that this has been deprecated for quite some time (since ember 2.3), it's might be fine to drop support for the deprecation in this odd edge case.

I'll work up a failing test case so you can see what I mean.

Object.defineProperty(object, 'container', {
configurable: true,
enumerable: false,
Expand Down Expand Up @@ -695,8 +696,9 @@ class FactoryManager {
` an invalid module export.`);
}

if (this.class.prototype) {
injectDeprecatedContainer(this.class.prototype, this.container);
let prototype = this.class.prototype;
if (prototype) {
injectDeprecatedContainer(prototype, this.container);
}

return this.class.create(props);
Expand Down