Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX beta] Fix basic component and helper resolution in engines. #14135

Merged
merged 3 commits into from
Aug 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
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
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 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
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 @@ -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
Original file line number Diff line number Diff line change
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
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
17 changes: 14 additions & 3 deletions packages/ember-glimmer/lib/template.js
Original file line number Diff line number Diff line change
@@ -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));
}
};
}
Loading