Skip to content

Commit

Permalink
Lazy router setup in non-application tests
Browse files Browse the repository at this point in the history
During non setupApplicationTests type tests, the Router being injected
into service:router and service:routing does not setup automatically,
during which it initializes its _routerMicrolib, etc. Public API on
service:router will throw in those tests.  This commit will trigger
setupRouter when accessing EmberRouter via router service to avoid those
errors.
  • Loading branch information
xg-wang committed Aug 9, 2020
1 parent 4b1ffe2 commit 5843603
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 15 deletions.
15 changes: 14 additions & 1 deletion packages/@ember/-internals/routing/lib/services/router.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { getOwner } from '@ember/-internals/owner';
import { Evented } from '@ember/-internals/runtime';
import { symbol } from '@ember/-internals/utils';
import { assert } from '@ember/debug';
import { readOnly } from '@ember/object/computed';
import { assign } from '@ember/polyfills';
Expand All @@ -8,6 +10,8 @@ import { Transition } from 'router_js';
import EmberRouter, { QueryParam } from '../system/router';
import { extractRouteArgs, resemblesURL, shallowEqual } from '../utils';

const ROUTER = symbol('ROUTER');

let freezeRouteInfo: Function;
if (DEBUG) {
freezeRouteInfo = (transition: Transition) => {
Expand Down Expand Up @@ -61,7 +65,16 @@ function cleanURL(url: string, rootURL: string) {
@class RouterService
*/
export default class RouterService extends Service {
_router!: EmberRouter;
get _router(): EmberRouter {
let router = this[ROUTER];
if (router !== undefined) {
return router;
}
const owner = getOwner(this) as any;
router = owner.lookup('router:main');
router.setupRouter();
return (this[ROUTER] = router);
}

init() {
super.init(...arguments);
Expand Down
13 changes: 10 additions & 3 deletions packages/@ember/-internals/routing/lib/system/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ class EmberRouter extends EmberObject {
location!: string | IEmberLocation;
rootURL!: string;
_routerMicrolib!: Router<Route>;
_didSetupRouter = false;

currentURL: string | null = null;
currentRouteName: string | null = null;
Expand Down Expand Up @@ -389,9 +390,8 @@ class EmberRouter extends EmberObject {
@private
*/
startRouting() {
let initialURL = get(this, 'initialURL');

if (this.setupRouter()) {
let initialURL = get(this, 'initialURL');
if (initialURL === undefined) {
initialURL = get(this, 'location').getURL();
}
Expand All @@ -403,6 +403,10 @@ class EmberRouter extends EmberObject {
}

setupRouter() {
if (this._didSetupRouter) {
return false;
}
this._didSetupRouter = true;
this._setupLocation();

let location = get(this, 'location');
Expand Down Expand Up @@ -472,7 +476,9 @@ class EmberRouter extends EmberObject {
this._toplevelView = OutletView.create();
this._toplevelView.setOutletState(liveRoutes as GlimmerOutletState);
let instance: any = owner.lookup('-application-instance:main');
instance.didCreateRootView(this._toplevelView);
if (instance) {
instance.didCreateRootView(this._toplevelView);
}
} else {
this._toplevelView.setOutletState(liveRoutes as GlimmerOutletState);
}
Expand Down Expand Up @@ -599,6 +605,7 @@ class EmberRouter extends EmberObject {
@method reset
*/
reset() {
this._didSetupRouter = false;
if (this._routerMicrolib) {
this._routerMicrolib.reset();
}
Expand Down
14 changes: 14 additions & 0 deletions packages/@ember/-internals/routing/tests/system/router_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,20 @@ moduleFor(
assert.ok(!router._routerMicrolib);
}

['@test should create a router.js instance after setupRouter'](assert) {
let router = createRouter(undefined, { disableSetup: false });

assert.ok(router._didSetupRouter);
assert.ok(router._routerMicrolib);
}

['@test should return false if setupRouter is called multiple times'](assert) {
let router = createRouter(undefined, { disableSetup: true });

assert.ok(router.setupRouter());
assert.notOk(router.setupRouter());
}

['@test should not reify location until setupRouter is called'](assert) {
let router = createRouter(undefined, { disableSetup: true });
assert.equal(typeof router.location, 'string', 'location is specified as a string');
Expand Down
12 changes: 3 additions & 9 deletions packages/@ember/application/instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ const ApplicationInstance = EngineInstance.extend({
*/
startRouting() {
this.router.startRouting();
this._didSetupRouter = true;
},

/**
Expand All @@ -168,23 +167,18 @@ const ApplicationInstance = EngineInstance.extend({
Because setup should only occur once, multiple calls to `setupRouter`
beyond the first call have no effect.
This is commonly used in order to confirm things that rely on the router
are functioning properly from tests that are primarily rendering related.
For example, from within [ember-qunit](https://github.com/emberjs/ember-qunit)'s
`setupRenderingTest` calling `this.owner.setupRouter()` would allow that
rendering test to confirm that any `<LinkTo></LinkTo>`'s that are rendered
have the correct URL.
@public
*/
setupRouter() {
if (this._didSetupRouter) {
return;
}
this._didSetupRouter = true;

this.router.setupRouter();
},

Expand Down
3 changes: 1 addition & 2 deletions packages/@ember/application/lib/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@ Application.reopenClass({
});

function commonSetupRegistry(registry) {
registry.register('router:main', Router.extend());
registry.register('router:main', Router);
registry.register('-view-registry:main', {
create() {
return dictionary(null);
Expand All @@ -1167,7 +1167,6 @@ function commonSetupRegistry(registry) {
});

registry.register('service:router', RouterService);
registry.injection('service:router', '_router', 'router:main');
}

function registerLibraries() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { inject as injectService } from '@ember/service';
import { Router, NoneLocation } from '@ember/-internals/routing';
import { get } from '@ember/-internals/metal';
import { run } from '@ember/runloop';
import { Component } from '@ember/-internals/glimmer';
import { RenderingTestCase, moduleFor } from 'internal-test-helpers';

moduleFor(
'Router Service - non application test',
class extends RenderingTestCase {
constructor() {
super(...arguments);

this.resolver.add('router:main', Router.extend(this.routerOptions));
this.router.map(function() {
this.route('parent', { path: '/' }, function() {
this.route('child');
this.route('sister');
this.route('brother');
});
this.route('dynamic', { path: '/dynamic/:dynamic_id' });
this.route('dynamicWithChild', { path: '/dynamic-with-child/:dynamic_id' }, function() {
this.route('child', { path: '/:child_id' });
});
});
}

get routerOptions() {
return {
location: 'none',
};
}

get router() {
return this.owner.resolveRegistration('router:main');
}

get routerService() {
return this.owner.lookup('service:router');
}

afterEach() {
super.afterEach();
}

['@test RouterService can be instantiated in non application test'](assert) {
assert.ok(this.routerService);
}

['@test RouterService properties can be accessed with default'](assert) {
assert.expect(5);
assert.equal(this.routerService.get('currentRouteName'), null);
assert.equal(this.routerService.get('currentURL'), null);
assert.ok(this.routerService.get('location') instanceof NoneLocation);
assert.equal(this.routerService.get('rootURL'), '/');
assert.equal(this.routerService.get('currentRoute'), null);
}

['@test RouterService#urlFor returns url'](assert) {
assert.equal(this.routerService.urlFor('parent.child'), '/child');
}

['@test RouterService#transitionTo with basic route'](assert) {
assert.expect(2);

let componentInstance;

this.addTemplate('parent.index', '{{foo-bar}}');

this.addComponent('foo-bar', {
ComponentClass: Component.extend({
routerService: injectService('router'),
init() {
this._super(...arguments);
componentInstance = this;
},
actions: {
transitionToSister() {
get(this, 'routerService').transitionTo('parent.sister');
},
},
}),
template: `foo-bar`,
});

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

run(function() {
componentInstance.send('transitionToSister');
});

assert.equal(this.routerService.get('currentRouteName'), 'parent.sister');
assert.ok(this.routerService.isActive('parent.sister'));
}

['@test RouterService#recognize recognize returns routeInfo'](assert) {
let routeInfo = this.routerService.recognize('/dynamic-with-child/123/1?a=b');
assert.ok(routeInfo);
let { name, localName, parent, child, params, queryParams, paramNames } = routeInfo;
assert.equal(name, 'dynamicWithChild.child');
assert.equal(localName, 'child');
assert.ok(parent);
assert.equal(parent.name, 'dynamicWithChild');
assert.notOk(child);
assert.deepEqual(params, { child_id: '1' });
assert.deepEqual(queryParams, { a: 'b' });
assert.deepEqual(paramNames, ['child_id']);
}
}
);

0 comments on commit 5843603

Please sign in to comment.