diff --git a/FEATURES.md b/FEATURES.md index 3456d4ec2a3..4233f6c3a20 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -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. diff --git a/features.json b/features.json index 4f73e99c77f..5c4ee8a2222 100644 --- a/features.json +++ b/features.json @@ -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": { diff --git a/packages/container/lib/container.js b/packages/container/lib/container.js index fd1c54a8c41..818ebd96e38 100644 --- a/packages/container/lib/container.js +++ b/packages/container/lib/container.js @@ -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, @@ -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 @@ -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; @@ -180,7 +195,7 @@ Container.prototype = { manager = wrapManagerInDeprecationProxy(manager); } - this.factoryManagerCache[normalizedName] = manager; + this.factoryManagerCache[cacheKey] = manager; return manager; } }; @@ -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; } @@ -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 } diff --git a/packages/container/lib/registry.js b/packages/container/lib/registry.js index 6128cc6463f..6de09fdee55 100644 --- a/packages/container/lib/registry.js +++ b/packages/container/lib/registry.js @@ -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'; @@ -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; } }; @@ -686,20 +695,32 @@ 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) { @@ -707,9 +728,9 @@ function resolve(registry, normalizedName, options) { } if (resolved === undefined) { - registry._failCache[normalizedName] = true; + registry._failCache[cacheKey] = true; } else { - registry._resolveCache[normalizedName] = resolved; + registry._resolveCache[cacheKey] = resolved; } return resolved; diff --git a/packages/container/tests/container_test.js b/packages/container/tests/container_test.js index 71f69d3f1c9..2d6f931ff11 100644 --- a/packages/container/tests/container_test.js +++ b/packages/container/tests/container_test.js @@ -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'; @@ -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.'); + }); +} diff --git a/packages/container/tests/registry_test.js b/packages/container/tests/registry_test.js index 286b3b5294f..804624a7021 100644 --- a/packages/container/tests/registry_test.js +++ b/packages/container/tests/registry_test.js @@ -1,5 +1,6 @@ import { Registry, privatize } from '..'; import { factory } from 'internal-test-helpers'; +import { EMBER_MODULE_UNIFICATION } from 'ember/features'; QUnit.module('Registry'); @@ -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!'; @@ -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'); + }); +} diff --git a/packages/ember-glimmer/lib/environment.js b/packages/ember-glimmer/lib/environment.js index 7367d64c1c9..e8087ff92d3 100644 --- a/packages/ember-glimmer/lib/environment.js +++ b/packages/ember-glimmer/lib/environment.js @@ -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, @@ -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; @@ -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); diff --git a/packages/ember-glimmer/tests/integration/components/local-lookup-test.js b/packages/ember-glimmer/tests/integration/components/local-lookup-test.js index ab640904df4..514881230b2 100644 --- a/packages/ember-glimmer/tests/integration/components/local-lookup-test.js +++ b/packages/ember-glimmer/tests/integration/components/local-lookup-test.js @@ -1,36 +1,10 @@ import { moduleFor, RenderingTest } from '../../utils/test-case'; +import { ModuleBasedTestResolver } from 'internal-test-helpers'; import { Component } from '../../utils/helpers'; +import { EMBER_MODULE_UNIFICATION } from 'ember/features'; +import { helper, Helper } from 'ember-glimmer'; -function buildResolver() { - let resolver = { - resolve() { }, - expandLocalLookup(fullName, sourceFullName) { - let [sourceType, sourceName ] = sourceFullName.split(':'); - let [type, name ] = fullName.split(':'); - - if (type !== 'template' && sourceType === 'template' && sourceName.slice(0, 11) === 'components/') { - sourceName = sourceName.slice(11); - } - - if (type === 'template' && sourceType === 'template' && name.slice(0, 11) === 'components/') { - name = name.slice(11); - } - - - let result = `${type}:${sourceName}/${name}`; - - return result; - } - }; - - return resolver; -} - -moduleFor('Components test: local lookup', class extends RenderingTest { - getResolver() { - return buildResolver(); - } - +class LocalLookupTest extends RenderingTest { ['@test it can lookup a local template']() { this.registerComponent('x-outer/x-inner', { template: 'Nested template says: {{yield}}' }); this.registerComponent('x-outer', { template: '{{#x-inner}}Hi!{{/x-inner}}' }); @@ -217,4 +191,114 @@ moduleFor('Components test: local lookup', class extends RenderingTest { this.assertText('Nested template says (from global): Hi! Nested template says (from local): Hi! Nested template says (from local): Hi!'); } +} + +// first run these tests with expandLocalLookup + +function buildResolver() { + let resolver = { + resolve() { }, + expandLocalLookup(fullName, sourceFullName) { + let [sourceType, sourceName ] = sourceFullName.split(':'); + let [type, name ] = fullName.split(':'); + + if (type !== 'template' && sourceType === 'template' && sourceName.slice(0, 11) === 'components/') { + sourceName = sourceName.slice(11); + } + + if (type === 'template' && sourceType === 'template' && name.slice(0, 11) === 'components/') { + name = name.slice(11); + } + + + let result = `${type}:${sourceName}/${name}`; + + return result; + } + }; + + return resolver; +} + +moduleFor('Components test: local lookup with expandLocalLookup feature', class extends LocalLookupTest { + getResolver() { + return buildResolver(); + } }); + +if (EMBER_MODULE_UNIFICATION) { + class LocalLookupTestResolver extends ModuleBasedTestResolver { + resolve(specifier, referrer) { + let fullSpecifier = specifier; + + if (referrer) { + let namespace = referrer.split('template:components/')[1]; + if (specifier.indexOf('template:components/') !== -1) { + let name = specifier.split('template:components/')[1]; + fullSpecifier = `template:components/${namespace}/${name}`; + } else if (specifier.indexOf(':') !== -1) { + let [type, name] = specifier.split(':'); + fullSpecifier = `${type}:${namespace}/${name}`; + } + } + + return super.resolve(fullSpecifier); + } + } + + /* + * This sub-classing changes `registerXXX` methods to use the resolver. + * Required for testing the module unification-friendly `resolve` call + * with a `referrer` argument. + * + * In theory all these tests can be ported to use the resolver instead of + * the registry. + */ + moduleFor('Components test: local lookup with resolution referrer', class extends LocalLookupTest { + get resolver() { + return this.owner.__registry__.fallback.resolver; + } + + getResolver() { + return new LocalLookupTestResolver(); + } + + registerComponent(name, { ComponentClass = null, template = null }) { + let { resolver } = this; + + if (ComponentClass) { + resolver.add(`component:${name}`, ComponentClass); + } + + if (typeof template === 'string') { + resolver.add(`template:components/${name}`, this.compile(template, { + moduleName: `components/${name}` + })); + } + } + + registerTemplate(name, template) { + let { resolver } = this; + if (typeof template === 'string') { + resolver.add(`template:${name}`, this.compile(template, { + moduleName: name + })); + } else { + throw new Error(`Registered template "${name}" must be a string`); + } + } + + registerHelper(name, funcOrClassBody) { + let { resolver } = this; + let type = typeof funcOrClassBody; + + if (type === 'function') { + resolver.add(`helper:${name}`, helper(funcOrClassBody)); + } else if (type === 'object' && type !== null) { + resolver.add(`helper:${name}`, Helper.extend(funcOrClassBody)); + } else { + throw new Error(`Cannot register ${funcOrClassBody} as a helper`); + } + } + }); +} diff --git a/packages/ember-views/lib/utils/lookup-component.js b/packages/ember-views/lib/utils/lookup-component.js index 2be90a86886..7457c3f6696 100644 --- a/packages/ember-views/lib/utils/lookup-component.js +++ b/packages/ember-views/lib/utils/lookup-component.js @@ -1,6 +1,39 @@ import { privatize as P } from 'container'; +import { EMBER_MODULE_UNIFICATION } from 'ember/features'; + +function lookupModuleUnificationComponentPair(componentLookup, owner, name, options) { + let localComponent = componentLookup.componentFor(name, owner, options); + let localLayout = componentLookup.layoutFor(name, owner, options); + + let globalComponent = componentLookup.componentFor(name, owner); + let globalLayout = componentLookup.layoutFor(name, owner); + + let localAndUniqueComponent = !!localComponent && (!globalComponent || localComponent.class !== globalComponent.class); + let localAndUniqueLayout = !!localLayout && (!globalLayout || localLayout.meta.moduleName !== globalLayout.meta.moduleName); + + if (localAndUniqueComponent && localAndUniqueLayout) { + return { layout: localLayout, component: localComponent }; + } + + if (localAndUniqueComponent && !localAndUniqueLayout) { + return { layout: null, component: localComponent }; + } + + let defaultComponentFactory = owner.factoryFor(P`component:-default`); + + if (!localAndUniqueComponent && localAndUniqueLayout) { + return { layout: localLayout, component: defaultComponentFactory }; + } + + let component = globalComponent || (globalLayout && defaultComponentFactory); + return { layout: globalLayout, component }; +} function lookupComponentPair(componentLookup, owner, name, options) { + if (EMBER_MODULE_UNIFICATION) { + return lookupModuleUnificationComponentPair(componentLookup, owner, name, options); + } + let component = componentLookup.componentFor(name, owner, options); let layout = componentLookup.layoutFor(name, owner, options); diff --git a/yarn.lock b/yarn.lock index 857fb39ebd3..9a7f1e05cb9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -838,9 +838,9 @@ backbone@^1.1.2: dependencies: underscore ">=1.8.3" -backburner.js@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/backburner.js/-/backburner.js-1.0.0.tgz#fffae139998f20a161ac2140d85639b152dfc226" +backburner.js@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/backburner.js/-/backburner.js-1.1.0.tgz#16ef021891bc330a2d021c63d8d68cb8611eb3e4" backo2@1.0.2: version "1.0.2"