[Module Unification] Introduce source to injections

FEATURES.md

@@ -43,9 +43,8 @@ for a detailed explanation.
   ([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`).
+  - Passing the `source` of a `lookup`/`factoryFor` call as an argument to
+    `expandLocalLookup` on the resolver.
   - Making `lookupComponentPair` friendly to local/private resolutions. The
     new code ensures a local resolution is not paired with a global resolution.
 

packages/container/lib/container.js

@@ -130,10 +130,6 @@ export default class Container {
     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
@@ -168,29 +164,7 @@ export default class Container {
       }
     }
 
-    let cacheKey = this._resolverCacheKey(normalizedName, options);
-    let cached = this.factoryManagerCache[cacheKey];
-
-    if (cached !== undefined) { return cached; }
-
-    let factory = EMBER_MODULE_UNIFICATION ? this.registry.resolve(normalizedName, options) : this.registry.resolve(normalizedName);
-
-    if (factory === undefined) {
-      return;
-    }
-
-    if (DEBUG && factory && typeof factory._onLookup === 'function') {
-      factory._onLookup(fullName);
-    }
-
-    let manager = new FactoryManager(this, factory, fullName, normalizedName);
-
-    if (DEBUG) {
-      manager = wrapManagerInDeprecationProxy(manager);
-    }
-
-    this.factoryManagerCache[cacheKey] = manager;
-    return manager;
+    return factoryFor(this, normalizedName, fullName);
   }
 }
 /*
@@ -232,6 +206,7 @@ function isInstantiatable(container, fullName) {
 }
 
 function lookup(container, fullName, options = {}) {
+  let normalizedName = fullName;
   if (options.source) {
     let expandedFullName = container.registry.expandLocalLookup(fullName, options);
 
@@ -241,24 +216,49 @@ function lookup(container, fullName, options = {}) {
         return;
       }
 
-      fullName = expandedFullName;
+      normalizedName = expandedFullName;
     } else if (expandedFullName) {
       // with ember-module-unification, if expandLocalLookup returns something,
       // pass it to the resolve without the source
-      fullName = expandedFullName;
+      normalizedName = expandedFullName;
       options = {};
     }
   }
 
   if (options.singleton !== false) {
-    let cacheKey = container._resolverCacheKey(fullName, options);
-    let cached = container.cache[cacheKey];
+    let cached = container.cache[normalizedName];
     if (cached !== undefined) {
       return cached;
     }
   }
 
-  return instantiateFactory(container, fullName, options);
+  return instantiateFactory(container, normalizedName, fullName, options);
+}
+
+
+function factoryFor(container, normalizedName, fullName) {
+  let cached = container.factoryManagerCache[normalizedName];
+
+  if (cached !== undefined) { return cached; }
+
+  let factory = container.registry.resolve(normalizedName);
+
+  if (factory === undefined) {
+    return;
+  }
+
+  if (DEBUG && factory && typeof factory._onLookup === 'function') {
+    factory._onLookup(fullName); // What should this pass? fullname or the normalized key?
+  }
+
+  let manager = new FactoryManager(container, factory, fullName, normalizedName);
+
+  if (DEBUG) {
+    manager = wrapManagerInDeprecationProxy(manager);
+  }
+
+  container.factoryManagerCache[normalizedName] = manager;
+  return manager;
 }
 
 function isSingletonClass(container, fullName, { instantiate, singleton }) {
@@ -277,8 +277,8 @@ function isFactoryInstance(container, fullName, { instantiate, singleton }) {
   return instantiate !== false && (singleton !== false || isSingleton(container, fullName)) && isInstantiatable(container, fullName);
 }
 
-function instantiateFactory(container, fullName, options) {
-  let factoryManager = EMBER_MODULE_UNIFICATION && options && options.source ? container.factoryFor(fullName, options) : container.factoryFor(fullName);
+function instantiateFactory(container, normalizedName, fullName, options) {
+  let factoryManager = factoryFor(container, normalizedName, fullName);
 
   if (factoryManager === undefined) {
     return;
@@ -287,8 +287,7 @@ function instantiateFactory(container, 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)) {
-    let cacheKey = container._resolverCacheKey(fullName, options);
-    return container.cache[cacheKey] = factoryManager.create();
+    return container.cache[normalizedName] = factoryManager.create();
   }
 
   // SomeClass { singleton: false, instantiate: true }
@@ -313,12 +312,15 @@ function buildInjections(container, injections) {
       container.registry.validateInjections(injections);
     }
 
-    let injection;
     for (let i = 0; i < injections.length; i++) {
-      injection = injections[i];
-      hash[injection.property] = lookup(container, injection.fullName);
+      let {property, specifier, source} = injections[i];
+      if (source) {
+        hash[property] = lookup(container, specifier, {source});
+      } else {
+        hash[property] = lookup(container, specifier);
+      }
       if (!isDynamic) {
-        isDynamic = !isSingleton(container, injection.fullName);
+        isDynamic = !isSingleton(container, specifier);
       }
     }
   }

packages/container/lib/registry.js

@@ -224,7 +224,6 @@ export default class Registry {
    @return {Function} fullName's factory
    */
   resolve(fullName, options) {
-    assert('fullName must be a proper full name', this.isValidFullName(fullName));
     let factory = resolve(this, this.normalize(fullName), options);
     if (factory === undefined && this.fallback !== null) {
       factory = this.fallback.resolve(...arguments);
@@ -450,7 +449,7 @@ export default class Registry {
     let injections = this._typeInjections[type] ||
                      (this._typeInjections[type] = []);
 
-    injections.push({ property, fullName });
+    injections.push({ property, specifier: fullName });
   }
 
   /**
@@ -514,7 +513,7 @@ export default class Registry {
     let injections = this._injections[normalizedName] ||
                      (this._injections[normalizedName] = []);
 
-    injections.push({ property, fullName: normalizedInjectionName });
+    injections.push({ property, specifier: normalizedInjectionName });
   }
 
   /**
@@ -566,14 +565,6 @@ export default class Registry {
     return injections;
   }
 
-  resolverCacheKey(name, options) {
-    if (!EMBER_MODULE_UNIFICATION) {
-      return name;
-    }
-
-    return (options && options.source) ? `${options.source}:${name}` : name;
-  }
-
   /**
    Given a fullName and a source fullName returns the fully resolved
    fullName. Used to allow for local lookup.
@@ -625,11 +616,13 @@ if (DEBUG) {
 
     for (let key in hash) {
       if (hash.hasOwnProperty(key)) {
-        assert(`Expected a proper full name, given '${hash[key]}'`, this.isValidFullName(hash[key]));
+        let { specifier, source } = hash[key];
+        assert(`Expected a proper full name, given '${specifier}'`, this.isValidFullName(specifier));
 
         injections.push({
           property: key,
-          fullName: hash[key]
+          specifier,
+          source
         });
       }
     }
@@ -640,12 +633,10 @@ if (DEBUG) {
   Registry.prototype.validateInjections = function(injections) {
     if (!injections) { return; }
 
-    let fullName;
-
     for (let i = 0; i < injections.length; i++) {
-      fullName = injections[i].fullName;
+      let {specifier, source} = injections[i];
 
-      assert(`Attempting to inject an unknown injection: '${fullName}'`, this.has(fullName));
+      assert(`Attempting to inject an unknown injection: '${specifier}'`, this.has(specifier, {source}));
     }
   };
 }
@@ -688,25 +679,24 @@ function resolve(registry, normalizedName, options) {
     }
   }
 
-  let cacheKey = registry.resolverCacheKey(normalizedName, options);
-  let cached = registry._resolveCache[cacheKey];
+  let cached = registry._resolveCache[normalizedName];
   if (cached !== undefined) { return cached; }
-  if (registry._failSet.has(cacheKey)) { return; }
+  if (registry._failSet.has(normalizedName)) { return; }
 
   let resolved;
 
   if (registry.resolver) {
-    resolved = registry.resolver.resolve(normalizedName, options && options.source);
+    resolved = registry.resolver.resolve(normalizedName);
   }
 
   if (resolved === undefined) {
     resolved = registry.registrations[normalizedName];
   }
 
   if (resolved === undefined) {
-    registry._failSet.add(cacheKey);
+    registry._failSet.add(normalizedName);
   } else {
-    registry._resolveCache[cacheKey] = resolved;
+    registry._resolveCache[normalizedName] = resolved;
   }
 
   return resolved;

packages/container/tests/container_test.js

@@ -388,15 +388,18 @@ moduleFor('Container', class extends AbstractTestCase {
     assert.deepEqual(resolveWasCalled, ['foo:post']);
   }
 
-  ['@test A factory\'s lazy injections are validated when first instantiated'](assert) {
+  [`@test A factory's lazy injections are validated when first instantiated`](assert) {
     let registry = new Registry();
     let container = registry.container();
     let Apple = factory();
     let Orange = factory();
 
     Apple.reopenClass({
       _lazyInjections() {
-        return ['orange:main', 'banana:main'];
+        return [
+          {specifier: 'orange:main'},
+          {specifier: 'banana:main'}
+        ];
       }
     });
 
@@ -419,7 +422,9 @@ moduleFor('Container', class extends AbstractTestCase {
     Apple.reopenClass({
       _lazyInjections: () => {
         assert.ok(true, 'should call lazy injection method');
-        return ['orange:main'];
+        return [
+          {specifier: 'orange:main'}
+        ];
       }
     });
 
@@ -637,18 +642,20 @@ moduleFor('Container', class extends AbstractTestCase {
 });
 
 if (EMBER_MODULE_UNIFICATION) {
-  QUnit.module('Container module unification');
-
   moduleFor('Container module unification', class extends AbstractTestCase {
-    ['@test The container can pass a source to factoryFor'](assert) {
+    ['@test The container can expand and resolve a source to factoryFor'](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 }) {
+      let expandedKey = 'boom, special expanded key';
+      registry.expandLocalLookup = function() {
+        return expandedKey;
+      };
+      registry.resolve = function(fullName) {
         resolveCount++;
-        if (fullName === lookup && source === expectedSource) {
+        if (fullName === expandedKey) {
           return PrivateComponent;
         }
       };
@@ -660,13 +667,17 @@ if (EMBER_MODULE_UNIFICATION) {
       assert.equal(resolveCount, 1, 'resolve called only once and a cached factory was returned the second time');
     }
 
-    ['@test The container can pass a source to lookup']() {
+    ['@test The container can expand and resolve a source to lookup']() {
       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) {
+      let expandedKey = 'boom, special expanded key';
+      registry.expandLocalLookup = function() {
+        return expandedKey;
+      };
+      registry.resolve = function(fullName) {
+        if (fullName === expandedKey) {
           return PrivateComponent;
         }
       };
@@ -676,7 +687,7 @@ if (EMBER_MODULE_UNIFICATION) {
       let result = container.lookup(lookup, { source: expectedSource });
       this.assert.ok(result instanceof PrivateComponent, 'The correct factory was provided');
 
-      this.assert.ok(container.cache[`template:routes/application:component:my-input`] instanceof PrivateComponent,
+      this.assert.ok(container.cache[expandedKey] instanceof PrivateComponent,
         'The correct factory was stored in the cache with the correct key which includes the source.');
     }
   });

packages/ember-application/tests/system/application_instance_test.js

@@ -121,6 +121,29 @@ moduleFor('ApplicationInstance', class extends TestCase {
     assert.notStrictEqual(postController1, postController2, 'lookup creates a brand new instance, because the previous one was reset');
   }
 
+  ['@skip unregistering a factory clears caches with source of that factory'](assert) {
+    assert.expect(1);
+
+    appInstance = run(() => ApplicationInstance.create({ application }));
+
+    let PostController1 = factory();
+    let PostController2 = factory();
+
+    appInstance.register('controller:post', PostController1);
+
+    appInstance.lookup('controller:post');
+    let postControllerLookupWithSource = appInstance.lookup('controller:post', {source: 'doesnt-even-matter'});
+
+    appInstance.unregister('controller:post');
+    appInstance.register('controller:post', PostController2);
+
+    // The cache that is source-specific is not cleared
+    assert.ok(
+      postControllerLookupWithSource !== appInstance.lookup('controller:post', {source: 'doesnt-even-matter'}),
+      'lookup with source creates a new instance'
+    );
+  }
+
   ['@test can build and boot a registered engine'](assert) {
     assert.expect(11);
 

packages/ember-application/tests/test-helpers/registry-check.js

@@ -18,7 +18,7 @@ export function verifyInjection(assert, owner, fullName, property, injectionName
 
   for (let i = 0, l = injections.length; i < l; i++) {
     injection = injections[i];
-    if (injection.property === property && injection.fullName === normalizedName) {
+    if (injection.property === property && injection.specifier === normalizedName) {
       hasInjection = true;
       break;
     }

packages/ember-glimmer/tests/integration/application/rendering-test.js

@@ -407,7 +407,7 @@ moduleFor('Application test: rendering', class extends ApplicationTest {
       template: 'Hi {{person.name}} from component'
     });
 
-    let expectedBacktrackingMessage = /modified "model\.name" twice on \[object Object\] in a single render\. It was rendered in "template:routeWithError" and modified in "component:x-foo"/;
+    let expectedBacktrackingMessage = /modified "model\.name" twice on \[object Object\] in a single render\. It was rendered in "template:my-app\/templates\/routeWithError.hbs" and modified in "component:x-foo"/;
 
     return this.visit('/').then(() => {
       expectAssertion(() => {