Skip to content

Commit

Permalink
Make factoryFor return the double extend when flag is off
Browse files Browse the repository at this point in the history
  • Loading branch information
@chadhietala
chadhietala committed Oct 24, 2016
1 parent c295808 commit f6dfba0
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 52 deletions.
142 changes: 105 additions & 37 deletions packages/container/lib/container.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import {
symbol,
setOwner,
OWNER,
assign
assign,
NAME_KEY
} from 'ember-utils';
import { ENV } from 'ember-environment';
import { assert, deprecate, runInDebug, isFeatureEnabled } from 'ember-metal';
Expand Down Expand Up @@ -145,54 +146,66 @@ Container.prototype = {

factoryFor(fullName, options = {}) {
let container = this;
let normalizedName = this.registry.normalize(fullName);
assert('fullName must be a proper full name', this.registry.validateFullName(normalizedName));
let manager;
if (isFeatureEnabled('container-factoryFor')) {
let normalizedName = this.registry.normalize(fullName);
assert('fullName must be a proper full name', this.registry.validateFullName(normalizedName));

if (options.source) {
normalizedName = this.registry.expandLocalLookup(fullName, options);
// if expandLocalLookup returns falsey, we do not support local lookup
if (!normalizedName) { return; }
}
if (options.source) {
normalizedName = this.registry.expandLocalLookup(fullName, options);
// if expandLocalLookup returns falsey, we do not support local lookup
if (!normalizedName) { return; }
}

let factory = this.registry.resolve(normalizedName);
let factory = this.registry.resolve(normalizedName);

if (factory === undefined) { return; }
if (factory === undefined) { return; }

let manager = {
class: factory,
create(options = {}) {
let injections = injectionsFor(container, normalizedName);
let props = assign({}, injections, options);
manager = {
class: factory,
create(options = {}) {
let injections = injectionsFor(container, normalizedName);
let props = assign({}, injections, options);

runInDebug(() => {
let lazyInjections;
let validationCache = container.validationCache;
// Ensure that all lazy injections are valid at instantiation time
if (!validationCache[fullName] && factory && typeof factory._lazyInjections === 'function') {
lazyInjections = factory._lazyInjections();
lazyInjections = container.registry.normalizeInjectionsHash(lazyInjections);
runInDebug(() => {
let lazyInjections;
let validationCache = container.validationCache;
// Ensure that all lazy injections are valid at instantiation time
if (!validationCache[fullName] && factory && typeof factory._lazyInjections === 'function') {
lazyInjections = factory._lazyInjections();
lazyInjections = container.registry.normalizeInjectionsHash(lazyInjections);

container.registry.validateInjections(lazyInjections);
}
container.registry.validateInjections(lazyInjections);
}

validationCache[fullName] = true;
});
validationCache[fullName] = true;
});

this.class._toString = container.registry.makeToString(factory, fullName);
this.class[NAME_KEY] = container.registry.makeToString(factory, fullName);

if (!this.class.create) {
throw new Error(`Failed to create an instance of '${normalizedName}'. Most likely an improperly defined class or` +
` an invalid module export.`);
}
if (!this.class.create) {
throw new Error(`Failed to create an instance of '${normalizedName}'. Most likely an improperly defined class or` +
` an invalid module export.`);
}

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

return this.class.create(props);
}
};
} else {
let factory = this.lookupFactory(fullName, options);
if (factory === undefined) { return; }

return this.class.create(props);
}
};
manager = {
class: factory,
create(props = {}) {
return instantiate(factory, props, container, fullName);
}
};
}

runInDebug(() => {
if (HAS_PROXY) {
Expand Down Expand Up @@ -430,6 +443,61 @@ function injectionsFor(container, fullName) {
return injections;
}

function instantiate(factory, props, container, fullName) {
let lazyInjections, validationCache;

if (container.registry.getOption(fullName, 'instantiate') === false) {
return factory;
}

if (factory) {
if (typeof factory.create !== 'function') {
throw new Error(`Failed to create an instance of '${fullName}'. Most likely an improperly defined class or` +
` an invalid module export.`);
}

validationCache = container.validationCache;

runInDebug(() => {
// Ensure that all lazy injections are valid at instantiation time
if (!validationCache[fullName] && typeof factory._lazyInjections === 'function') {
lazyInjections = factory._lazyInjections();
lazyInjections = container.registry.normalizeInjectionsHash(lazyInjections);

container.registry.validateInjections(lazyInjections);
}
});

validationCache[fullName] = true;

let obj;

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

// Ensure that a container is available to an object during instantiation.
// TODO - remove when Ember reaches v3.0.0
// This "fake" container will be replaced after instantiation with a
// property that raises deprecations every time it is accessed.
injections.container = container._fakeContainerToInject;
obj = factory.create(assign({}, injections, props));

// TODO - remove when Ember reaches v3.0.0
if (!Object.isFrozen(obj) && 'container' in obj) {
injectDeprecatedContainer(obj, container);
}
}

return obj;
}
}

function factoryInjectionsFor(container, fullName) {
let registry = container.registry;
let splitName = fullName.split(':');
Expand Down
34 changes: 23 additions & 11 deletions packages/container/tests/container_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,13 @@ QUnit.test('A deprecated `container` property is appended to every object instan
});

// This is testing that container was passed as an option
QUnit.skip('A deprecated `container` property is appended to every object instantiated from a non-extendable factory, and a fake container is available during instantiation.', function() {
expect(8);
QUnit.test('A deprecated `container` property is appended to every object instantiated from a non-extendable factory, and a fake container is available during instantiation.', function() {
if (!isFeatureEnabled('container-factoryFor')) {
expect(8);
} else {
expect(1);
ok(true, '[SKIPPED] This will be removed when `factoryFor` lands.');
}

let owner = {};
let registry = new Registry();
Expand Down Expand Up @@ -620,16 +625,19 @@ QUnit.skip('A deprecated `container` property is appended to every object instan
};

registry.register('controller:post', PostController);
let postController = container.lookup('controller:post');

expectDeprecation(() => {
get(postController, 'container');
}, 'Using the injected `container` is deprecated. Please use the `getOwner` helper instead to access the owner of this object.');
if (!isFeatureEnabled('container-factoryFor')) {
let postController = container.lookup('controller:post');

expectDeprecation(() => {
let c = postController.container;
strictEqual(c, container, 'Injected container is now regular (not fake) container, but access is still deprecated.');
}, 'Using the injected `container` is deprecated. Please use the `getOwner` helper instead to access the owner of this object.');
expectDeprecation(() => {
get(postController, 'container');
}, 'Using the injected `container` is deprecated. Please use the `getOwner` helper instead to access the owner of this object.');

expectDeprecation(() => {
let c = postController.container;
strictEqual(c, container, 'Injected container is now regular (not fake) container, but access is still deprecated.');
}, 'Using the injected `container` is deprecated. Please use the `getOwner` helper instead to access the owner of this object.');
}
});

// We're now setting this on the prototype
Expand Down Expand Up @@ -764,7 +772,11 @@ QUnit.test('#factoryFor class returns the factory function', (assert) => {
registry.register('component:foo-bar', Component);

let factoryCreator = container.factoryFor('component:foo-bar');
assert.deepEqual(factoryCreator.class, Component);
if (isFeatureEnabled('container-factoryFor')) {
assert.deepEqual(factoryCreator.class, Component, 'No double extend');
} else {
assert.notDeepEqual(factoryCreator.class, Component, 'Double extended class');
}
});

QUnit.test('#factoryFor instance have a common parent', (assert) => {
Expand Down
4 changes: 0 additions & 4 deletions packages/ember-routing/lib/system/route.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, {
let controllerProto, combinedQueryParameterConfiguration;

let controllerName = this.controllerName || this.routeName;
<<<<<<< 292bb92758080866f40d54131c1e55432e3eb69a
let definedControllerClass = getOwner(this)._lookupFactory(`controller:${controllerName}`);
=======
let definedControllerClass = getOwner(this).factoryFor(`controller:${controllerName}`);
>>>>>>> [WIP] Feature factoryFor
let queryParameterConfiguraton = get(this, 'queryParams');
let hasRouterDefinedQueryParams = !!Object.keys(queryParameterConfiguraton).length;

Expand Down

0 comments on commit f6dfba0

Please sign in to comment.