Merge pull request #16565 from emberjs/simplify-cache

we don't need to build a composite key
emberjs · Apr 23, 2018 · c2de30a · c2de30a
2 parents 2cd2605 + b0ad54e
commit c2de30a
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 50 deletions.
diff --git a/packages/ember-glimmer/lib/resolver.ts b/packages/ember-glimmer/lib/resolver.ts
@@ -9,10 +9,9 @@ import {
 } from '@glimmer/interfaces';
 import { LazyCompiler, Macros, PartialDefinition } from '@glimmer/opcode-compiler';
 import { ComponentManager, getDynamicVar, Helper, ModifierManager } from '@glimmer/runtime';
-import { FACTORY_FOR, privatize as P } from 'container';
+import { privatize as P } from 'container';
 import { ENV } from 'ember-environment';
 import { LookupOptions, Owner, setOwner } from 'ember-owner';
-import { guidFor } from 'ember-utils';
 import { lookupComponent, lookupPartial, OwnedTemplateMeta } from 'ember-views';
 import CompileTimeLookup from './compile-time-lookup';
 import { CurlyComponentDefinition } from './component-managers/curly';
@@ -56,15 +55,6 @@ function makeOptions(moduleName: string, namespace?: string): LookupOptions {
   };
 }
 
-// returns the qualified / expanded name
-// which accounts for local lookup...
-function getNormalizedName(obj: any) {
-  let factoryManager = FACTORY_FOR.get(obj);
-  if (factoryManager) {
-    return factoryManager.normalizedName;
-  }
-}
-
 const BUILTINS_HELPERS = {
   if: inlineIf,
   action,
@@ -109,8 +99,8 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
   } = BUILTIN_MODIFIERS;
 
   // supports directly imported late bound layouts on component.prototype.layout
-  private templateCache: WeakMap<Owner, WeakMap<TemplateFactory, OwnedTemplate>> = new WeakMap();
-  private componentDefinitionCache: Map<string, ComponentDefinition | null> = new Map();
+  private templateCache: Map<Owner, Map<TemplateFactory, OwnedTemplate>> = new Map();
+  private componentDefinitionCache: Map<object, ComponentDefinition | null> = new Map();
 
   public templateCacheHits = 0;
   public templateCacheMisses = 0;
@@ -199,7 +189,7 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
   createTemplate(factory: TemplateFactory, owner: Owner): OwnedTemplate {
     let cache = this.templateCache.get(owner);
     if (cache === undefined) {
-      cache = new WeakMap();
+      cache = new Map();
       this.templateCache.set(owner, cache);
     }
     let template = cache.get(factory);
@@ -316,17 +306,20 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
       makeOptions(meta.moduleName, namespace)
     );
 
-    let ownerId = guidFor(meta.owner);
-    let componentDefinitionCacheKey =
-      ownerId + '|' + getNormalizedName(component) + '|' + getNormalizedName(layout);
-    let cachedComponentDefinition = this.componentDefinitionCache.get(componentDefinitionCacheKey);
+    let key = component === undefined ? layout : component;
+
+    if (key === undefined) {
+      return null;
+    }
+
+    let cachedComponentDefinition = this.componentDefinitionCache.get(key);
     if (cachedComponentDefinition !== undefined) {
       return cachedComponentDefinition;
     }
 
     if (layout && !component && ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) {
       let definition = new TemplateOnlyComponentDefinition(layout);
-      this.componentDefinitionCache.set(componentDefinitionCacheKey, definition);
+      this.componentDefinitionCache.set(key, definition);
       return definition;
     }
 
@@ -347,13 +340,13 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
             manager,
             component || meta.owner.factoryFor(P`component:-default`),
             null,
-            layout
+            layout! // TODO fix type
           )
         : null;
 
     finalizer();
 
-    this.componentDefinitionCache.set(componentDefinitionCacheKey, definition);
+    this.componentDefinitionCache.set(key, definition);
 
     return definition;
   }

diff --git a/packages/ember-views/index.d.ts b/packages/ember-views/index.d.ts
@@ -37,8 +37,8 @@ export function lookupComponent(
   name: string,
   options?: { source?: string }
 ): {
-  layout: Template<OwnedTemplateMeta>;
-  component: Factory<any, any>;
+  layout: Template<OwnedTemplateMeta> | undefined;
+  component: Factory<any, any> | undefined;
 };
 
 export function lookupPartial(templateName: string, owner: Owner): any;

diff --git a/packages/ember-views/lib/utils/lookup-component.js b/packages/ember-views/lib/utils/lookup-component.js
@@ -1,5 +1,3 @@
-import { privatize as P } from 'container';
-import { ENV } from 'ember-environment';
 import { EMBER_MODULE_UNIFICATION } from '@ember/canary-features';
 
 function lookupModuleUnificationComponentPair(componentLookup, owner, name, options) {
@@ -9,32 +7,26 @@ function lookupModuleUnificationComponentPair(componentLookup, owner, name, opti
   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.referrer.moduleName !== globalLayout.referrer.moduleName);
-
-  if (localAndUniqueComponent && localAndUniqueLayout) {
-    return { layout: localLayout, component: localComponent };
-  }
-
-  if (localAndUniqueComponent && !localAndUniqueLayout) {
-    return { layout: null, component: localComponent };
+  // TODO: we shouldn't have to recheck fallback, we should have a lookup that doesn't fallback
+  if (localComponent !== undefined) {
+    if (globalComponent !== undefined && globalComponent.class === localComponent.class) {
+      localComponent = undefined;
+    }
   }
-
-  let defaultComponentFactory = null;
-
-  if (!ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) {
-    defaultComponentFactory = owner.factoryFor(P`component:-default`);
+  if (localLayout !== undefined) {
+    if (
+      globalLayout !== undefined &&
+      localLayout.referrer.moduleName !== globalLayout.referrer.moduleName
+    ) {
+      localLayout = undefined;
+    }
   }
 
-  if (!localAndUniqueComponent && localAndUniqueLayout) {
-    return { layout: localLayout, component: defaultComponentFactory };
+  if (localLayout !== undefined || localComponent !== undefined) {
+    return { layout: localLayout, component: localComponent };
   }
 
-  let component = globalComponent || (globalLayout && defaultComponentFactory);
-  return { layout: globalLayout, component };
+  return { layout: globalLayout, component: globalComponent };
 }
 
 function lookupComponentPair(componentLookup, owner, name, options) {
@@ -47,10 +39,6 @@ function lookupComponentPair(componentLookup, owner, name, options) {
 
   let result = { layout, component };
 
-  if (!ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS && layout && !component) {
-    result.component = owner.factoryFor(P`component:-default`);
-  }
-
   return result;
 }