Skip to content

Commit

Permalink
[BUGFIX beta] Ensure injections happen in engine instances.
Browse files Browse the repository at this point in the history
Prior to this `_environment` could not be relied upon in routes or
views (which is used when `shouldRender: false` is used with `visit`).

The refactor uses the environment to determine what injections to setup
in the engine-instance, and has the `application-instance` defer to the
`engine-instance` to avoid duplication.
  • Loading branch information
Robert Jackson committed Aug 27, 2016
1 parent e19a71d commit 3de2360
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 15 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
1 change: 0 additions & 1 deletion packages/ember-glimmer/lib/syntax/curly-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,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
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');
});
}
});
}
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

0 comments on commit 3de2360

Please sign in to comment.