Skip to content

Commit

Permalink
[BUGFIX beta] Ensure container can still be provided to .create.
Browse files Browse the repository at this point in the history
In Ember versions until 2.3.0 it was common to provide a `container`
property (if needed) when creating instances of a factory manually. This
might look like:

```
let Foo = container.lookupFactory('model:foo');
let instance = Foo.create({
  container
});
```

Many addons did this (examples include ember-cp-validations and ember-data).

Our initial implementation of the deprecated `container` property on extendable
factories used `Object.defineProperty` to ensure calling
`instance.container` returned the correct value. Unfortunately, we only
defined a getter which means calling `Foo.create({ container });`
triggers the following error:

```
Cannot set property container of [object Object] which has only a getter
```

This change adds a setter (with an appropriate deprecation) to prevent
triggering an error.
  • Loading branch information
rwjblue committed Dec 19, 2015
1 parent 039167c commit 258fc30
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
18 changes: 17 additions & 1 deletion packages/container/lib/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import dictionary from 'ember-metal/dictionary';
import isEnabled from 'ember-metal/features';
import { setOwner, OWNER } from './owner';
import { buildFakeContainerWithDeprecations } from 'ember-runtime/mixins/container_proxy';
import symbol from 'ember-metal/symbol';

const CONTAINER_OVERRIDE = symbol('CONTAINER_OVERRIDE');

/**
A container used to instantiate and cache objects.
Expand All @@ -27,6 +30,7 @@ function Container(registry, options) {

if (isEnabled('ember-container-inject-owner')) {
this._fakeContainerToInject = buildFakeContainerWithDeprecations(this);
this[CONTAINER_OVERRIDE] = undefined;
}
}

Expand Down Expand Up @@ -404,7 +408,19 @@ function injectDeprecatedContainer(object, container) {
deprecate('Using the injected `container` is deprecated. Please use the `getOwner` helper instead to access the owner of this object.',
false,
{ id: 'ember-application.injected-container', until: '3.0.0', url: 'http://emberjs.com/deprecations/v2.x#toc_injected-container-access' });
return container;
return this[CONTAINER_OVERRIDE] || container;
},

set(value) {
deprecate(
`Providing the \`container\` property to ${this} is deprecated. Please use \`Ember.setOwner\` or \`owner.ownerInjection()\` instead to provide an owner to the instance being created.`,
false,
{ id: 'ember-application.injected-container', until: '3.0.0', url: 'http://emberjs.com/deprecations/v2.x#toc_injected-container-access' }
);

this[CONTAINER_OVERRIDE] = value;

return value;
}
});
}
Expand Down
22 changes: 22 additions & 0 deletions packages/container/tests/container_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,28 @@ if (isEnabled('ember-container-inject-owner')) {

equal(otherController.container, 'foo', 'container was not added');
});

QUnit.test('An extendable factory can provide `container` upon create, with a deprecation', function(assert) {
let registry = new Registry();
let container = registry.container();

registry.register('controller:post', factory());

let PostController = container.lookupFactory('controller:post');

let postController;

expectDeprecation(function() {
postController = PostController.create({
container: 'foo'
});
}, /Providing the \`container\` property to .+ is deprecated. Please use \`Ember.setOwner\` or \`owner.ownerInjection\(\)\` instead to provide an owner to the instance being created/);

expectDeprecation(function() {
let c = postController.container;
assert.equal(c, 'foo', 'the `container` provided to `.create`was used');
}, 'Using the injected `container` is deprecated. Please use the `getOwner` helper instead to access the owner of this object.');
});
} else {
QUnit.test('A `container` property is appended to every instantiated object', function() {
let registry = new Registry();
Expand Down

0 comments on commit 258fc30

Please sign in to comment.