Skip to content

Commit

Permalink
Use the correct owner for each template lookup.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Robert Jackson committed Aug 27, 2016
1 parent 447df33 commit 02814c2
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 17 deletions.
3 changes: 3 additions & 0 deletions packages/ember-application/lib/system/engine-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -39,6 +40,8 @@ const EngineInstance = EmberObject.extend(RegistryProxy, ContainerProxy, {
init() {
this._super(...arguments);

guidFor(this);

let base = this.base;

if (!base) {
Expand Down
20 changes: 12 additions & 8 deletions packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { guidFor } from 'ember-metal/utils';
import lookupPartial, { hasPartial } from 'ember-views/system/lookup_partial';
import {
Environment as GlimmerEnvironment,
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions packages/ember-glimmer/lib/syntax/curly-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`;

Expand Down Expand Up @@ -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);
Expand Down
11 changes: 7 additions & 4 deletions packages/ember-glimmer/lib/template.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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));
}
};
}
Original file line number Diff line number Diff line change
@@ -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`
<h1>{{contextType}}</h1>
{{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`
<p>Component!</p>
`));
}
}));

return this.visit('/blog').then(() => {
this.assertText('ApplicationController Data!EngineComponent!');
});
}
});
}
2 changes: 1 addition & 1 deletion packages/ember-glimmer/tests/unit/layout-cache-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-glimmer/tests/unit/template-factory-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
4 changes: 4 additions & 0 deletions packages/ember-glimmer/tests/utils/abstract-test-case.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 02814c2

Please sign in to comment.