From 02814c2da26a1558fb06eb527ef83e829dae7016 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 26 Aug 2016 18:54:27 -0400 Subject: [PATCH] Use the correct owner for each template lookup. Ensure that the owner being used is not the environments default `Owner`, but it is the owner that the template is being looked up by. For late bound templates this means the components owner, and for normal templates (looked up from registry) it is passed to the factories `.create` method by the container itself. --- .../lib/system/engine-instance.js | 3 + packages/ember-glimmer/lib/environment.js | 20 ++++--- .../lib/syntax/curly-component.js | 5 +- packages/ember-glimmer/lib/template.js | 11 ++-- .../integration/application/engine-test.js | 60 +++++++++++++++++++ .../tests/unit/layout-cache-test.js | 2 +- .../tests/unit/template-factory-test.js | 4 +- .../tests/utils/abstract-test-case.js | 4 ++ 8 files changed, 92 insertions(+), 17 deletions(-) create mode 100644 packages/ember-glimmer/tests/integration/application/engine-test.js diff --git a/packages/ember-application/lib/system/engine-instance.js b/packages/ember-application/lib/system/engine-instance.js index 37f8e6e22b5..6475f642650 100644 --- a/packages/ember-application/lib/system/engine-instance.js +++ b/packages/ember-application/lib/system/engine-instance.js @@ -13,6 +13,7 @@ import { getEngineParent, setEngineParent } from 'ember-application/system/engin import { assert } from 'ember-metal/debug'; import run from 'ember-metal/run_loop'; import RSVP from 'ember-runtime/ext/rsvp'; +import { guidFor } from 'ember-metal/utils'; import isEnabled from 'ember-metal/features'; /** @@ -39,6 +40,8 @@ const EngineInstance = EmberObject.extend(RegistryProxy, ContainerProxy, { init() { this._super(...arguments); + guidFor(this); + let base = this.base; if (!base) { diff --git a/packages/ember-glimmer/lib/environment.js b/packages/ember-glimmer/lib/environment.js index bb39fc564c9..0eefb4d8276 100644 --- a/packages/ember-glimmer/lib/environment.js +++ b/packages/ember-glimmer/lib/environment.js @@ -1,3 +1,4 @@ +import { guidFor } from 'ember-metal/utils'; import lookupPartial, { hasPartial } from 'ember-views/system/lookup_partial'; import { Environment as GlimmerEnvironment, @@ -140,12 +141,15 @@ export default class Environment extends GlimmerEnvironment { return new CurlyComponentDefinition(name, ComponentClass, layout); } }, ({ name, source, owner }) => { - return source && owner._resolveLocalLookupName(name, source) || name; + let expandedName = source && owner._resolveLocalLookupName(name, source) || name; + let ownerGuid = guidFor(owner); + + return ownerGuid + '|' + expandedName; }); - this._templateCache = new Cache(1000, Template => { - return Template.create({ env: this }); - }, template => template.id); + this._templateCache = new Cache(1000, ({ Template, owner }) => { + return Template.create({ env: this, [OWNER]: owner }); + }, ({ Template, owner }) => guidFor(owner) + '|' + Template.id); this._compilerCache = new Cache(10, Compiler => { return new Cache(2000, template => { @@ -276,14 +280,14 @@ export default class Environment extends GlimmerEnvironment { // normally templates should be exported at the proper module name // and cached in the container, but this cache supports templates // that have been set directly on the component's layout property - getTemplate(Template) { - return this._templateCache.get(Template); + getTemplate(Template, owner) { + return this._templateCache.get({ Template, owner }); } // a Compiler can wrap the template so it needs its own cache - getCompiledBlock(Compiler, template) { + getCompiledBlock(Compiler, template, owner) { let compilerCache = this._compilerCache.get(Compiler); - return compilerCache.get(template); + return compilerCache.get(template, owner); } hasPartial(name) { diff --git a/packages/ember-glimmer/lib/syntax/curly-component.js b/packages/ember-glimmer/lib/syntax/curly-component.js index fc7e7fb2968..44c905b5435 100644 --- a/packages/ember-glimmer/lib/syntax/curly-component.js +++ b/packages/ember-glimmer/lib/syntax/curly-component.js @@ -9,6 +9,7 @@ import get from 'ember-metal/property_get'; import { _instrumentStart } from 'ember-metal/instrumentation'; import { ComponentDefinition } from 'glimmer-runtime'; import Component from '../component'; +import { OWNER } from 'container/owner'; const DEFAULT_LAYOUT = P`template:components/-default`; @@ -230,10 +231,10 @@ class CurlyComponentManager { templateFor(component, env) { let Template = component.layout; + let owner = component[OWNER]; if (Template) { - return env.getTemplate(Template); + return env.getTemplate(Template, owner); } - let { owner } = env; let layoutName = get(component, 'layoutName'); if (layoutName) { let template = owner.lookup('template:' + layoutName); diff --git a/packages/ember-glimmer/lib/template.js b/packages/ember-glimmer/lib/template.js index b0e97e7dcf9..637a4ed6338 100644 --- a/packages/ember-glimmer/lib/template.js +++ b/packages/ember-glimmer/lib/template.js @@ -1,8 +1,8 @@ import { Template } from 'glimmer-runtime'; +import { OWNER } from 'container/owner'; class Wrapper { - constructor(id, env, spec) { - let { owner } = env; + constructor(id, env, owner, spec) { if (spec.meta) { spec.meta.owner = owner; } else { @@ -39,8 +39,11 @@ export default function template(json) { let id = ++templateId; return { id, - create({ env }) { - return new Wrapper(id, env, JSON.parse(json)); + create(options) { + let env = options.env; + let owner = options[OWNER]; + + return new Wrapper(id, env, owner, JSON.parse(json)); } }; } diff --git a/packages/ember-glimmer/tests/integration/application/engine-test.js b/packages/ember-glimmer/tests/integration/application/engine-test.js new file mode 100644 index 00000000000..c3bbcd40d03 --- /dev/null +++ b/packages/ember-glimmer/tests/integration/application/engine-test.js @@ -0,0 +1,60 @@ +import packageName from '../../utils/package-name'; +import { moduleFor, ApplicationTest } from '../../utils/test-case'; +import { strip } from '../../utils/abstract-test-case'; +import { compile } from '../../utils/helpers'; +import Controller from 'ember-runtime/controllers/controller'; +import Engine from 'ember-application/system/engine'; +import isEnabled from 'ember-metal/features'; + +// only run these tests for ember-glimmer when the feature is enabled, or for +// ember-htmlbars when the feature is not enabled +const shouldRun = isEnabled('ember-application-engines') && ( + ( + (isEnabled('ember-glimmer') && packageName === 'glimmer') || + (!isEnabled('ember-glimmer') && packageName === 'htmlbars') + ) +); + +if (shouldRun) { + moduleFor('Application test: engine rendering', class extends ApplicationTest { + ['@test sharing a template between engine and application has separate refinements']() { + this.assert.expect(1); + + let sharedTemplate = compile(strip` +

{{contextType}}

+ {{ambiguous-curlies}} + + {{outlet}} + `); + + this.application.register('template:application', sharedTemplate); + this.registerController('application', Controller.extend({ + contextType: 'Application', + 'ambiguous-curlies': 'Controller Data!' + })); + + this.router.map(function() { + this.mount('blog'); + }); + this.application.register('route-map:blog', function() { }); + + this.registerEngine('blog', Engine.extend({ + init() { + this._super(...arguments); + + this.register('controller:application', Controller.extend({ + contextType: 'Engine' + })); + this.register('template:application', sharedTemplate); + this.register('template:components/ambiguous-curlies', compile(strip` +

Component!

+ `)); + } + })); + + return this.visit('/blog').then(() => { + this.assertText('ApplicationController Data!EngineComponent!'); + }); + } + }); +} diff --git a/packages/ember-glimmer/tests/unit/layout-cache-test.js b/packages/ember-glimmer/tests/unit/layout-cache-test.js index 407f4ca6267..ef810751613 100644 --- a/packages/ember-glimmer/tests/unit/layout-cache-test.js +++ b/packages/ember-glimmer/tests/unit/layout-cache-test.js @@ -51,7 +51,7 @@ moduleFor('Layout cache test', class extends RenderingTest { templateFor(content) { let Factory = this.compile(content); - return this.env.getTemplate(Factory); + return this.env.getTemplate(Factory, this.owner); } ['@test each template is only compiled once'](assert) { diff --git a/packages/ember-glimmer/tests/unit/template-factory-test.js b/packages/ember-glimmer/tests/unit/template-factory-test.js index a836d5d78dc..0271df21d4c 100644 --- a/packages/ember-glimmer/tests/unit/template-factory-test.js +++ b/packages/ember-glimmer/tests/unit/template-factory-test.js @@ -28,12 +28,12 @@ moduleFor('Template factory test', class extends RenderingTest { assert.equal(env._templateCache.misses, 0, 'misses 0'); assert.equal(env._templateCache.hits, 0, 'hits 0'); - let precompiled = env.getTemplate(Precompiled); + let precompiled = env.getTemplate(Precompiled, env.owner); assert.equal(env._templateCache.misses, 1, 'misses 1'); assert.equal(env._templateCache.hits, 0, 'hits 0'); - let compiled = env.getTemplate(Compiled); + let compiled = env.getTemplate(Compiled, env.owner); assert.equal(env._templateCache.misses, 2, 'misses 2'); assert.equal(env._templateCache.hits, 0, 'hits 0'); diff --git a/packages/ember-glimmer/tests/utils/abstract-test-case.js b/packages/ember-glimmer/tests/utils/abstract-test-case.js index b7244372d68..346a559c7d8 100644 --- a/packages/ember-glimmer/tests/utils/abstract-test-case.js +++ b/packages/ember-glimmer/tests/utils/abstract-test-case.js @@ -307,6 +307,10 @@ export class AbstractApplicationTest extends TestCase { registerController(name, controller) { this.application.register(`controller:${name}`, controller); } + + registerEngine(name, engine) { + this.application.register(`engine:${name}`, engine); + } } export class AbstractRenderingTest extends TestCase {