Skip to content

Commit

Permalink
[BUGFIX beta] Avoid storing container on the prototype.
Browse files Browse the repository at this point in the history
In order to support `this.container` (deprecated since 2.3) being available
to objects for backwards compatibility, we define the `container` property
on all classes that we instantiate via normal DI APIs.

The bulk of the work to support this was done in prior refactorings. This
change is targetted at ensuring that we do not leak the container instance
onto the object's prototype in the `CONTAINER_OVERRIDE` slot.

After these changes, we still support all of the APIs that we supported
previously, but will fallback to the current objects owner `__container__`
property (instead of storing it on the prototype directly).

Also, the test being updated here was not providing the `owner` to the
container in the unit test. This test being changed was previously only
doing part of the required setup for creating a `registry` / `container` /
`owner` group.

(cherry picked from commit 4cd5b80)
  • Loading branch information
rwjblue committed Apr 27, 2017
1 parent 3a1628c commit fcfa84d
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
4 changes: 2 additions & 2 deletions packages/container/lib/container.js
Expand Up @@ -3,6 +3,7 @@ import {
dictionary,
symbol,
setOwner,
getOwner,
OWNER,
assign,
NAME_KEY,
Expand Down Expand Up @@ -569,7 +570,7 @@ const INJECTED_DEPRECATED_CONTAINER_DESC = {
enumerable: false,
get() {
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 this[CONTAINER_OVERRIDE];
return this[CONTAINER_OVERRIDE] || getOwner(this).__container__;
},

set(value) {
Expand All @@ -585,7 +586,6 @@ const INJECTED_DEPRECATED_CONTAINER_DESC = {
function injectDeprecatedContainer(object, container) {
if ('container' in object) { return; }
Object.defineProperty(object, 'container', INJECTED_DEPRECATED_CONTAINER_DESC);
object[CONTAINER_OVERRIDE] = container;
}

function eachDestroyable(container, callback) {
Expand Down
3 changes: 2 additions & 1 deletion packages/container/tests/container_test.js
Expand Up @@ -553,8 +553,9 @@ QUnit.test('An object with its owner pre-set should be returned from ownerInject
});

QUnit.test('A deprecated `container` property is appended to every object instantiated from an extendable factory', function() {
let owner = { };
let registry = new Registry();
let container = registry.container();
let container = owner.__container__ = registry.container({ owner });
let PostController = factory();
registry.register('controller:post', PostController);
let postController = container.lookup('controller:post');
Expand Down

0 comments on commit fcfa84d

Please sign in to comment.