Skip to content

Commit

Permalink
Merge pull request #14135 from emberjs/glimmer-owner
Browse files Browse the repository at this point in the history
[BUGFIX beta] Fix basic component and helper resolution in engines.
  • Loading branch information
rwjblue committed Aug 27, 2016
2 parents b769492 + 02814c2 commit e19a71d
Show file tree
Hide file tree
Showing 13 changed files with 234 additions and 49 deletions.
Expand Up @@ -484,6 +484,7 @@ BootOptions.prototype.toEnvironment = function() {
let env = assign({}, environment);
// For compatibility with existing code
env.hasDOM = this.isBrowser;
env.isInteractive = this.isInteractive;
env.options = this;
return env;
};
Expand Down
24 changes: 19 additions & 5 deletions packages/ember-application/lib/system/engine-instance.js
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 Expand Up @@ -170,18 +173,29 @@ if (isEnabled('ember-application-engines')) {
cloneParentDependencies() {
let parent = getEngineParent(this);

[
let registrations = [
'route:basic',
'event_dispatcher:main',
'service:-routing'
].forEach(key => this.register(key, parent.resolveRegistration(key)));
];

if (isEnabled('ember-glimmer')) {
registrations.push('service:-glimmer-environment');
}

[
registrations.forEach(key => this.register(key, parent.resolveRegistration(key)));

let env = parent.lookup('-environment:main');
this.register('-environment:main', env, { instantiate: false });

let singletons = [
'router:main',
P`-bucket-cache:main`,
'-view-registry:main',
'-environment:main'
].forEach(key => this.register(key, parent.lookup(key), { instantiate: false }));
`renderer:-${env.isInteractive ? 'dom' : 'inert'}`
];

singletons.forEach(key => this.register(key, parent.lookup(key), { instantiate: false }));

this.inject('view', '_environment', '-environment:main');
this.inject('route', '_environment', '-environment:main');
Expand Down
Expand Up @@ -141,7 +141,7 @@ QUnit.test('unregistering a factory clears all cached instances of that factory'

if (isEnabled('ember-application-engines')) {
QUnit.test('can build and boot a registered engine', function(assert) {
assert.expect(8);
assert.expect(isEnabled('ember-glimmer') ? 10 : 9);

let ChatEngine = Engine.extend();
let chatEngineInstance;
Expand All @@ -158,23 +158,34 @@ if (isEnabled('ember-application-engines')) {
.then(() => {
assert.ok(true, 'boot successful');

[
let registrations = [
'route:basic',
'event_dispatcher:main',
'service:-routing'
].forEach(key => {
];

if (isEnabled('ember-glimmer')) {
registrations.push('service:-glimmer-environment');
}

registrations.forEach(key => {
assert.strictEqual(
chatEngineInstance.resolveRegistration(key),
appInstance.resolveRegistration(key),
`Engine and parent app share registrations for '${key}'`);
});

[
let singletons = [
'router:main',
P`-bucket-cache:main`,
'-view-registry:main',
'-environment:main'
].forEach(key => {
];

let env = appInstance.lookup('-environment:main');
singletons.push(env.isInteractive ? 'renderer:-dom' : 'renderer:-inert');

singletons.forEach(key => {
assert.strictEqual(
chatEngineInstance.lookup(key),
appInstance.lookup(key),
Expand Down
70 changes: 70 additions & 0 deletions packages/ember-application/tests/system/visit_test.js
Expand Up @@ -9,6 +9,7 @@ import Route from 'ember-routing/system/route';
import Router from 'ember-routing/system/router';
import Component from 'ember-templates/component';
import { compile } from 'ember-template-compiler/tests/utils/helpers';
import { helper } from 'ember-templates/helper';
import jQuery from 'ember-views/system/jquery';

let App = null;
Expand Down Expand Up @@ -386,6 +387,75 @@ QUnit.test('visit() returns a promise that resolves without rendering when shoul
});
});

QUnit.test('visit() on engine resolves engine component', function(assert) {
assert.expect(2);

run(() => {
createApplication();

// Register engine
let BlogEngine = Engine.extend({
init(...args) {
this._super.apply(this, args);
this.register('template:application', compile('{{cache-money}}'));
this.register('template:components/cache-money', compile(`
<p>Dis cache money</p>
`));
this.register('component:cache-money', Component.extend({}));
}
});
App.register('engine:blog', BlogEngine);

// Register engine route map
let BlogMap = function() {};
App.register('route-map:blog', BlogMap);

App.Router.map(function() {
this.mount('blog');
});
});

assert.strictEqual(jQuery('#qunit-fixture').children().length, 0, 'there are no elements in the fixture element');

return run(App, 'visit', '/blog', { shouldRender: true }).then(instance => {
assert.strictEqual(jQuery('#qunit-fixture').find('p').text(), 'Dis cache money', 'Engine component is resolved');
});
});

QUnit.test('visit() on engine resolves engine helper', function(assert) {
assert.expect(2);

run(() => {
createApplication();

// Register engine
let BlogEngine = Engine.extend({
init(...args) {
this._super.apply(this, args);
this.register('template:application', compile('{{swag}}'));
this.register('helper:swag', helper(function() {
return 'turnt up';
}));
}
});
App.register('engine:blog', BlogEngine);

// Register engine route map
let BlogMap = function() {};
App.register('route-map:blog', BlogMap);

App.Router.map(function() {
this.mount('blog');
});
});

assert.strictEqual(jQuery('#qunit-fixture').children().length, 0, 'there are no elements in the fixture element');

return run(App, 'visit', '/blog', { shouldRender: true }).then(instance => {
assert.strictEqual(jQuery('#qunit-fixture').text(), 'turnt up', 'Engine component is resolved');
});
});

QUnit.module('Ember.Application - visit() Integration Tests', {
teardown() {
if (instances) {
Expand Down
58 changes: 36 additions & 22 deletions 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,
Expand Down Expand Up @@ -60,7 +61,7 @@ function buildTextFieldSyntax({ args, templates }, getDefinition) {
}

const builtInDynamicComponents = {
input({ key, args, templates }, getDefinition) {
input({ key, args, templates }, symbolTable, getDefinition) {
if (args.named.has('type')) {
let typeArg = args.named.at('type');
if (typeArg.type === 'value') {
Expand All @@ -79,7 +80,7 @@ const builtInDynamicComponents = {
} else {
return buildTextFieldSyntax({ args, templates }, getDefinition);
}
return DynamicComponentSyntax.create({ args, templates });
return DynamicComponentSyntax.create({ args, templates, symbolTable });
}
};

Expand Down Expand Up @@ -134,18 +135,21 @@ export default class Environment extends GlimmerEnvironment {

this.uselessAnchor = document.createElement('a');

this._definitionCache = new Cache(2000, ({ name, source }) => {
this._definitionCache = new Cache(2000, ({ name, source, owner }) => {
let { component: ComponentClass, layout } = lookupComponent(owner, name, { source });
if (ComponentClass || layout) {
return new CurlyComponentDefinition(name, ComponentClass, layout);
}
}, ({ name, source }) => {
return source && owner._resolveLocalLookupName(name, source) || name;
}, ({ name, source, owner }) => {
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 @@ -241,7 +245,7 @@ export default class Environment extends GlimmerEnvironment {

let generateBuiltInSyntax = builtInDynamicComponents[key];
if (generateBuiltInSyntax) {
return generateBuiltInSyntax(statement, (path) => this.getComponentDefinition([path], symbolTable));
return generateBuiltInSyntax(statement, symbolTable, (path) => this.getComponentDefinition([path], symbolTable));
}

assert(`A helper named "${key}" could not be found`, !isBlock || this.hasHelper(key, symbolTable));
Expand All @@ -266,21 +270,24 @@ export default class Environment extends GlimmerEnvironment {

getComponentDefinition(path, symbolTable) {
let name = path[0];
let source = symbolTable && `template:${symbolTable.getMeta().moduleName}`;
return this._definitionCache.get({ name, source });
let blockMeta = symbolTable.getMeta();
let owner = blockMeta.owner;
let source = `template:${blockMeta.moduleName}`;

return this._definitionCache.get({ name, source, owner });
}

// 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 All @@ -300,20 +307,27 @@ export default class Environment extends GlimmerEnvironment {
}

hasHelper(name, symbolTable) {
let options = symbolTable && { source: `template:${symbolTable.getMeta().moduleName}` } || {};
let blockMeta = symbolTable.getMeta();
let owner = blockMeta.owner;
let options = { source: `template:${blockMeta.moduleName}` };

return !!builtInHelpers[name[0]] ||
this.owner.hasRegistration(`helper:${name}`, options) ||
this.owner.hasRegistration(`helper:${name}`);
owner.hasRegistration(`helper:${name}`, options) ||
owner.hasRegistration(`helper:${name}`);
}

lookupHelper(name, symbolTable) {
let options = symbolTable && { source: `template:${symbolTable.getMeta().moduleName}` } || {};
let blockMeta = symbolTable.getMeta();
let owner = blockMeta.owner;
let options = blockMeta.moduleName && { source: `template:${blockMeta.moduleName}` } || {};

let helper = builtInHelpers[name[0]] ||
this.owner.lookup(`helper:${name}`, options) ||
this.owner.lookup(`helper:${name}`);
owner.lookup(`helper:${name}`, options) ||
owner.lookup(`helper:${name}`);

// TODO: try to unify this into a consistent protocol to avoid wasteful closure allocations
if (helper.isInternalHelper) {
return (vm, args) => helper.toReference(args, this);
return (vm, args) => helper.toReference(args, this, symbolTable);
} else if (helper.isHelperInstance) {
return (vm, args) => SimpleHelperReference.create(helper.compute, args);
} else if (helper.isHelperFactory) {
Expand Down
5 changes: 2 additions & 3 deletions packages/ember-glimmer/lib/helpers/component.js
Expand Up @@ -126,8 +126,7 @@ function curryArgs(definition, newArgs) {
export default {
isInternalHelper: true,

toReference(args, env) {
// TODO: Need to figure out what to do about symbolTable here.
return ClosureComponentReference.create(args, null, env);
toReference(args, env, symbolTable) {
return ClosureComponentReference.create(args, symbolTable, env);
}
};
5 changes: 3 additions & 2 deletions packages/ember-glimmer/lib/syntax/curly-component.js
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
17 changes: 14 additions & 3 deletions packages/ember-glimmer/lib/template.js
@@ -1,7 +1,15 @@
import { Template } from 'glimmer-runtime';
import { OWNER } from 'container/owner';

class Wrapper {
constructor(id, env, spec) {
constructor(id, env, owner, spec) {
if (spec.meta) {
spec.meta.owner = owner;
} else {
spec.meta = {
owner
};
}
this.id = id;
this.env = env;
this.spec = spec;
Expand Down Expand Up @@ -31,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));
}
};
}

0 comments on commit e19a71d

Please sign in to comment.