Skip to content

Commit

Permalink
[FEATURE ember-container-inject-owner] Inject owner instead of `con…
Browse files Browse the repository at this point in the history
…tainer` during `lookup`.

This change represents the final brick needed to wall off the
`Container` as fully private.

The Container no longer injects itself into every object that it looks
up. Instead, it uses the new `setOwner` helper to inject the "owner",
which can then be retrieved from any object using the new `getOwner`
helper.

The current net effect is that an app instance is injected into every
looked up object instead of that app instance's container. This
provides clean, public access to methods exposed by the app
instance's ContainerProxy and RegistryProxy methods. It also guarantees
that the only supported path to get to a Container or Registry is
through a proxied method. This guarantee is important because it allows
for owner-specific logic to be placed in proxy methods.

In the future, other classes, such as Engine (coming soon), may mix in
ContainerProxy and thus have the potential to be "owners".

This work is behind the `ember-container-inject-owner` flag. Without
this flag enabled, the Container will continue to inject itself directly
into objects that it instantiates (as `container`).
  • Loading branch information
dgeb committed Nov 4, 2015
1 parent 7d7450c commit 9845a9a
Show file tree
Hide file tree
Showing 93 changed files with 1,799 additions and 1,675 deletions.
3 changes: 2 additions & 1 deletion features.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"ember-debug-handlers": true,
"ember-routing-routable-components": null,
"ember-metal-ember-assign": null,
"ember-contextual-components": true
"ember-contextual-components": true,
"ember-container-inject-owner": null
}
}
50 changes: 47 additions & 3 deletions packages/container/lib/container.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import Ember from 'ember-metal/core';
import { assert } from 'ember-metal/debug';
import dictionary from 'ember-metal/dictionary';
import isEnabled from 'ember-metal/features';
import { setOwner } from './owner';

/**
A container used to instantiate and cache objects.
Expand All @@ -17,12 +19,20 @@ import dictionary from 'ember-metal/dictionary';
*/
function Container(registry, options) {
this.registry = registry;
this.owner = options && options.owner ? options.owner : null;
this.cache = dictionary(options && options.cache ? options.cache : null);
this.factoryCache = dictionary(options && options.factoryCache ? options.factoryCache : null);
this.validationCache = dictionary(options && options.validationCache ? options.validationCache : null);
}

Container.prototype = {
/**
@private
@property owner
@type Object
*/
owner: null,

/**
@private
@property registry
Expand Down Expand Up @@ -232,6 +242,11 @@ function factoryFor(container, fullName) {
factoryInjections._toString = registry.makeToString(factory, fullName);

var injectedFactory = factory.extend(injections);

if (isEnabled('ember-container-inject-owner')) {
injectDeprecatedContainer(injectedFactory.prototype, container);
}

injectedFactory.reopenClass(factoryInjections);

if (factory && typeof factory._onLookup === 'function') {
Expand All @@ -255,7 +270,14 @@ function injectionsFor(container, fullName) {
registry.getTypeInjections(type),
registry.getInjections(fullName));
injections._debugContainerKey = fullName;
injections.container = container;

setOwner(injections, container.owner);

// TODO - Inject a `FakeContainer` instead here. The `FakeContainer` will
// proxy all properties of the container with deprecations.
if (!isEnabled('ember-container-inject-owner')) {
injections.container = container;
}

return injections;
}
Expand Down Expand Up @@ -299,18 +321,40 @@ function instantiate(container, fullName) {

validationCache[fullName] = true;

let obj;

if (typeof factory.extend === 'function') {
// assume the factory was extendable and is already injected
return factory.create();
obj = factory.create();
} else {
// assume the factory was extendable
// to create time injections
// TODO: support new'ing for instantiation and merge injections for pure JS Functions
return factory.create(injectionsFor(container, fullName));
obj = factory.create(injectionsFor(container, fullName));

if (isEnabled('ember-container-inject-owner')) {
injectDeprecatedContainer(obj, container);
}
}

return obj;
}
}

// TODO - remove when Ember reaches v3.0.0
function injectDeprecatedContainer(object, container) {
Object.defineProperty(object, 'container', {
configurable: true,
enumerable: false,
get() {
Ember.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' });
return container;
}
});
}

function eachDestroyable(container, callback) {
var cache = container.cache;
var keys = Object.keys(cache);
Expand Down
51 changes: 31 additions & 20 deletions packages/container/tests/container_test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Ember from 'ember-metal/core';
import Registry from 'container/registry';
import { factory } from 'container/tests/container_helper';
import factory from 'container/tests/test-helpers/factory';
import isEnabled from 'ember-metal/features';

var originalModelInjections;

Expand Down Expand Up @@ -57,8 +58,6 @@ QUnit.test('A factory returned from lookupFactory has a debugkey', function() {

registry.register('controller:post', PostController);
var PostFactory = container.lookupFactory('controller:post');

ok(!PostFactory.container, 'factory instance receives a container');
equal(PostFactory._debugContainerKey, 'controller:post', 'factory instance receives _debugContainerKey');
});

Expand All @@ -76,8 +75,6 @@ QUnit.test('fallback for to create time injections if factory has no extend', fu

var postController = container.lookup('controller:post');

ok(postController.container, 'instance receives a container');
equal(postController.container, container, 'instance receives the correct container');
equal(postController._debugContainerKey, 'controller:post', 'instance receives _debugContainerKey');
ok(postController.apple instanceof AppleController, 'instance receives an apple of instance AppleController');
});
Expand All @@ -91,7 +88,6 @@ QUnit.test('The descendants of a factory returned from lookupFactory have a cont
registry.register('controller:post', PostController);
instance = container.lookupFactory('controller:post').create();

ok(instance.container, 'factory instance receives a container');
equal(instance._debugContainerKey, 'controller:post', 'factory instance receives _debugContainerKey');

ok(instance instanceof PostController, 'factory instance is instance of factory');
Expand Down Expand Up @@ -120,18 +116,6 @@ QUnit.test('A registered factory returns a fresh instance if singleton: false is
ok(postController4 instanceof PostController, 'All instances are instances of the registered factory');
});

QUnit.test('A container lookup has access to the container', function() {
var registry = new Registry();
var container = registry.container();
var PostController = factory();

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

var postController = container.lookup('controller:post');

equal(postController.container, container);
});

QUnit.test('A factory type with a registered injection\'s instances receive that injection', function() {
var registry = new Registry();
var container = registry.container();
Expand Down Expand Up @@ -163,10 +147,8 @@ QUnit.test('An individual factory with a registered injection receives the injec
var postController = container.lookup('controller:post');
var store = container.lookup('store:main');

equal(store.container, container);
equal(store._debugContainerKey, 'store:main');

equal(postController.container, container);
equal(postController._debugContainerKey, 'controller:post');
equal(postController.store, store, 'has the correct store injected');
});
Expand Down Expand Up @@ -535,3 +517,32 @@ QUnit.test('Lazy injection validations are cached', function() {
container.lookup('apple:main');
container.lookup('apple:main');
});

if (isEnabled('ember-container-inject-owner')) {
QUnit.test('A deprecated `container` property is appended to every instantiated object', function() {
let registry = new Registry();
let container = registry.container();
let PostController = factory();
registry.register('controller:post', PostController);
let postController = container.lookup('controller:post');

expectDeprecation(function() {
Ember.get(postController, 'container');
}, 'Using the injected `container` is deprecated. Please use the `getOwner` helper instead to access the owner of this object.');

expectDeprecation(function() {
let c = postController.container;
strictEqual(c, container);
}, '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() {
const registry = new Registry();
const container = registry.container();
const PostController = factory();
registry.register('controller:post', PostController);
const postController = container.lookup('controller:post');

strictEqual(postController.container, container, '');
});
}
2 changes: 1 addition & 1 deletion packages/container/tests/registry_test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Ember from 'ember-metal/core';
import { Registry } from 'container';
import { factory } from 'container/tests/container_helper';
import factory from 'container/tests/test-helpers/factory';

var originalModelInjections;

Expand Down
15 changes: 15 additions & 0 deletions packages/container/tests/test-helpers/build-owner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import EmberObject from 'ember-runtime/system/object';
import Registry from 'container/registry';
import RegistryProxy from 'ember-runtime/mixins/registry_proxy';
import ContainerProxy from 'ember-runtime/mixins/container_proxy';

export default function buildOwner(props) {
let Owner = EmberObject.extend(RegistryProxy, ContainerProxy, {
init() {
this._super(...arguments);
const registry = this.__registry__ = new Registry();
this.__container__ = registry.container({ owner: this });
}
});
return Owner.create(props);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ var setProperties = function(object, properties) {

var guids = 0;

var factory = function() {
export default function factory() {
/*jshint validthis: true */

var Klass = function(options) {
Expand Down Expand Up @@ -61,9 +61,4 @@ var factory = function() {

return Child;
}
};

export {
factory,
setProperties
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ let ApplicationInstance = EmberObject.extend(RegistryProxy, ContainerProxy, {
registry.makeToString = applicationRegistry.makeToString;

// Create a per-instance container from the instance's registry
this.__container__ = registry.container();
this.__container__ = registry.container({ owner: this });

// Register this instance in the per-instance registry.
//
Expand Down
11 changes: 0 additions & 11 deletions packages/ember-extension-support/lib/container_debug_adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,6 @@ import EmberObject from 'ember-runtime/system/object';
@public
*/
export default EmberObject.extend({
/**
The container of the application being debugged.
This property will be injected
on creation.
@property container
@default null
@public
*/
container: null,

/**
The resolver instance of the application
being debugged. This property will be injected
Expand Down
16 changes: 2 additions & 14 deletions packages/ember-extension-support/lib/data_adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import Namespace from 'ember-runtime/system/namespace';
import EmberObject from 'ember-runtime/system/object';
import { A as emberA } from 'ember-runtime/system/native_array';
import Application from 'ember-application/system/application';
import { getOwner } from 'container/owner';

/**
@module ember
Expand Down Expand Up @@ -59,19 +60,6 @@ export default EmberObject.extend({
this.releaseMethods = emberA();
},

/**
The container of the application being debugged.
This property will be injected
on creation.
@property container
@default null
@since 1.3.0
@public
*/
container: null,


/**
The container-debug-adapter which is used
to list all models.
Expand Down Expand Up @@ -171,7 +159,7 @@ export default EmberObject.extend({

_nameToClass(type) {
if (typeof type === 'string') {
type = this.container.lookupFactory(`model:${type}`);
type = getOwner(this)._lookupFactory(`model:${type}`);
}
return type;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import EmberController from 'ember-runtime/controllers/controller';
import 'ember-extension-support'; // Must be required to export Ember.ContainerDebugAdapter
import Application from 'ember-application/system/application';

var adapter, App;
var adapter, App, appInstance;


function boot() {
Expand All @@ -19,14 +19,16 @@ QUnit.module('Container Debug Adapter', {
});
boot();
run(function() {
adapter = App.__container__.lookup('container-debug-adapter:main');
appInstance = App.__deprecatedInstance__;
adapter = appInstance.lookup('container-debug-adapter:main');
});
},
teardown() {
run(function() {
adapter.destroy();
appInstance.destroy();
App.destroy();
App = null;
App = appInstance = adapter = null;
});
}
});
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-htmlbars/lib/hooks/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export default function componentHook(renderNode, env, scope, _tagName, params,

let component, layout;
if (isDasherized || !isAngleBracket) {
let result = lookupComponent(env.container, tagName);
let result = lookupComponent(env.owner, tagName);
component = result.component;
layout = result.layout;

Expand Down
8 changes: 4 additions & 4 deletions packages/ember-htmlbars/lib/hooks/has-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ export default function hasHelperHook(env, scope, helperName) {
return true;
}

var container = env.container;
if (validateLazyHelperName(helperName, container, env.hooks.keywords)) {
var containerName = 'helper:' + helperName;
if (container.registry.has(containerName)) {
const owner = env.owner;
if (validateLazyHelperName(helperName, owner, env.hooks.keywords)) {
var registrationName = 'helper:' + helperName;
if (owner.hasRegistration(registrationName)) {
return true;
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-htmlbars/lib/keywords/closure-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function createClosureComponentCell(env, originalComponentPath, params, hash, la
}

function isValidComponentPath(env, path) {
const result = lookupComponent(env.container, path);
const result = lookupComponent(env.owner, path);

return !!(result.component || result.layout);
}
Expand All @@ -88,7 +88,7 @@ function createNestedClosureComponentCell(componentCell, params, hash) {
}

function createNewClosureComponentCell(env, componentPath, params, hash) {
let positionalParams = getPositionalParams(env.container, componentPath);
let positionalParams = getPositionalParams(env.owner, componentPath);

// This needs to be done in each nesting level to avoid raising assertions
processPositionalParams(null, positionalParams, params, hash);
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-htmlbars/lib/keywords/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export default {

return assign({}, state, {
parentView: env.view,
viewClassOrInstance: getView(read(params[0]), env.container)
viewClassOrInstance: getView(read(params[0]), env.owner)
});
},

Expand Down
Loading

0 comments on commit 9845a9a

Please sign in to comment.