From 83cbc0e1a13a5f436021dd4601c0d38fc3dd4b74 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Thu, 30 May 2019 15:44:09 -0700 Subject: [PATCH] Refactor template factory Per RFC #481, we want to align the factory signature between templates and component managers. Since the factories for component managers are just a function that takes the owner, we decided to move template factories in the same direction here as well. Co-authored-by: Robert Jackson --- packages/@ember/-internals/glimmer/index.ts | 2 +- .../glimmer/lib/component-managers/curly.ts | 45 +++--- .../glimmer/lib/component-managers/mount.ts | 6 +- .../@ember/-internals/glimmer/lib/renderer.ts | 12 +- .../@ember/-internals/glimmer/lib/resolver.ts | 60 ++------ .../-internals/glimmer/lib/setup-registry.ts | 12 +- .../@ember/-internals/glimmer/lib/template.ts | 61 +++++---- .../-internals/glimmer/lib/views/outlet.ts | 3 +- .../glimmer/tests/integration/outlet-test.js | 26 ++-- .../tests/unit/runtime-resolver-cache-test.js | 129 +++++++----------- .../tests/unit/template-factory-test.js | 74 +++++++--- .../-internals/routing/lib/system/route.ts | 10 +- packages/@ember/-internals/views/index.d.ts | 5 +- .../application/tests/application_test.js | 1 - packages/@ember/engine/tests/engine_test.js | 1 - 15 files changed, 222 insertions(+), 225 deletions(-) diff --git a/packages/@ember/-internals/glimmer/index.ts b/packages/@ember/-internals/glimmer/index.ts index d5b5d99492b..96b5e93fa85 100644 --- a/packages/@ember/-internals/glimmer/index.ts +++ b/packages/@ember/-internals/glimmer/index.ts @@ -344,7 +344,7 @@ */ export { default as RootTemplate } from './lib/templates/root'; -export { default as template } from './lib/template'; +export { default as template, counters as templateCacheCounters, Factory as TemplateFactory, OwnedTemplate } from './lib/template'; export { default as Checkbox } from './lib/components/checkbox'; export { default as TextField } from './lib/components/text-field'; export { default as TextArea } from './lib/components/textarea'; diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts index 37177b06b2b..b7a2b5a3bd4 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts @@ -62,7 +62,7 @@ function aliasIdToElementId(args: Arguments, props: any) { } function isTemplateFactory(template: OwnedTemplate | TemplateFactory): template is TemplateFactory { - return typeof (template as TemplateFactory).create === 'function'; + return typeof template === 'function'; } // We must traverse the attributeBindings in reverse keeping track of @@ -119,33 +119,33 @@ export default class CurlyComponentManager }; } - templateFor(component: Component, resolver: RuntimeResolver): OwnedTemplate { - let { layout, layoutName } = component; + templateFor(component: Component): OwnedTemplate { + let { layout: _layout, layoutName } = component; let owner = getOwner(component); - if (layout !== undefined) { - // This needs to be cached by template.id - if (isTemplateFactory(layout)) { - return resolver.createTemplate(layout, getOwner(component)); - } else { - // we were provided an instance already - return layout; - } - } + let layout: TemplateFactory; - if (layoutName) { - let template = owner.lookup('template:' + layoutName); - if (template) { - return template; + if (_layout === undefined) { + if (layoutName !== undefined) { + layout = owner.lookup(`template:${layoutName}`); + assert(`Layout \`${layoutName}\` not found!`, layout !== undefined); + } else { + layout = owner.lookup(DEFAULT_LAYOUT); } + } else if (isTemplateFactory(_layout)) { + layout = _layout; + } else { + // we were provided an instance already + return _layout; } - return owner.lookup(DEFAULT_LAYOUT); + return layout(owner); } - getDynamicLayout({ component }: ComponentStateBucket, resolver: RuntimeResolver): Invocation { - const template = this.templateFor(component, resolver); - const layout = template.asWrappedLayout(); + getDynamicLayout({ component }: ComponentStateBucket): Invocation { + let template = this.templateFor(component); + let layout = template.asWrappedLayout(); + return { handle: layout.compile(), symbolTable: layout.symbolTable, @@ -153,7 +153,7 @@ export default class CurlyComponentManager } getTagName(state: ComponentStateBucket): Option { - const { component, hasWrappedElement } = state; + let { component, hasWrappedElement } = state; if (!hasWrappedElement) { return null; @@ -574,7 +574,6 @@ export const CURLY_CAPABILITIES: ComponentCapabilities = { const CURLY_COMPONENT_MANAGER = new CurlyComponentManager(); export class CurlyComponentDefinition implements ComponentDefinition { - public template: OwnedTemplate; public args: CurriedArgs | undefined; public state: DefinitionState; public symbolTable: ProgramSymbolTable | undefined; @@ -584,7 +583,7 @@ export class CurlyComponentDefinition implements ComponentDefinition { public name: string, public ComponentClass: any, public handle: Option, - template: OwnedTemplate, + public template?: OwnedTemplate, args?: CurriedArgs ) { const layout = template && template.asLayout(); diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/mount.ts b/packages/@ember/-internals/glimmer/lib/component-managers/mount.ts index 37d05308538..0d6e554993b 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/mount.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/mount.ts @@ -7,9 +7,9 @@ import { Destroyable, Opaque, Option } from '@glimmer/util'; import { Owner } from '@ember/-internals/owner'; import { generateControllerFactory } from '@ember/-internals/routing'; import { OwnedTemplateMeta } from '@ember/-internals/views'; +import { TemplateFactory } from '../..'; import Environment from '../environment'; import RuntimeResolver from '../resolver'; -import { OwnedTemplate } from '../template'; import { RootReference } from '../utils/references'; import AbstractManager from './abstract'; @@ -54,8 +54,10 @@ class MountManager implements WithDynamicLayout { getDynamicLayout(state: EngineState, _: RuntimeResolver): Invocation { - let template = state.engine.lookup('template:application') as OwnedTemplate; + let templateFactory = state.engine.lookup('template:application') as TemplateFactory; + let template = templateFactory(state.engine); let layout = template.asLayout(); + return { handle: layout.compile(), symbolTable: layout.symbolTable, diff --git a/packages/@ember/-internals/glimmer/lib/renderer.ts b/packages/@ember/-internals/glimmer/lib/renderer.ts index 8907264c87d..e58c95a2c14 100644 --- a/packages/@ember/-internals/glimmer/lib/renderer.ts +++ b/packages/@ember/-internals/glimmer/lib/renderer.ts @@ -23,7 +23,7 @@ import { BOUNDS } from './component'; import { createRootOutlet } from './component-managers/outlet'; import { RootComponentDefinition } from './component-managers/root'; import Environment from './environment'; -import { OwnedTemplate } from './template'; +import { Factory as TemplateFactory, OwnedTemplate } from './template'; import { Component } from './utils/curly-component-state-bucket'; import { OutletState } from './utils/outlet'; import { UnboundReference } from './utils/references'; @@ -246,7 +246,7 @@ interface ViewRegistry { export abstract class Renderer { private _env: Environment; - private _rootTemplate: any; + private _rootTemplate: OwnedTemplate; private _viewRegistry: ViewRegistry; private _destinedForDOM: boolean; private _destroyed: boolean; @@ -258,13 +258,13 @@ export abstract class Renderer { constructor( env: Environment, - rootTemplate: OwnedTemplate, + rootTemplate: TemplateFactory, viewRegistry: ViewRegistry, destinedForDOM = false, builder = clientBuilder ) { this._env = env; - this._rootTemplate = rootTemplate; + this._rootTemplate = rootTemplate(env.owner); this._viewRegistry = viewRegistry; this._destinedForDOM = destinedForDOM; this._destroyed = false; @@ -517,7 +517,7 @@ export class InertRenderer extends Renderer { builder, }: { env: Environment; - rootTemplate: OwnedTemplate; + rootTemplate: TemplateFactory; _viewRegistry: any; builder: any; }) { @@ -539,7 +539,7 @@ export class InteractiveRenderer extends Renderer { builder, }: { env: Environment; - rootTemplate: OwnedTemplate; + rootTemplate: TemplateFactory; _viewRegistry: any; builder: any; }) { diff --git a/packages/@ember/-internals/glimmer/lib/resolver.ts b/packages/@ember/-internals/glimmer/lib/resolver.ts index f05f221f3e2..dc44347c33e 100644 --- a/packages/@ember/-internals/glimmer/lib/resolver.ts +++ b/packages/@ember/-internals/glimmer/lib/resolver.ts @@ -1,6 +1,6 @@ import { privatize as P } from '@ember/-internals/container'; import { ENV } from '@ember/-internals/environment'; -import { LookupOptions, Owner, setOwner } from '@ember/-internals/owner'; +import { FactoryClass, LookupOptions, Owner } from '@ember/-internals/owner'; import { lookupComponent, lookupPartial, OwnedTemplateMeta } from '@ember/-internals/views'; import { EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS, @@ -49,7 +49,6 @@ import OnModifierManager from './modifiers/on'; import { populateMacros } from './syntax'; import { mountHelper } from './syntax/mount'; import { outletHelper } from './syntax/outlet'; -import { Factory as TemplateFactory, Injections, OwnedTemplate } from './template'; import { getModifierManager } from './utils/custom-modifier-manager'; import { getManager } from './utils/managers'; import { ClassBasedHelperReference, SimpleHelperReference } from './utils/references'; @@ -114,13 +113,9 @@ export default class RuntimeResolver implements IRuntimeResolver> = new Map(); private componentDefinitionCache: Map = new Map(); private customManagerCache: Map> = new Map(); - public templateCacheHits = 0; - public templateCacheMisses = 0; public componentDefinitionCount = 0; public helperDefinitionCount = 0; @@ -213,34 +208,6 @@ export default class RuntimeResolver implements IRuntimeResolver(`modifier:${name}`); if (modifier !== undefined) { let managerFactory = getModifierManager>(modifier.class); let manager = managerFactory!(owner); @@ -367,13 +331,13 @@ export default class RuntimeResolver implements IRuntimeResolver = null; if (layout !== undefined && component === undefined && ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) { - definition = new TemplateOnlyComponentDefinition(layout); + definition = new TemplateOnlyComponentDefinition(layout(owner)); } if (component !== undefined && component.class !== undefined) { let wrapper = getManager(component.class); - if (wrapper && wrapper.type === 'component') { + if (wrapper !== null && wrapper.type === 'component') { let { factory } = wrapper; if (wrapper.internal) { @@ -382,14 +346,18 @@ export default class RuntimeResolver implements IRuntimeResolver, component.class, - layout! + layout!(owner) ); } else { + if (layout === undefined) { + layout = owner.lookup(P`template:components/-default`); + } + definition = new CustomManagerDefinition( name, component, factory(owner) as ManagerDelegate, - layout || owner.lookup(P`template:components/-default`) + layout!(owner) ); } } @@ -400,7 +368,7 @@ export default class RuntimeResolver implements IRuntimeResolver; export type OwnedTemplate = Template; -export default function template(json: StaticTemplate): Factory { - return new FactoryWrapper(templateFactory(json)); +export function id(factory: Factory): string { + return factory.__id; } -export interface Injections { - compiler: LazyCompiler; - [key: string]: any; +export function meta(factory: Factory): StaticTemplateMeta { + return factory.__meta; } -export interface Factory { - id: string; - meta: StaticTemplateMeta; - create(injections: Injections): OwnedTemplate; -} +export let counters = { + cacheHit: 0, + cacheMiss: 0, +}; + +export default function template(json: StaticTemplate): Factory { + let glimmerFactory = templateFactory(json); + let cache = new WeakMap(); -class FactoryWrapper implements Factory { - public id: string; - public meta: StaticTemplateMeta; + let factory = ((owner: Owner) => { + let result = cache.get(owner); - constructor(public factory: TemplateFactory) { - this.id = factory.id; - this.meta = factory.meta; - } + if (result === undefined) { + counters.cacheMiss++; + let compiler: LazyCompiler = owner.lookup(P`template-compiler:main`); + result = glimmerFactory.create(compiler, { owner }); + cache.set(owner, result); + } else { + counters.cacheHit++; + } - create(injections: Injections): OwnedTemplate { - const owner = getOwner(injections); - return this.factory.create(injections.compiler, { owner }); - } + return result; + }) as Factory; + + factory.__id = glimmerFactory.id; + factory.__meta = glimmerFactory.meta; + + return factory; +} + +export interface Factory { + __id: string; + __meta: StaticTemplateMeta; + (owner: Owner): OwnedTemplate; } diff --git a/packages/@ember/-internals/glimmer/lib/views/outlet.ts b/packages/@ember/-internals/glimmer/lib/views/outlet.ts index e42a3b882d9..9af41ceaa54 100644 --- a/packages/@ember/-internals/glimmer/lib/views/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/views/outlet.ts @@ -34,8 +34,9 @@ export default class OutletView { } static create(options: any) { - let { _environment, renderer, template } = options; + let { _environment, renderer, template: templateFactory } = options; let owner = options[OWNER]; + let template = templateFactory(owner); return new OutletView(_environment, renderer, owner, template); } diff --git a/packages/@ember/-internals/glimmer/tests/integration/outlet-test.js b/packages/@ember/-internals/glimmer/tests/integration/outlet-test.js index 333decd71ee..49d15bb6d26 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/outlet-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/outlet-test.js @@ -62,7 +62,7 @@ moduleFor( outlet: 'main', name: 'application', controller: {}, - template: this.owner.lookup('template:application'), + template: this.owner.lookup('template:application')(this.owner), }, outlets: Object.create(null), }; @@ -81,7 +81,7 @@ moduleFor( outlet: 'main', name: 'index', controller: {}, - template: this.owner.lookup('template:index'), + template: this.owner.lookup('template:index')(this.owner), }, outlets: Object.create(null), }; @@ -100,7 +100,7 @@ moduleFor( outlet: 'main', name: 'application', controller: {}, - template: this.owner.lookup('template:application'), + template: this.owner.lookup('template:application')(this.owner), }, outlets: Object.create(null), }; @@ -121,7 +121,7 @@ moduleFor( outlet: 'main', name: 'index', controller: {}, - template: this.owner.lookup('template:index'), + template: this.owner.lookup('template:index')(this.owner), }, outlets: Object.create(null), }; @@ -140,7 +140,7 @@ moduleFor( outlet: 'main', name: 'application', controller: {}, - template: this.owner.lookup('template:application'), + template: this.owner.lookup('template:application')(this.owner), }, outlets: Object.create(null), }; @@ -161,7 +161,7 @@ moduleFor( outlet: 'main', name: 'special', controller: {}, - template: this.owner.lookup('template:special'), + template: this.owner.lookup('template:special')(this.owner), }, outlets: Object.create(null), }; @@ -180,7 +180,7 @@ moduleFor( outlet: 'main', name: 'application', controller: {}, - template: this.owner.lookup('template:application'), + template: this.owner.lookup('template:application')(this.owner), }, outlets: Object.create(null), }; @@ -201,7 +201,7 @@ moduleFor( outlet: 'main', name: 'special', controller: {}, - template: this.owner.lookup('template:special'), + template: this.owner.lookup('template:special')(this.owner), }, outlets: Object.create(null), }; @@ -221,7 +221,7 @@ moduleFor( outlet: 'main', name: 'application', controller, - template: this.owner.lookup('template:application'), + template: this.owner.lookup('template:application')(this.owner), }, outlets: Object.create(null), }; @@ -242,7 +242,7 @@ moduleFor( outlet: 'main', name: 'foo', controller: {}, - template: this.owner.lookup('template:foo'), + template: this.owner.lookup('template:foo')(this.owner), }, outlets: Object.create(null), }; @@ -255,7 +255,7 @@ moduleFor( outlet: 'main', name: 'bar', controller: {}, - template: this.owner.lookup('template:bar'), + template: this.owner.lookup('template:bar')(this.owner), }, outlets: Object.create(null), }; @@ -289,7 +289,7 @@ moduleFor( outlet: 'main', name: 'outer', controller: {}, - template: this.owner.lookup('template:outer'), + template: this.owner.lookup('template:outer')(this.owner), }, outlets: { main: { @@ -299,7 +299,7 @@ moduleFor( outlet: 'main', name: 'inner', controller: {}, - template: this.owner.lookup('template:inner'), + template: this.owner.lookup('template:inner')(this.owner), }, outlets: Object.create(null), }, diff --git a/packages/@ember/-internals/glimmer/tests/unit/runtime-resolver-cache-test.js b/packages/@ember/-internals/glimmer/tests/unit/runtime-resolver-cache-test.js index 628581258b8..6c156e40d4f 100644 --- a/packages/@ember/-internals/glimmer/tests/unit/runtime-resolver-cache-test.js +++ b/packages/@ember/-internals/glimmer/tests/unit/runtime-resolver-cache-test.js @@ -7,28 +7,18 @@ import { } from 'internal-test-helpers'; import { set } from '@ember/-internals/metal'; - +import { templateCacheCounters } from '@ember/-internals/glimmer'; import { Component } from '../utils/helpers'; moduleFor( 'ember-glimmer runtime resolver cache', class extends RenderingTestCase { - '@test a helper definition is only generated once'(assert) { + '@test a helper definition is only generated once'() { this.registerHelper('foo-bar', () => 'foo-bar helper!'); this.registerHelper('baz-qux', () => 'baz-qux helper!'); - // assert precondition - let state = this.getCacheCounters(); - assert.deepEqual( - state, - { - templateCacheHits: 0, - templateCacheMisses: 0, - componentDefinitionCount: 0, - helperDefinitionCount: 0, - }, - 'precondition' - ); + // snapshot counters + this.getCacheCounters(); this.render( ` @@ -43,9 +33,12 @@ moduleFor( ); this.assertText('foo-bar helper!'); - state = this.expectCacheChanges( - { helperDefinitionCount: 1 }, - state, + this.expectCacheChanges( + { + helperDefinitionCount: 1, + // from this.render + templateCacheMisses: 1, + }, 'calculate foo-bar helper only' ); @@ -53,11 +46,10 @@ moduleFor( runTask(() => set(this.context, 'cond', false)); this.assertText('baz-qux helper!'); - state = this.expectCacheChanges( + this.expectCacheChanges( { helperDefinitionCount: 1, }, - state, 'calculate baz-qux helper, misses cache' ); @@ -65,16 +57,16 @@ moduleFor( runTask(() => set(this.context, 'cond', true)); this.assertText('foo-bar helper!'); - state = this.expectCacheChanges({}, state, 'toggle back to foo-bar cache hit'); + this.expectCacheChanges({}, 'toggle back to foo-bar cache hit'); // show baz-qux again runTask(() => set(this.context, 'cond', false)); this.assertText('baz-qux helper!'); - state = this.expectCacheChanges({}, state, 'toggle back to baz-qux cache hit'); + this.expectCacheChanges({}, 'toggle back to baz-qux cache hit'); } - '@test a component definition is only generated once'(assert) { + '@test a component definition is only generated once'() { // static layout this.registerComponent('component-one', { template: 'One' }); this.registerComponent('component-two', { @@ -82,18 +74,8 @@ moduleFor( template: 'Two', }); - // assert precondition - let state = this.getCacheCounters(); - assert.deepEqual( - state, - { - templateCacheHits: 0, - templateCacheMisses: 0, - componentDefinitionCount: 0, - helperDefinitionCount: 0, - }, - 'precondition' - ); + // snapshot counters + this.getCacheCounters(); // show component-one for the first time this.render(`{{component componentName}}`, { @@ -101,9 +83,12 @@ moduleFor( }); this.assertText('One'); - state = this.expectCacheChanges( - { componentDefinitionCount: 1 }, - state, + this.expectCacheChanges( + { + componentDefinitionCount: 1, + // 1 from this.render, 1 from component-one + templateCacheMisses: 2, + }, 'test case component and component-one no change' ); @@ -111,9 +96,11 @@ moduleFor( runTask(() => set(this.context, 'componentName', 'component-two')); this.assertText('Two'); - state = this.expectCacheChanges( - { componentDefinitionCount: 1 }, - state, + this.expectCacheChanges( + { + componentDefinitionCount: 1, + templateCacheMisses: 1, + }, 'component-two first render' ); @@ -121,16 +108,16 @@ moduleFor( runTask(() => set(this.context, 'componentName', 'component-one')); this.assertText('One'); - state = this.expectCacheChanges({}, state, 'toggle back to component-one no change'); + this.expectCacheChanges({}, 'toggle back to component-one no change'); // show component-two again runTask(() => set(this.context, 'componentName', 'component-two')); this.assertText('Two'); - state = this.expectCacheChanges({}, state, 'toggle back to component-two no change'); + this.expectCacheChanges({}, 'toggle back to component-two no change'); } - ['@test each template is only compiled once'](assert) { + ['@test each template is only compiled once']() { // static layout this.registerComponent('component-one', { template: 'One' }); @@ -147,18 +134,8 @@ moduleFor( // template instance shared between to template managers let rootFactory = this.owner.factoryFor('component:root-component'); - // assert precondition - let state = this.getCacheCounters(); - assert.deepEqual( - state, - { - templateCacheHits: 0, - templateCacheMisses: 0, - componentDefinitionCount: 0, - helperDefinitionCount: 0, - }, - 'precondition' - ); + // snapshot counters + this.getCacheCounters(); // show component-one for the first time this.render( @@ -174,9 +151,11 @@ moduleFor( ); this.assertText('One'); - state = this.expectCacheChanges( - { componentDefinitionCount: 1 }, - state, + this.expectCacheChanges( + { + componentDefinitionCount: 1, + templateCacheMisses: 2, + }, 'test case component and component-one no change' ); @@ -184,12 +163,11 @@ moduleFor( runTask(() => set(this.context, 'cond', false)); this.assertText('Two'); - state = this.expectCacheChanges( + this.expectCacheChanges( { templateCacheMisses: 1, componentDefinitionCount: 1, }, - state, 'component-two first render misses template cache' ); @@ -197,17 +175,16 @@ moduleFor( runTask(() => set(this.context, 'cond', true)); this.assertText('One'); - state = this.expectCacheChanges({}, state, 'toggle back to component-one no change'); + this.expectCacheChanges({}, 'toggle back to component-one no change'); // show component-two again runTask(() => set(this.context, 'cond', false)); this.assertText('Two'); - state = this.expectCacheChanges( + this.expectCacheChanges( { templateCacheHits: 1, }, - state, 'toggle back to component-two hits template cache' ); @@ -217,14 +194,17 @@ moduleFor( runAppend(root); this.assertText('TwoOne'); // roots have different capabilities so this will hit - state = this.expectCacheChanges({}, state, 'append root with component-one no change'); + this.expectCacheChanges( + { templateCacheHits: 1 }, + 'append root with component-one no change' + ); // render new root append let root2 = rootFactory.create(); try { runAppend(root2); this.assertText('TwoOneOne'); - state = this.expectCacheChanges({}, state, 'append another root no change'); + this.expectCacheChanges({ templateCacheHits: 1 }, 'append another root no change'); } finally { runDestroy(root2); } @@ -234,26 +214,21 @@ moduleFor( } getCacheCounters() { - let { - templateCacheHits, - templateCacheMisses, - componentDefinitionCount, - helperDefinitionCount, - } = this.runtimeResolver; + let { componentDefinitionCount, helperDefinitionCount } = this.runtimeResolver; - return { - templateCacheHits, - templateCacheMisses, + return (this._counters = { + templateCacheHits: templateCacheCounters.cacheHit, + templateCacheMisses: templateCacheCounters.cacheMiss, componentDefinitionCount, helperDefinitionCount, - }; + }); } - expectCacheChanges(expected, lastState, message) { + expectCacheChanges(expected, message) { + let lastState = this._counters; let state = this.getCacheCounters(); let actual = diff(state, lastState); this.assert.deepEqual(actual, expected, message); - return state; } } ); diff --git a/packages/@ember/-internals/glimmer/tests/unit/template-factory-test.js b/packages/@ember/-internals/glimmer/tests/unit/template-factory-test.js index 2061982743f..bf5734c6fb9 100644 --- a/packages/@ember/-internals/glimmer/tests/unit/template-factory-test.js +++ b/packages/@ember/-internals/glimmer/tests/unit/template-factory-test.js @@ -1,6 +1,6 @@ import { RenderingTestCase, moduleFor } from 'internal-test-helpers'; -import { template } from '@ember/-internals/glimmer'; +import { template, templateCacheCounters } from '@ember/-internals/glimmer'; import { precompile, compile } from 'ember-template-compiler'; import { Component } from '../utils/helpers'; @@ -9,8 +9,10 @@ moduleFor( 'Template factory test', class extends RenderingTestCase { ['@test the template factory returned from precompile is the same as compile'](assert) { + // snapshot counters + this.getCacheCounters(); + let { owner } = this; - let { runtimeResolver } = this; let templateStr = 'Hello {{name}}'; let options = { moduleName: 'my-app/templates/some-module.hbs' }; @@ -21,27 +23,33 @@ moduleFor( let exports = {}; module(exports, template); let Precompiled = exports['default']; - let Compiled = compile(templateStr, options); - assert.equal(typeof Precompiled.create, 'function', 'precompiled is a factory'); - assert.ok(Precompiled.id, 'precompiled has id'); + assert.equal(typeof Precompiled, 'function', 'precompiled is a factory'); + assert.ok(Precompiled.__id, 'precompiled has id'); - assert.equal(typeof Compiled.create, 'function', 'compiled is a factory'); - assert.ok(Compiled.id, 'compiled has id'); + assert.equal(typeof Compiled, 'function', 'compiled is a factory'); + assert.ok(Compiled.__id, 'compiled has id'); - assert.equal(runtimeResolver.templateCacheMisses, 0, 'misses 0'); - assert.equal(runtimeResolver.templateCacheHits, 0, 'hits 0'); + this.expectCacheChanges({}, 'no changes'); - let precompiled = runtimeResolver.createTemplate(Precompiled, owner); + let precompiled = Precompiled(owner); - assert.equal(runtimeResolver.templateCacheMisses, 1, 'misses 1'); - assert.equal(runtimeResolver.templateCacheHits, 0, 'hits 0'); + this.expectCacheChanges( + { + templateCacheMisses: 1, + }, + 'misses 1' + ); - let compiled = runtimeResolver.createTemplate(Compiled, owner); + let compiled = Compiled(owner); - assert.equal(runtimeResolver.templateCacheMisses, 2, 'misses 2'); - assert.equal(runtimeResolver.templateCacheHits, 0, 'hits 0'); + this.expectCacheChanges( + { + templateCacheMisses: 1, + }, + 'misses 1' + ); assert.ok(typeof precompiled.spec !== 'string', 'Spec has been parsed'); assert.ok(typeof compiled.spec !== 'string', 'Spec has been parsed'); @@ -60,10 +68,40 @@ moduleFor( this.render('{{x-precompiled name="precompiled"}} {{x-compiled name="compiled"}}'); - assert.equal(runtimeResolver.templateCacheMisses, 2, 'misses 2'); - assert.equal(runtimeResolver.templateCacheHits, 2, 'hits 2'); - + this.expectCacheChanges( + { + templateCacheHits: 2, + // from this.render + templateCacheMisses: 1, + }, + 'hits 2' + ); this.assertText('Hello precompiled Hello compiled'); } + + getCacheCounters() { + return (this._counters = { + templateCacheHits: templateCacheCounters.cacheHit, + templateCacheMisses: templateCacheCounters.cacheMiss, + }); + } + + expectCacheChanges(expected, message) { + let lastState = this._counters; + let state = this.getCacheCounters(); + let actual = diff(state, lastState); + this.assert.deepEqual(actual, expected, message); + } } ); + +function diff(state, lastState) { + let res = {}; + Object.keys(state).forEach(key => { + let delta = state[key] - lastState[key]; + if (delta !== 0) { + res[key] = state[key] - lastState[key]; + } + }); + return res; +} diff --git a/packages/@ember/-internals/routing/lib/system/route.ts b/packages/@ember/-internals/routing/lib/system/route.ts index 30b114c9416..f151f5ecf58 100644 --- a/packages/@ember/-internals/routing/lib/system/route.ts +++ b/packages/@ember/-internals/routing/lib/system/route.ts @@ -1,3 +1,4 @@ +import { OwnedTemplate, TemplateFactory } from '@ember/-internals/glimmer'; import { computed, defineProperty, @@ -26,7 +27,6 @@ import { assign } from '@ember/polyfills'; import { once } from '@ember/runloop'; import { classify } from '@ember/string'; import { DEBUG } from '@glimmer/env'; -import { TemplateFactory } from '@glimmer/opcode-compiler'; import { InternalRouteInfo, PARAMS_SYMBOL, @@ -1817,7 +1817,7 @@ function buildRenderOptions( (controller! as any).set('model', model); } - let template: TemplateFactory = owner.lookup(`template:${templateName}`); + let template: TemplateFactory = owner.lookup(`template:${templateName}`); assert( `Could not find "${templateName}" template, view, or component.`, isDefaultRender || Boolean(template) @@ -1828,13 +1828,13 @@ function buildRenderOptions( into = undefined; } - let renderOptions = { + let renderOptions: RenderOptions = { owner, into, outlet, name, controller: controller! as any, - template: template || route._topLevelViewTemplate, + template: template !== undefined ? template(owner) : route._topLevelViewTemplate(owner), }; if (DEBUG) { @@ -1855,7 +1855,7 @@ export interface RenderOptions { outlet: string; name: string; controller: any; - template: TemplateFactory; + template: OwnedTemplate; } interface PartialRenderOptions { diff --git a/packages/@ember/-internals/views/index.d.ts b/packages/@ember/-internals/views/index.d.ts index 0c976f0d047..6630631fb99 100644 --- a/packages/@ember/-internals/views/index.d.ts +++ b/packages/@ember/-internals/views/index.d.ts @@ -1,6 +1,7 @@ import { Simple, Template, Option } from '@glimmer/interfaces'; import { Opaque } from '@glimmer/util'; import { Factory, Owner } from '@ember/-internals/owner'; +import { TemplateFactory } from '@ember/-internals/glimmer'; export interface StaticTemplateMeta { moduleName: string; @@ -41,11 +42,11 @@ export function lookupComponent( name: string, options?: { source?: string } ): { - layout: Template | undefined; + layout: TemplateFactory | undefined; component: Factory | undefined; }; -export function lookupPartial(templateName: string, owner: Owner): any; +export function lookupPartial(templateName: string, owner: Owner): TemplateFactory; export function getViewId(view: any): string; diff --git a/packages/@ember/application/tests/application_test.js b/packages/@ember/application/tests/application_test.js index 4aaabd8fbbe..14242a58787 100644 --- a/packages/@ember/application/tests/application_test.js +++ b/packages/@ember/application/tests/application_test.js @@ -207,7 +207,6 @@ moduleFor( verifyRegistration(assert, application, P`template:components/-default`); verifyRegistration(assert, application, 'template:-outlet'); verifyInjection(assert, application, 'view:-outlet', 'template', 'template:-outlet'); - verifyInjection(assert, application, 'template', 'compiler', P`template-compiler:main`); assert.deepEqual( application.registeredOptionsForType('helper'), diff --git a/packages/@ember/engine/tests/engine_test.js b/packages/@ember/engine/tests/engine_test.js index 31aa42fb01c..a21fb41949b 100644 --- a/packages/@ember/engine/tests/engine_test.js +++ b/packages/@ember/engine/tests/engine_test.js @@ -117,7 +117,6 @@ moduleFor( verifyRegistration(assert, engine, P`template:components/-default`); verifyRegistration(assert, engine, 'template:-outlet'); verifyInjection(assert, engine, 'view:-outlet', 'template', 'template:-outlet'); - verifyInjection(assert, engine, 'template', 'compiler', P`template-compiler:main`); assert.deepEqual( engine.registeredOptionsForType('helper'), { instantiate: false },