Introduce source to injections

Ember templates use a `source` argument to the container. This hints where to look for "local lookup", and also what namespace to look in for a partial specifier. For example a template in an addon's `src` tree can invoke components and helpers in that tree becuase it has a `source` argument passed to lookups. This introduces the same functionality for service injections. A service can now accept a `source` argumnet and its lookup will apply to the namespace of that file. As with templates, this is intended to be a build-time feature. We can run an AST transform on JS to add this `source` argumnet to any injections.
emberjs · Feb 19, 2018 · 3f89eb9 · 3f89eb9
1 parent b05e16d
commit 3f89eb9
Show file tree

Hide file tree

Showing 11 changed files with 220 additions and 35 deletions.
diff --git a/packages/container/lib/container.js b/packages/container/lib/container.js
@@ -313,12 +313,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);
       }
     }
   }
@@ -355,12 +358,13 @@ function resetCache(container) {
 }
 
 function resetMember(container, fullName) {
-  let member = container.cache[fullName];
+  let cacheKey = container._resolverCacheKey(fullName);
+  let member = container.cache[cacheKey];
 
-  delete container.factoryManagerCache[fullName];
+  delete container.factoryManagerCache[cacheKey];
 
   if (member) {
-    delete container.cache[fullName];
+    delete container.cache[cacheKey];
 
     if (member.destroy) {
       member.destroy();

diff --git a/packages/container/lib/registry.js b/packages/container/lib/registry.js
@@ -151,9 +151,10 @@ export default class Registry {
     assert(`Attempting to register an unknown factory: '${fullName}'`, factory !== undefined);
 
     let normalizedName = this.normalize(fullName);
-    assert(`Cannot re-register: '${fullName}', as it has already been resolved.`, !this._resolveCache[normalizedName]);
+    let cacheKey = this.resolverCacheKey(normalizedName);
+    assert(`Cannot re-register: '${fullName}', as it has already been resolved.`, !this._resolveCache[cacheKey]);
 
-    this._failSet.delete(normalizedName);
+    this._failSet.delete(cacheKey);
     this.registrations[normalizedName] = factory;
     this._options[normalizedName] = options;
   }
@@ -179,13 +180,14 @@ export default class Registry {
     assert('fullName must be a proper full name', this.isValidFullName(fullName));
 
     let normalizedName = this.normalize(fullName);
+    let cacheKey = this.resolverCacheKey(normalizedName);
 
     this._localLookupCache = Object.create(null);
 
     delete this.registrations[normalizedName];
-    delete this._resolveCache[normalizedName];
+    delete this._resolveCache[cacheKey];
     delete this._options[normalizedName];
-    this._failSet.delete(normalizedName);
+    this._failSet.delete(cacheKey);
   }
 
   /**
@@ -450,7 +452,7 @@ export default class Registry {
     let injections = this._typeInjections[type] ||
                      (this._typeInjections[type] = []);
 
-    injections.push({ property, fullName });
+    injections.push({ property, specifier: fullName });
   }
 
   /**
@@ -514,7 +516,7 @@ export default class Registry {
     let injections = this._injections[normalizedName] ||
                      (this._injections[normalizedName] = []);
 
-    injections.push({ property, fullName: normalizedInjectionName });
+    injections.push({ property, specifier: normalizedInjectionName });
   }
 
   /**
@@ -567,11 +569,7 @@ export default class Registry {
   }
 
   resolverCacheKey(name, options) {
-    if (!EMBER_MODULE_UNIFICATION) {
-      return name;
-    }
-
-    return (options && options.source) ? `${options.source}:${name}` : name;
+    return `${name}\0${(options && options.source) || ''}`;
   }
 
   /**
@@ -625,11 +623,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 +640,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}));
     }
   };
 }

diff --git a/packages/container/tests/container_test.js b/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'}
+        ];
       }
     });
 
@@ -676,7 +681,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[`component:my-input\0template:routes/application`] instanceof PrivateComponent,
         'The correct factory was stored in the cache with the correct key which includes the source.');
     }
   });

diff --git a/packages/ember-application/tests/system/application_instance_test.js b/packages/ember-application/tests/system/application_instance_test.js
@@ -120,6 +120,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);
+
+    let 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'
+    );
+  }
+
   ['can build and boot a registered engine'](assert) {
     assert.expect(11);
 

diff --git a/packages/ember-application/tests/test-helpers/registry-check.js b/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;
     }

diff --git a/packages/ember-glimmer/tests/integration/components/local-lookup-test.js b/packages/ember-glimmer/tests/integration/components/local-lookup-test.js
@@ -1,5 +1,7 @@
 import { moduleFor, RenderingTest } from '../../utils/test-case';
+import { compile } from 'ember-template-compiler';
 import { ModuleBasedTestResolver } from 'internal-test-helpers';
+import { moduleFor as applicationModuleFor, ApplicationTestCase } from 'internal-test-helpers';
 import { Component } from '../../utils/helpers';
 import { EMBER_MODULE_UNIFICATION } from 'ember/features';
 import { helper, Helper } from 'ember-glimmer';
@@ -302,3 +304,33 @@ if (EMBER_MODULE_UNIFICATION) {
     }
   });
 }
+
+if (EMBER_MODULE_UNIFICATION) {
+  applicationModuleFor('Components test: local lookup with resolution referrer (MU)', class extends ApplicationTestCase {
+    ['@test Ensure that the same specifier with two sources does not share a cache key'](assert) {
+      this.add({
+        specifier: 'template:components/x-not-shared',
+        source: 'template:x-top'
+      }, compile('child-x-not-shared'));
+
+      this.add({
+        specifier: 'template:components/x-top',
+        source: 'template:application'
+      }, compile('top-level-x-top ({{x-not-shared}})', { moduleName: 'x-top' }));
+
+      this.add({
+        specifier: 'template:components/x-not-shared',
+        source: 'template:application'
+      }, compile('top-level-x-not-shared'));
+
+      this.addTemplate('application', '{{x-not-shared}} {{x-top}} {{x-not-shared}} {{x-top}}');
+
+      return this.visit('/').then(() => {
+        assert.equal(
+          this.element.textContent,
+          'top-level-x-not-shared top-level-x-top (child-x-not-shared) top-level-x-not-shared top-level-x-top (child-x-not-shared)'
+        );
+      });
+    }
+  });
+}
diff --git a/packages/ember-metal/lib/injected_property.js b/packages/ember-metal/lib/injected_property.js
@@ -21,9 +21,10 @@ import { descriptorFor } from './meta';
          to the property's name
   @private
 */
-export default function InjectedProperty(type, name) {
+export default function InjectedProperty(type, name, options) {
   this.type = type;
   this.name = name;
+  this.source = options ? options.source : undefined;
 
   this._super$Constructor(injectedPropertyGet);
   AliasedPropertyPrototype.oneWay.call(this);
@@ -36,7 +37,12 @@ function injectedPropertyGet(keyName) {
   assert(`InjectedProperties should be defined with the inject computed property macros.`, desc && desc.type);
   assert(`Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container.`, owner);
 
-  return owner.lookup(`${desc.type}:${desc.name || keyName}`);
+  let specifier = `${desc.type}:${desc.name || keyName}`;
+  if (desc.source) {
+    return owner.lookup(specifier, {source: desc.source});
+  } else {
+    return owner.lookup(specifier);
+  }
 }
 
 InjectedProperty.prototype = Object.create(Descriptor.prototype);

diff --git a/packages/ember-runtime/lib/inject.js b/packages/ember-runtime/lib/inject.js
@@ -1,5 +1,7 @@
 import { InjectedProperty, descriptorFor } from 'ember-metal';
 import { assert } from 'ember-debug';
+import { EMBER_MODULE_UNIFICATION } from 'ember/features';
+
 /**
 @module ember
 */
@@ -35,7 +37,11 @@ const typeValidators = {};
 export function createInjectionHelper(type, validator) {
   typeValidators[type] = validator;
 
-  inject[type] = name => new InjectedProperty(type, name);
+  if (EMBER_MODULE_UNIFICATION) {
+    inject[type] = (name, options) => new InjectedProperty(type, name, options);
+  } else {
+    inject[type] = name => new InjectedProperty(type, name);
+  }
 }
 
 /**

diff --git a/packages/ember-runtime/lib/system/core_object.js b/packages/ember-runtime/lib/system/core_object.js
@@ -1046,7 +1046,10 @@ if (DEBUG) {
     for (key in proto) {
       desc = descriptorFor(proto, key);
       if (desc instanceof InjectedProperty) {
-        injections[key] = `${desc.type}:${desc.name || key}`;
+        injections[key] = {
+          specifier: `${desc.type}:${desc.name || key}`,
+          source: desc.source
+        };
       }
     }
 

diff --git a/packages/ember-runtime/tests/inject_test.js b/packages/ember-runtime/tests/inject_test.js
@@ -61,6 +61,11 @@ if (DEBUG) {
       bar: new InjectedProperty('quux')
     });
 
-    assert.deepEqual(AnObject._lazyInjections(), { 'foo': 'foo:bar', 'bar': 'quux:bar' }, 'should return injected container keys');
+    assert.deepEqual(
+      AnObject._lazyInjections(),
+      {
+        'foo': { specifier: 'foo:bar', source: undefined },
+        'bar': { specifier: 'quux:bar', source: undefined }
+      }, 'should return injected container keys');
   });
 }