Skip to content

Commit

Permalink
Merge pull request #14140 from rwjblue/cleanup-engine-registry-setup
Browse files Browse the repository at this point in the history
[BUGFIX beta] Ensure injections happen in engine instances.
  • Loading branch information
rwjblue authored Aug 28, 2016
2 parents e19a71d + 831c8cb commit c5b10f1
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 23 deletions.
11 changes: 1 addition & 10 deletions packages/ember-application/lib/system/application-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,18 +283,9 @@ ApplicationInstance.reopenClass({
*/
setupRegistry(registry, options = new BootOptions()) {
registry.register('-environment:main', options.toEnvironment(), { instantiate: false });
registry.injection('view', '_environment', '-environment:main');
registry.injection('route', '_environment', '-environment:main');

registry.register('service:-document', options.document, { instantiate: false });

if (options.isInteractive) {
registry.injection('view', 'renderer', 'renderer:-dom');
registry.injection('component', 'renderer', 'renderer:-dom');
} else {
registry.injection('view', 'renderer', 'renderer:-inert');
registry.injection('component', 'renderer', 'renderer:-inert');
}
this._super(registry, options);
}
});

Expand Down
32 changes: 31 additions & 1 deletion packages/ember-application/lib/system/engine-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const EngineInstance = EmberObject.extend(RegistryProxy, ContainerProxy, {
@param options {Object}
@return {Promise<Ember.EngineInstance,Error>}
*/
boot(options = {}) {
boot(options) {
if (this._bootPromise) { return this._bootPromise; }

this._bootPromise = new RSVP.Promise(resolve => resolve(this._bootSync(options)));
Expand Down Expand Up @@ -105,13 +105,19 @@ const EngineInstance = EmberObject.extend(RegistryProxy, ContainerProxy, {
this.cloneParentDependencies();
}

this.setupRegistry(options);

this.base.runInstanceInitializers(this);

this._booted = true;

return this;
},

setupRegistry(options = this.__container__.lookup('-environment:main')) {
this.constructor.setupRegistry(this.__registry__, options);
},

/**
Unregister a factory.
Expand All @@ -136,6 +142,30 @@ const EngineInstance = EmberObject.extend(RegistryProxy, ContainerProxy, {
}
});

EngineInstance.reopenClass({
/**
@private
@method setupRegistry
@param {Registry} registry
@param {BootOptions} options
*/
setupRegistry(registry, options) {
// when no options/environment is present, do nothing
if (!options) { return; }

registry.injection('view', '_environment', '-environment:main');
registry.injection('route', '_environment', '-environment:main');

if (options.isInteractive) {
registry.injection('view', 'renderer', 'renderer:-dom');
registry.injection('component', 'renderer', 'renderer:-dom');
} else {
registry.injection('view', 'renderer', 'renderer:-inert');
registry.injection('component', 'renderer', 'renderer:-inert');
}
}
});

if (isEnabled('ember-application-engines')) {
EngineInstance.reopen({
/**
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export default class Environment extends GlimmerEnvironment {
let name = path[0];
let blockMeta = symbolTable.getMeta();
let owner = blockMeta.owner;
let source = `template:${blockMeta.moduleName}`;
let source = blockMeta.moduleName && `template:${blockMeta.moduleName}`;

return this._definitionCache.get({ name, source, owner });
}
Expand Down
2 changes: 2 additions & 0 deletions packages/ember-glimmer/lib/setup-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import TextField from './components/text_field';
import TextArea from './components/text_area';
import Checkbox from './components/checkbox';
import LinkToComponent from './components/link-to';
import Component from './component';
import ComponentTemplate from './templates/component';
import RootTemplate from './templates/root';
import OutletTemplate from './templates/outlet';
Expand Down Expand Up @@ -50,4 +51,5 @@ export function setupEngineRegistry(registry) {
registry.register('component:-text-area', TextArea);
registry.register('component:-checkbox', Checkbox);
registry.register('component:link-to', LinkToComponent);
registry.register(P`component:-default`, Component);
}
4 changes: 1 addition & 3 deletions packages/ember-glimmer/lib/syntax/curly-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import assign from 'ember-metal/assign';
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 @@ -188,7 +187,6 @@ class CurlyComponentManager {
aliasIdToElementId(args, props);

props.parentView = parentView;
props.renderer = parentView.renderer;
props[HAS_BLOCK] = hasBlock;

// dynamicScope here is inherited from the parent dynamicScope,
Expand Down Expand Up @@ -370,7 +368,7 @@ function ariaRole(vm) {

export class CurlyComponentDefinition extends ComponentDefinition {
constructor(name, ComponentClass, template, args) {
super(name, MANAGER, ComponentClass || Component);
super(name, MANAGER, ComponentClass);
this.template = template;
this.args = args;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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 Route from 'ember-routing/system/route';
import isEnabled from 'ember-metal/features';

// only run these tests for ember-glimmer when the feature is enabled, or for
Expand Down Expand Up @@ -56,5 +57,41 @@ if (shouldRun) {
this.assertText('ApplicationController Data!EngineComponent!');
});
}

['@test can use shouldRender: false'](assert) {
this.assert.expect(2);
let hooks = [];

this.registerRoute('application', Route.extend({
model() {
hooks.push('application - application');
}
}));

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('route:application', Route.extend({
model() {
hooks.push('engine - application');
}
}));
}
}));

return this.visit('/blog', { shouldRender: false }).then(() => {
this.assertText('');
this.assert.deepEqual(hooks, [
'application - application',
'engine - application'
], 'the expected model hooks were fired');
});
}
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ moduleFor('Components test: curly components', class extends RenderingTest {
this.assertComponentElement(this.firstChild, { content: 'hello' });
}

['@test it can render a template only component']() {
this.registerComponent('foo-bar', { template: 'hello' });

this.render('{{foo-bar}}');

this.assertComponentElement(this.firstChild, { content: 'hello' });

this.runTask(() => this.rerender());

this.assertComponentElement(this.firstChild, { content: 'hello' });
}

['@test it can have a custom id and it is not bound']() {
this.registerComponent('foo-bar', { template: '{{id}} {{elementId}}' });

Expand Down
6 changes: 3 additions & 3 deletions packages/ember-glimmer/tests/utils/abstract-test-case.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,13 @@ export class AbstractApplicationTest extends TestCase {
runDestroy(this.application);
}

visit(url) {
visit(url, options) {
let { applicationInstance } = this;

if (applicationInstance) {
return run(applicationInstance, 'visit', url);
return run(applicationInstance, 'visit', url, options);
} else {
return run(this.application, 'visit', url).then(instance => {
return run(this.application, 'visit', url, options).then(instance => {
this.applicationInstance = instance;
});
}
Expand Down
5 changes: 4 additions & 1 deletion packages/ember-htmlbars/lib/setup-registry.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { privatize as P } from 'container/registry';
import { InteractiveRenderer, InertRenderer } from 'ember-htmlbars/renderer';
import HTMLBarsDOMHelper from 'ember-htmlbars/system/dom-helper';
import topLevelViewTemplate from 'ember-htmlbars/templates/top-level-view';
import { OutletView as HTMLBarsOutletView } from 'ember-htmlbars/views/outlet';
import EmberView from 'ember-views/views/view';
import Component from 'ember-htmlbars/component';
import TextField from 'ember-htmlbars/components/text_field';
import TextArea from 'ember-htmlbars/components/text_area';
import Checkbox from 'ember-htmlbars/components/checkbox';
import LinkToComponent from 'ember-htmlbars/components/link-to';
import TemplateSupport from 'ember-views/mixins/template_support';


export function setupApplicationRegistry(registry) {
registry.register('renderer:-dom', InteractiveRenderer);
registry.register('renderer:-inert', InertRenderer);
Expand All @@ -32,4 +33,6 @@ export function setupEngineRegistry(registry) {
registry.register('component:-text-area', TextArea);
registry.register('component:-checkbox', Checkbox);
registry.register('component:link-to', LinkToComponent);

registry.register(P`component:-default`, Component);
}
14 changes: 10 additions & 4 deletions packages/ember-views/lib/utils/lookup-component.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { privatize as P } from 'container/registry';

function lookupComponentPair(componentLookup, owner, name, options) {
let component = componentLookup.componentFor(name, owner, options);
let layout = componentLookup.layoutFor(name, owner, options);
return {
component,
layout
};

let result = { layout, component };

if (layout && !component) {
result.component = owner._lookupFactory(P`component:-default`);
}

return result;
}

export default function lookupComponent(owner, name, options) {
Expand Down

0 comments on commit c5b10f1

Please sign in to comment.