Skip to content

Commit

Permalink
[BUGFIX release] Avoid creating event dispatcher in FastBoot with Eng…
Browse files Browse the repository at this point in the history
…ines

Prior to this commit, the `event_dispatcher:main` was always being
instantiated so that it can be injected eagerly into child engines. In
general, this is good (we should only ever have a single event
dispatcher going) but in the FastBoot case the event dispatcher should
not have been created at all (since there is no DOM or interactivity
booting the event dispatcher to listen to DOM events is wasteful and
error prone).

This commit updates the EventDispatcher base class to properly look for
the `isInteractive` flag (which is what it should have been doing all
along) on the `-environment:main` that is injected from the
`Application.visit` API. When `isInteractive` is falsey and the event
dispatcher is instantiated we will issue an assertion.

We also avoid eagerly looking up (and therefore forcing to initialize)
the EventDispatcher if `.visit` API is passed `isInteractive: false`.
This avoids the specifically reported issue of triggering an assertion
in FastBoot mode.
  • Loading branch information
rwjblue committed Oct 3, 2017
1 parent ed381d7 commit 1121a37
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 3 deletions.
5 changes: 4 additions & 1 deletion packages/ember-application/lib/system/engine-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,12 @@ const EngineInstance = EmberObject.extend(RegistryProxyMixin, ContainerProxyMixi
'-view-registry:main',
`renderer:-${env.isInteractive ? 'dom' : 'inert'}`,
'service:-document',
'event_dispatcher:main'
];

if (env.isInteractive) {
singletons.push('event_dispatcher:main');
}

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

this.inject('view', '_environment', '-environment:main');
Expand Down
46 changes: 46 additions & 0 deletions packages/ember-application/tests/system/visit_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,52 @@ moduleFor('Ember.Application - visit()', class extends ApplicationTestCase {
});
}

[`@test visit() does not setup the event_dispatcher:main if isInteractive is false (with Engines) GH#15615`](assert) {
assert.expect(3);

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

this.addTemplate('application', '<h1>Hello world</h1>{{outlet}}');
this.add('event_dispatcher:main', {
create() { throw new Error('should not happen!'); }
});

// 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({}));
}
});
this.add('engine:blog', BlogEngine);

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

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

return this.visit('/blog', { isInteractive: false }).then(instance => {
assert.ok(
instance instanceof ApplicationInstance,
'promise is resolved with an ApplicationInstance'
);
assert.strictEqual(
this.$().find('p').text(), 'Dis cache money',
'Engine component is resolved'
);
});
}

[`@test visit() on engine resolves engine component`](assert) {
assert.expect(2);

Expand Down
8 changes: 6 additions & 2 deletions packages/ember-views/lib/system/event_dispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { deprecate } from 'ember-debug';
import { Object as EmberObject } from 'ember-runtime';
import jQuery from './jquery';
import ActionManager from './action_manager';
import { environment } from 'ember-environment';
import fallbackViewRegistry from '../compat/fallback-view-registry';

const ROOT_ELEMENT_CLASS = 'ember-application';
Expand Down Expand Up @@ -136,7 +135,12 @@ export default EmberObject.extend({

init() {
this._super();
assert('EventDispatcher should never be instantiated in fastboot mode. Please report this as an Ember bug.', environment.hasDOM);

assert('EventDispatcher should never be instantiated in fastboot mode. Please report this as an Ember bug.', (() => {
let owner = getOwner(this);
let environment = owner.lookup('-environment:main');
return environment.isInteractive;
})());

deprecate(
`\`canDispatchToEventManager\` has been deprecated in ${this}.`,
Expand Down

0 comments on commit 1121a37

Please sign in to comment.