Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE factory-for] Implement factoryFor #14360

Merged
merged 6 commits into from
Dec 18, 2016
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion features.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
"ember-improved-instrumentation": null,
"ember-metal-weakmap": null,
"ember-glimmer-allow-backtracking-rerender": null,
"ember-testing-resume-test": null
"ember-testing-resume-test": null,
"ember-factory-for": null,
"ember-no-double-extend": null
}
}
286 changes: 259 additions & 27 deletions packages/container/lib/container.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
import { dictionary, symbol, setOwner, OWNER, NAME_KEY } from 'ember-utils';
/* globals Proxy */
import {
dictionary,
symbol,
setOwner,
OWNER,
assign,
NAME_KEY
} from 'ember-utils';
import { ENV } from 'ember-environment';
import { assert, deprecate, runInDebug } from 'ember-metal';
import { assert, deprecate, runInDebug, isFeatureEnabled } from 'ember-metal';

const CONTAINER_OVERRIDE = symbol('CONTAINER_OVERRIDE');
const HAS_PROXY = typeof Proxy === 'function';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this into ember-utils and export as HAS_NATIVE_PROXY? Very similar to what was done in #14649 for packages/ember-utils/lib/weak-map-utils.js.

export const FACTORY_FOR = symbol('FACTORY_FOR');
export const LOOKUP_FACTORY = symbol('LOOKUP_FACTORY');

/**
A container used to instantiate and cache objects.
Expand All @@ -28,6 +39,9 @@ export default function Container(registry, options) {
this.isDestroyed = false;
}

Container.__FACTORY_FOR__ = FACTORY_FOR;
Copy link
Contributor Author

@chadhietala chadhietala Nov 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems bad but don't know how to make the Node tests pass without doing this. Don't have access to the internal loader.

Container.__LOOKUP_FACTORY__ = LOOKUP_FACTORY;

Container.prototype = {
/**
@private
Expand Down Expand Up @@ -127,7 +141,45 @@ Container.prototype = {
*/
lookupFactory(fullName, options) {
assert('fullName must be a proper full name', this.registry.validateFullName(fullName));
return factoryFor(this, this.registry.normalize(fullName), options);

deprecate(
'Using "_lookupFactory" is deprecated. Please use container.factoryFor instead.',
!isFeatureEnabled('ember-factory-for'),
{ id: 'container-lookupFactory', until: '2.13.0', url: 'TODO' }
);

return deprecatedFactoryFor(this, this.registry.normalize(fullName), options);
},

[LOOKUP_FACTORY](fullName, options) {
assert('fullName must be a proper full name', this.registry.validateFullName(fullName));
return deprecatedFactoryFor(this, this.registry.normalize(fullName), options);
},

/*
* This internal version of factoryFor swaps between the public API for
* factoryFor (class is the registered class) and a transition implementation
* (class is the double-extended class). It is *not* the public API version
* of factoryFor, which always returns the registered class.
*/
[FACTORY_FOR](fullName, options = {}) {
if (isFeatureEnabled('ember-no-double-extend')) {
if (isFeatureEnabled('ember-factory-for')) {
return this.factoryFor(fullName, options);
} else {
/* This throws in case of a poorly designed build */
throw new Error('If ember-no-double-extend is enabled, ember-factory-for must also be enabled');
}
}
let factory = this.lookupFactory(fullName, options);
if (factory === undefined) { return; }
let manager = new DeprecatedFactoryManager(this, factory, fullName);

runInDebug(() => {
manager = wrapManagerInDeprecationProxy(manager);
});

return manager;
},

/**
Expand Down Expand Up @@ -175,10 +227,88 @@ Container.prototype = {
}
};

/*
* Wrap a factory manager in a proxy which will not permit properties to be
* set on the manager.
*/
function wrapManagerInDeprecationProxy(manager) {
if (HAS_PROXY) {
let validator = {
get(obj, prop) {
if (prop !== 'class' && prop !== 'create') {
throw new Error(`You attempted to access "${prop}" on a factory manager created by container#factoryFor. "${prop}" is not a member of a factory manager."`);
}

return obj[prop];
},
set(obj, prop, value) {
throw new Error(`You attempted to set "${prop}" on a factory manager created by container#factoryFor. A factory manager is a read-only construct.`);
}
};

// Note:
// We have to proxy access to the manager here so that private property
// access doesn't cause the above errors to occur.
let m = manager;
let proxiedManager = {
class: m.class,
create(props) {
return m.create(props);
}
};

return new Proxy(proxiedManager, validator);
}

return manager;
}

if (isFeatureEnabled('ember-factory-for')) {
/**
Given a fullName, return the corresponding factory. The consumer of the factory
is responsible for the destruction of any factory instances, as there is no
way for the container to ensure instances are destroyed when it itself is
destroyed.

@public
@method factoryFor
@param {String} fullName
@param {Object} [options]
@param {String} [options.source] The fullname of the request source (used for local lookup)
@return {any}
*/
Container.prototype.factoryFor = function _factoryFor(fullName, options = {}) {
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; }
}

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

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

let manager = new FactoryManager(this, factory, fullName, normalizedName);

runInDebug(() => {
manager = wrapManagerInDeprecationProxy(manager);
});

return manager;
};
}

function isSingleton(container, fullName) {
return container.registry.getOption(fullName, 'singleton') !== false;
}

function shouldInstantiate(container, fullName) {
return container.registry.getOption(fullName, 'instantiate') !== false;
}

function lookup(container, fullName, options = {}) {
if (options.source) {
fullName = container.registry.expandLocalLookup(fullName, options);
Expand All @@ -191,15 +321,64 @@ function lookup(container, fullName, options = {}) {
return container.cache[fullName];
}

let value = instantiate(container, fullName);
if (isFeatureEnabled('ember-factory-for')) {
return instantiateFactory(container, fullName, options);
} else {
let factory = deprecatedFactoryFor(container, fullName);
let value = instantiate(factory, {}, container, fullName);

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

if (isSingleton(container, fullName) && options.singleton !== false) {
container.cache[fullName] = value;
}

return value;
}
}

function isSingletonClass(container, fullName, { instantiate, singleton }) {
return (singleton !== false && isSingleton(container, fullName)) &&
(!instantiate && !shouldInstantiate(container, fullName));
}

function isSingletonInstance(container, fullName, { instantiate, singleton }) {
return (singleton !== false && isSingleton(container, fullName)) &&
(instantiate !== false && shouldInstantiate(container, fullName));
}

function isFactoryClass(container, fullname, { instantiate, singleton }) {
return (singleton === false || !isSingleton(container, fullname)) &&
(instantiate === false && !shouldInstantiate(container, fullname));
}

function isFactoryInstance(container, fullName, { instantiate, singleton }) {
return (singleton !== false || isSingleton(container, fullName)) &&
(instantiate !== false && shouldInstantiate(container, fullName));
}

function instantiateFactory(container, fullName, options) {
let factoryManager = container[FACTORY_FOR](fullName);

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

if (value === undefined) { return; }
// SomeClass { singleton: true, instantiate: true } | { singleton: true } | { instantiate: true } | {}
// By default majority of objects fall into this case
if (isSingletonInstance(container, fullName, options)) {
return container.cache[fullName] = factoryManager.create();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Importantly, this cache should only be populated with instances created during lookup. Instances created as a result of factory.create are not the singleton instance. 👍

}

// SomeClass { singleton: false, instantiate: true }
if (isFactoryInstance(container, fullName, options)) {
return factoryManager.create();
}

if (isSingleton(container, fullName) && options.singleton !== false) {
container.cache[fullName] = value;
// SomeClass { singleton: true, instantiate: false } | { instantiate: false } | { singleton: false, instantiation: false }
if (isSingletonClass(container, fullName, options) || isFactoryClass(container, fullName, options)) {
return factoryManager.class;
}

return value;
throw new Error('Could not create factory');
}

function markInjectionsAsDynamic(injections) {
Expand Down Expand Up @@ -238,12 +417,11 @@ function buildInjections(/* container, ...injections */) {
return hash;
}

function factoryFor(container, fullName, options = {}) {
function deprecatedFactoryFor(container, fullName, options = {}) {
let registry = container.registry;

if (options.source) {
fullName = registry.expandLocalLookup(fullName, options);

// if expandLocalLookup returns falsey, we do not support local lookup
if (!fullName) { return; }
}
Expand Down Expand Up @@ -305,23 +483,11 @@ function injectionsFor(container, fullName) {
return injections;
}

function factoryInjectionsFor(container, fullName) {
let registry = container.registry;
let splitName = fullName.split(':');
let type = splitName[0];

let factoryInjections = buildInjections(container,
registry.getFactoryTypeInjections(type),
registry.getFactoryInjections(fullName));
factoryInjections._debugContainerKey = fullName;

return factoryInjections;
}

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

props = props || {};

if (container.registry.getOption(fullName, 'instantiate') === false) {
return factory;
}
Expand Down Expand Up @@ -350,7 +516,7 @@ function instantiate(container, fullName) {

if (typeof factory.extend === 'function') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this refer to the factory.class or our new factory object, if it is our new factory object this may no longer make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refers to extend on the registered class, not extend on the factory object. The idea being extend is supported, injections have been made.

instantiate is only called by lookup though, and in that case we want the double extend to continue for now so this test would hold true.

// assume the factory was extendable and is already injected
obj = factory.create();
obj = factory.create(props);
} else {
// assume the factory was extendable
// to create time injections
Expand All @@ -362,7 +528,7 @@ function instantiate(container, fullName) {
// 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(injections);
obj = factory.create(assign({}, injections, props));

// TODO - remove when Ember reaches v3.0.0
if (!Object.isFrozen(obj) && 'container' in obj) {
Expand All @@ -374,6 +540,19 @@ function instantiate(container, fullName) {
}
}

function factoryInjectionsFor(container, fullName) {
let registry = container.registry;
let splitName = fullName.split(':');
let type = splitName[0];

let factoryInjections = buildInjections(container,
registry.getFactoryTypeInjections(type),
registry.getFactoryInjections(fullName));
factoryInjections._debugContainerKey = fullName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mixonic / @rwjblue should this become a symbol or something in the future?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future perhaps. I don't think it should be changed in this effort.


return factoryInjections;
}

// TODO - remove when Ember reaches v3.0.0
function injectDeprecatedContainer(object, container) {
Object.defineProperty(object, 'container', {
Expand Down Expand Up @@ -466,3 +645,56 @@ function buildFakeContainerFunction(container, containerProperty, ownerProperty)
return container[containerProperty](...arguments);
};
}

class DeprecatedFactoryManager {
constructor(container, factory, fullName) {
this.container = container;
this.class = factory;
this.fullName = fullName;
}

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

class FactoryManager {
constructor(container, factory, fullName, normalizedName) {
this.container = container;
this.class = factory;
this.fullName = fullName;
this.normalizedName = normalizedName;
}

create(options = {}) {
let injections = injectionsFor(this.container, this.normalizedName);
let props = assign({}, injections, options);

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

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

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

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

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

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

return this.class.create(props);
}
}
4 changes: 3 additions & 1 deletion packages/container/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ The public API, specified on the application namespace should be considered the
export { default as Registry, privatize } from './registry';
export {
default as Container,
buildFakeContainerWithDeprecations
buildFakeContainerWithDeprecations,
FACTORY_FOR,
LOOKUP_FACTORY
} from './container';
Loading