From 4cd5b808a2e62b324a2ca7f25c360ed351e60f83 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 24 Apr 2017 15:19:43 -0400 Subject: [PATCH] [BUGFIX beta] Avoid storing container on the prototype. 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. --- packages/container/lib/container.js | 4 ++-- packages/container/tests/container_test.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/container/lib/container.js b/packages/container/lib/container.js index 4d4a882adfc..62c05610c44 100644 --- a/packages/container/lib/container.js +++ b/packages/container/lib/container.js @@ -5,6 +5,7 @@ import { dictionary, symbol, setOwner, + getOwner, OWNER, assign, NAME_KEY, @@ -521,7 +522,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) { @@ -537,7 +538,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 destroyDestroyables(container) { diff --git a/packages/container/tests/container_test.js b/packages/container/tests/container_test.js index 4033b5eab2e..76efa3df20f 100644 --- a/packages/container/tests/container_test.js +++ b/packages/container/tests/container_test.js @@ -554,8 +554,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'); @@ -741,4 +742,3 @@ QUnit.test('#factoryFor options passed to create clobber injections', (assert) = assert.equal(instrance.ajax, 'fetch'); }); -