Skip to content

Commit

Permalink
[FEATURE ember-module-unification] Initial work
Browse files Browse the repository at this point in the history
* Pass `source` to the resolver's `resolve` method as a second argument
* Update the logic for component pairs to ensure local/private
  resolution of a component/template are only matched with another local/private
  resolution.
  • Loading branch information
iezer authored and mixonic committed Jul 1, 2017
1 parent 16a9ede commit 5e0a1fd
Show file tree
Hide file tree
Showing 10 changed files with 317 additions and 54 deletions.
16 changes: 16 additions & 0 deletions FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,19 @@ for a detailed explanation.

Adds an ability to for developers to integrate their own custom component managers
into Ember Applications per [RFC](https://github.com/emberjs/rfcs/blob/custom-components/text/0000-custom-components.md).

* `ember-module-unification`

Introduces support for Module Unification
([RFC](https://github.com/dgeb/rfcs/blob/module-unification/text/0000-module-unification.md))
to Ember. This includes:

- Passing the `source` of a `lookup`/`factoryFor` call as the second argument
to an Ember resolver's `resolve` method (as a positional arg we will call
`referrer`).
- Making `lookupComponentPair` friendly to local/private resolutions. The
new code ensures a local resolution is not paired with a global resolution.

This feature is paired with the
[`EMBER_RESOLVER_MODULE_UNIFICATION`](https://github.com/ember-cli/ember-resolver#ember_resolver_module_unification)
flag on the ember-resolver package.
1 change: 1 addition & 0 deletions features.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"ember-glimmer-allow-backtracking-rerender": null,
"ember-routing-router-service": true,
"ember-engines-mount-params": true,
"ember-module-unification": null,
"glimmer-custom-component-manager": null
},
"deprecations": {
Expand Down
53 changes: 40 additions & 13 deletions packages/container/lib/container.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* globals Proxy */
import { assert } from 'ember-debug';
import { EMBER_MODULE_UNIFICATION } from 'ember/features';
import { DEBUG } from 'ember-env-flags';
import {
dictionary,
Expand Down Expand Up @@ -135,6 +136,10 @@ Container.prototype = {
return { [OWNER]: this.owner };
},

_resolverCacheKey(name, options) {
return this.registry.resolverCacheKey(name, options);
},

/**
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
Expand All @@ -153,18 +158,28 @@ Container.prototype = {
assert('fullName must be a proper full name', this.registry.validateFullName(normalizedName));

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

normalizedName = expandedFullName;
} else if (expandedFullName) {
// with ember-module-unification, if expandLocalLookup returns something,
// pass it to the resolve without the source
normalizedName = expandedFullName;
options = {};
}
}

let cached = this.factoryManagerCache[normalizedName];
let cacheKey = this._resolverCacheKey(normalizedName, options);
let cached = this.factoryManagerCache[cacheKey];

if (cached !== undefined) { return cached; }

let factory = this.registry.resolve(normalizedName);
let factory = EMBER_MODULE_UNIFICATION ? this.registry.resolve(normalizedName, options) : this.registry.resolve(normalizedName);

if (factory === undefined) {
return;
Expand All @@ -180,7 +195,7 @@ Container.prototype = {
manager = wrapManagerInDeprecationProxy(manager);
}

this.factoryManagerCache[normalizedName] = manager;
this.factoryManagerCache[cacheKey] = manager;
return manager;
}
};
Expand Down Expand Up @@ -224,15 +239,25 @@ function isInstantiatable(container, fullName) {

function lookup(container, fullName, options = {}) {
if (options.source) {
fullName = container.registry.expandLocalLookup(fullName, options);
let expandedFullName = container.registry.expandLocalLookup(fullName, options);

// if expandLocalLookup returns falsey, we do not support local lookup
if (!fullName) {
return;
if (!EMBER_MODULE_UNIFICATION) {
// if expandLocalLookup returns falsey, we do not support local lookup
if (!expandedFullName) {
return;
}

fullName = expandedFullName;
} else if (expandedFullName) {
// with ember-module-unification, if expandLocalLookup returns something,
// pass it to the resolve without the source
fullName = expandedFullName;
options = {};
}
}

let cached = container.cache[fullName];
let cacheKey = container._resolverCacheKey(fullName, options);
let cached = container.cache[cacheKey];
if (cached !== undefined && options.singleton !== false) {
return cached;
}
Expand All @@ -257,16 +282,18 @@ function isFactoryInstance(container, fullName, { instantiate, singleton }) {
}

function instantiateFactory(container, fullName, options) {
let factoryManager = container.factoryFor(fullName);
let factoryManager = EMBER_MODULE_UNIFICATION && options && options.source ? container.factoryFor(fullName, options) : container.factoryFor(fullName);

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

let cacheKey = container._resolverCacheKey(fullName, options);

// 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();
return container.cache[cacheKey] = factoryManager.create();
}

// SomeClass { singleton: false, instantiate: true }
Expand Down
35 changes: 28 additions & 7 deletions packages/container/lib/registry.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { dictionary, assign, intern } from 'ember-utils';
import { assert, deprecate } from 'ember-debug';
import { EMBER_MODULE_UNIFICATION } from 'ember/features';
import Container from './container';
import { DEBUG } from 'ember-env-flags';

Expand Down Expand Up @@ -604,6 +605,14 @@ Registry.prototype = {
injections = injections.concat(this.fallback.getTypeInjections(type));
}
return injections;
},

resolverCacheKey(name, options) {
if (!EMBER_MODULE_UNIFICATION) {
return name;
}

return (options && options.source) ? `${options.source}:${name}` : name;
}
};

Expand Down Expand Up @@ -686,30 +695,42 @@ function resolve(registry, normalizedName, options) {
if (options && options.source) {
// when `source` is provided expand normalizedName
// and source into the full normalizedName
normalizedName = registry.expandLocalLookup(normalizedName, options);
let expandedNormalizedName = registry.expandLocalLookup(normalizedName, options);

// if expandLocalLookup returns falsey, we do not support local lookup
if (!normalizedName) { return; }
if (!EMBER_MODULE_UNIFICATION) {
if (!expandedNormalizedName) {
return;
}

normalizedName = expandedNormalizedName;
} else if (expandedNormalizedName) {
// with ember-module-unification, if expandLocalLookup returns something,
// pass it to the resolve without the source
normalizedName = expandedNormalizedName;
options = {};
}
}

let cached = registry._resolveCache[normalizedName];
let cacheKey = registry.resolverCacheKey(normalizedName, options);
let cached = registry._resolveCache[cacheKey];
if (cached !== undefined) { return cached; }
if (registry._failCache[normalizedName]) { return; }
if (registry._failCache[cacheKey]) { return; }

let resolved;

if (registry.resolver) {
resolved = registry.resolver.resolve(normalizedName);
resolved = registry.resolver.resolve(normalizedName, options && options.source);
}

if (resolved === undefined) {
resolved = registry.registrations[normalizedName];
}

if (resolved === undefined) {
registry._failCache[normalizedName] = true;
registry._failCache[cacheKey] = true;
} else {
registry._resolveCache[normalizedName] = resolved;
registry._resolveCache[cacheKey] = resolved;
}

return resolved;
Expand Down
45 changes: 45 additions & 0 deletions packages/container/tests/container_test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { getOwner, OWNER, assign } from 'ember-utils';
import { ENV } from 'ember-environment';
import { get } from 'ember-metal';
import { EMBER_MODULE_UNIFICATION } from 'ember/features';
import { Registry } from '..';
import { factory } from 'internal-test-helpers';

Expand Down Expand Up @@ -615,3 +616,47 @@ QUnit.skip('#factoryFor does not add properties to the object being instantiated
// not via registry/container shenanigans
assert.deepEqual(Object.keys(instance), []);
});

if (EMBER_MODULE_UNIFICATION) {
QUnit.module('Container module unification');

QUnit.test('The container can pass a source to factoryFor', function(assert) {
let PrivateComponent = factory();
let lookup = 'component:my-input';
let expectedSource = 'template:routes/application';
let registry = new Registry();
let resolveCount = 0;
registry.resolve = function(fullName, { source }) {
resolveCount++;
if (fullName === lookup && source === expectedSource) {
return PrivateComponent;
}
};

let container = registry.container();

assert.strictEqual(container.factoryFor(lookup, { source: expectedSource }).class, PrivateComponent, 'The correct factory was provided');
assert.strictEqual(container.factoryFor(lookup, { source: expectedSource }).class, PrivateComponent, 'The correct factory was provided again');
assert.equal(resolveCount, 1, 'resolve called only once and a cached factory was returned the second time');
});

QUnit.test('The container can pass a source to lookup', function(assert) {
let PrivateComponent = factory();
let lookup = 'component:my-input';
let expectedSource = 'template:routes/application';
let registry = new Registry();
registry.resolve = function(fullName, { source }) {
if (fullName === lookup && source === expectedSource) {
return PrivateComponent;
}
};

let container = registry.container();

let result = container.lookup(lookup, { source: expectedSource });
assert.ok(result instanceof PrivateComponent, 'The correct factory was provided');

assert.ok(container.cache[`template:routes/application:component:my-input`] instanceof PrivateComponent,
'The correct factory was stored in the cache with the correct key which includes the source.');
});
}
29 changes: 29 additions & 0 deletions packages/container/tests/registry_test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Registry, privatize } from '..';
import { factory } from 'internal-test-helpers';
import { EMBER_MODULE_UNIFICATION } from 'ember/features';

QUnit.module('Registry');

Expand Down Expand Up @@ -691,6 +692,7 @@ QUnit.test('has uses expandLocalLookup', function(assert) {

let resolver = {
resolve(name) {
if (EMBER_MODULE_UNIFICATION && name === 'foo:baz') { return; }
resolvedFullNames.push(name);

return 'yippie!';
Expand Down Expand Up @@ -737,3 +739,30 @@ QUnit.test('valid format', function(assert) {
assert.equal(matched[2], 'factory');
assert.ok(/^\d+$/.test(matched[3]));
});

if (EMBER_MODULE_UNIFICATION) {
QUnit.module('Registry module unification');

QUnit.test('The registry can pass a source to the resolver', function(assert) {
let PrivateComponent = factory();
let lookup = 'component:my-input';
let source = 'template:routes/application';
let resolveCount = 0;
let resolver = {
resolve(fullName, src) {
resolveCount++;
if (fullName === lookup && src === source) {
return PrivateComponent;
}
}
};
let registry = new Registry({ resolver });
registry.normalize = function(name) {
return name;
};

assert.strictEqual(registry.resolve(lookup, { source }), PrivateComponent, 'The correct factory was provided');
assert.strictEqual(registry.resolve(lookup, { source }), PrivateComponent, 'The correct factory was provided again');
assert.equal(resolveCount, 1, 'resolve called only once and a cached factory was returned the second time');
});
}
9 changes: 8 additions & 1 deletion packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { guidFor, OWNER } from 'ember-utils';
import { Cache, _instrumentStart } from 'ember-metal';
import { assert, warn } from 'ember-debug';
import { EMBER_MODULE_UNIFICATION } from 'ember/features';
import { DEBUG } from 'ember-env-flags';
import {
lookupPartial,
Expand Down Expand Up @@ -91,7 +92,8 @@ export default class Environment extends GlimmerEnvironment {
return new CurlyComponentDefinition(name, componentFactory, layout, undefined, customManager);
}
}, ({ name, source, owner }) => {
let expandedName = source && owner._resolveLocalLookupName(name, source) || name;
let expandedName = source && this._resolveLocalLookupName(name, source, owner) || name;

let ownerGuid = guidFor(owner);

return ownerGuid + '|' + expandedName;
Expand Down Expand Up @@ -147,6 +149,11 @@ export default class Environment extends GlimmerEnvironment {
}
}

_resolveLocalLookupName(name, source, owner) {
return EMBER_MODULE_UNIFICATION ? `${source}:${name}`
: owner._resolveLocalLookupName(name, source);
}

macros() {
let macros = super.macros();
populateMacros(macros.blocks, macros.inlines);
Expand Down
Loading

0 comments on commit 5e0a1fd

Please sign in to comment.