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.

Sign up for GitHub

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

Jump to bottom

Remove internal factoryInjection API #14816

Closed
wants to merge 3 commits into from
Closed

Remove internal factoryInjection API #14816

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d69f8d3
Remove _factoryInjections and _factoryTypeInjections dictionaries
locks Jan 11, 2017
5b2dc4d
Avoid `injectedFactory.reopenClass`
locks Jan 20, 2017
7bf7b00
Merge branch 'master' into skull-emoji-factory-injection
locks Mar 16, 2017
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
20 changes: 2 additions & 18 deletions packages/container/lib/container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,16 +447,13 @@ function deprecatedFactoryFor(container, fullName, options = {}) {
return factory;
} else {
let injections = injectionsFor(container, fullName);
let factoryInjections = factoryInjectionsFor(container, fullName);
let cacheable = !areInjectionsDynamic(injections) && !areInjectionsDynamic(factoryInjections);

factoryInjections[NAME_KEY] = registry.makeToString(factory, fullName);
let cacheable = !areInjectionsDynamic(injections);

let injectedFactory = factory.extend(injections);

// TODO - remove all `container` injections when Ember reaches v3.0.0
injectDeprecatedContainer(injectedFactory.prototype, container);
injectedFactory.reopenClass(factoryInjections);
injectedFactory.reopenClass({ [NAME_KEY]: registry.makeToString(factory, fullName) });
Copy link
Member

Choose a reason for hiding this comment

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

Ideally we can get rid of this also (I'm not sure we have solved the "problem" of factory injections if we cannot remove this reopenClass).


if (factory && typeof factory._onLookup === 'function') {
factory._onLookup(fullName);
Expand Down Expand Up @@ -542,19 +539,6 @@ function instantiate(factory, props, 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;

return factoryInjections;
}

// TODO - remove when Ember reaches v3.0.0
function injectDeprecatedContainer(object, container) {
Object.defineProperty(object, 'container', {
Expand Down
143 changes: 0 additions & 143 deletions packages/container/lib/registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ export default function Registry(options) {

this._typeInjections = dictionary(null);
this._injections = dictionary(null);
this._factoryTypeInjections = dictionary(null);
this._factoryInjections = dictionary(null);

this._localLookupCache = new EmptyObject();
this._normalizeCache = dictionary(null);
Expand Down Expand Up @@ -86,22 +84,6 @@ Registry.prototype = {
*/
_injections: null,

/**
@private

@property _factoryTypeInjections
@type InheritingDict
*/
_factoryTypeInjections: null,

/**
@private

@property _factoryInjections
@type InheritingDict
*/
_factoryInjections: null,

/**
@private

Expand Down Expand Up @@ -549,115 +531,6 @@ Registry.prototype = {
});
},


/**
Used only via `factoryInjection`.

Provides a specialized form of injection, specifically enabling
all factory of one type to be injected with a reference to another
object.

For example, provided each factory of type `model` needed a `store`.
one would do the following:

```javascript
let registry = new Registry();

registry.register('store:main', SomeStore);

registry.factoryTypeInjection('model', 'store', 'store:main');

let store = registry.lookup('store:main');
let UserFactory = registry.lookupFactory('model:user');

UserFactory.store instanceof SomeStore; //=> true
```

@private
@method factoryTypeInjection
@param {String} type
@param {String} property
@param {String} fullName
*/
factoryTypeInjection(type, property, fullName) {
let injections = this._factoryTypeInjections[type] ||
(this._factoryTypeInjections[type] = []);

injections.push({
property: property,
fullName: this.normalize(fullName)
});
},

/**
Defines factory injection rules.

Similar to regular injection rules, but are run against factories, via
`Registry#lookupFactory`.

These rules are used to inject objects onto factories when they
are looked up.

Two forms of injections are possible:

* Injecting one fullName on another fullName
* Injecting one fullName on a type

Example:

```javascript
let registry = new Registry();
let container = registry.container();

registry.register('store:main', Store);
registry.register('store:secondary', OtherStore);
registry.register('model:user', User);
registry.register('model:post', Post);

// injecting one fullName on another type
registry.factoryInjection('model', 'store', 'store:main');

// injecting one fullName on another fullName
registry.factoryInjection('model:post', 'secondaryStore', 'store:secondary');

let UserFactory = container.lookupFactory('model:user');
let PostFactory = container.lookupFactory('model:post');
let store = container.lookup('store:main');

UserFactory.store instanceof Store; //=> true
UserFactory.secondaryStore instanceof OtherStore; //=> false

PostFactory.store instanceof Store; //=> true
PostFactory.secondaryStore instanceof OtherStore; //=> true

// and both models share the same source instance
UserFactory.store === PostFactory.store; //=> true
```

@private
@method factoryInjection
@param {String} factoryName
@param {String} property
@param {String} injectionName
*/
factoryInjection(fullName, property, injectionName) {
let normalizedName = this.normalize(fullName);
let normalizedInjectionName = this.normalize(injectionName);

this.validateFullName(injectionName);

if (fullName.indexOf(':') === -1) {
return this.factoryTypeInjection(normalizedName, property, normalizedInjectionName);
}

let injections = this._factoryInjections[normalizedName] || (this._factoryInjections[normalizedName] = []);

injections.push({
property: property,
fullName: normalizedInjectionName
});
},

/**
@private
@method knownForType
Expand Down Expand Up @@ -745,22 +618,6 @@ Registry.prototype = {
injections = injections.concat(this.fallback.getTypeInjections(type));
}
return injections;
},

getFactoryInjections(fullName) {
let injections = this._factoryInjections[fullName] || [];
if (this.fallback) {
injections = injections.concat(this.fallback.getFactoryInjections(fullName));
}
return injections;
},

getFactoryTypeInjections(type) {
let injections = this._factoryTypeInjections[type] || [];
if (this.fallback) {
injections = injections.concat(this.fallback.getFactoryTypeInjections(type));
}
return injections;
}
};

Expand Down
32 changes: 0 additions & 32 deletions packages/container/tests/container_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,6 @@ QUnit.test('A registered factory is returned from lookupFactory is the same fact
deepEqual(Post1, Post2, 'The return of lookupFactory is always the same');
});

QUnit.test('A factory returned from lookupFactory has a debugkey', function() {
let registry = new Registry();
let container = registry.container();
let PostController = factory();

registry.register('controller:post', PostController);
let PostFactory = lookupFactory('controller:post', container);
equal(PostFactory._debugContainerKey, 'controller:post', 'factory instance receives _debugContainerKey');
});

QUnit.test('fallback for to create time injections if factory has no extend', function() {
let registry = new Registry();
let container = registry.container();
Expand Down Expand Up @@ -184,28 +174,6 @@ QUnit.test('A factory with both type and individual injections', function() {
equal(postController.router, router);
});

QUnit.test('A factory with both type and individual factoryInjections', function() {
let registry = new Registry();
let container = registry.container();
let PostController = factory();
let Store = factory();
let Router = factory();

registry.register('controller:post', PostController);
registry.register('store:main', Store);
registry.register('router:main', Router);

registry.factoryInjection('controller:post', 'store', 'store:main');
registry.factoryTypeInjection('controller', 'router', 'router:main');

let PostControllerFactory = lookupFactory('controller:post', container);
let store = container.lookup('store:main');
let router = container.lookup('router:main');

equal(PostControllerFactory.store, store, 'PostControllerFactory has the instance of store');
equal(PostControllerFactory.router, router, 'PostControllerFactory has the route instance');
});

QUnit.test('A non-singleton instance is never cached', function() {
let registry = new Registry();
let container = registry.container();
Expand Down
23 changes: 0 additions & 23 deletions packages/container/tests/registry_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,29 +436,6 @@ QUnit.test('`getTypeInjections` includes type injections from a fallback registr
equal(registry.getTypeInjections('model').length, 1, 'Injections from the fallback registry are merged');
});

QUnit.test('`getFactoryInjections` includes factory injections from a fallback registry', function() {
let fallback = new Registry();
let registry = new Registry({ fallback: fallback });

equal(registry.getFactoryInjections('model:user').length, 0, 'No factory injections in the primary registry');

fallback.factoryInjection('model:user', 'store', 'store:main');

equal(registry.getFactoryInjections('model:user').length, 1, 'Factory injections from the fallback registry are merged');
});


QUnit.test('`getFactoryTypeInjections` includes factory type injections from a fallback registry', function() {
let fallback = new Registry();
let registry = new Registry({ fallback: fallback });

equal(registry.getFactoryTypeInjections('model').length, 0, 'No factory type injections in the primary registry');

fallback.factoryInjection('model', 'store', 'store:main');

equal(registry.getFactoryTypeInjections('model').length, 1, 'Factory type injections from the fallback registry are merged');
});

QUnit.test('`knownForType` contains keys for each item of a given type', function() {
let registry = new Registry();

Expand Down