From 584360364cf19b7bcff26dc775b6ba57148b1851 Mon Sep 17 00:00:00 2001 From: Thomas Wang Date: Sun, 3 Feb 2019 14:40:39 -0800 Subject: [PATCH] Lazy router setup in non-application tests 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. --- .../-internals/routing/lib/services/router.ts | 15 ++- .../-internals/routing/lib/system/router.ts | 13 ++- .../routing/tests/system/router_test.js | 14 +++ packages/@ember/application/instance.js | 12 +- .../@ember/application/lib/application.js | 3 +- .../non_application_test_test.js | 110 ++++++++++++++++++ 6 files changed, 152 insertions(+), 15 deletions(-) create mode 100644 packages/ember/tests/routing/router_service_test/non_application_test_test.js diff --git a/packages/@ember/-internals/routing/lib/services/router.ts b/packages/@ember/-internals/routing/lib/services/router.ts index 4079bb1f484..7b0e580f266 100644 --- a/packages/@ember/-internals/routing/lib/services/router.ts +++ b/packages/@ember/-internals/routing/lib/services/router.ts @@ -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'; @@ -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) => { @@ -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); diff --git a/packages/@ember/-internals/routing/lib/system/router.ts b/packages/@ember/-internals/routing/lib/system/router.ts index 55ebc2588ad..36715eea422 100644 --- a/packages/@ember/-internals/routing/lib/system/router.ts +++ b/packages/@ember/-internals/routing/lib/system/router.ts @@ -127,6 +127,7 @@ class EmberRouter extends EmberObject { location!: string | IEmberLocation; rootURL!: string; _routerMicrolib!: Router; + _didSetupRouter = false; currentURL: string | null = null; currentRouteName: string | null = null; @@ -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(); } @@ -403,6 +403,10 @@ class EmberRouter extends EmberObject { } setupRouter() { + if (this._didSetupRouter) { + return false; + } + this._didSetupRouter = true; this._setupLocation(); let location = get(this, 'location'); @@ -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); } @@ -599,6 +605,7 @@ class EmberRouter extends EmberObject { @method reset */ reset() { + this._didSetupRouter = false; if (this._routerMicrolib) { this._routerMicrolib.reset(); } diff --git a/packages/@ember/-internals/routing/tests/system/router_test.js b/packages/@ember/-internals/routing/tests/system/router_test.js index 719358796e9..8ffa765e2ca 100644 --- a/packages/@ember/-internals/routing/tests/system/router_test.js +++ b/packages/@ember/-internals/routing/tests/system/router_test.js @@ -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'); diff --git a/packages/@ember/application/instance.js b/packages/@ember/application/instance.js index 06fbd0e8f63..930306eb22f 100644 --- a/packages/@ember/application/instance.js +++ b/packages/@ember/application/instance.js @@ -159,7 +159,6 @@ const ApplicationInstance = EngineInstance.extend({ */ startRouting() { this.router.startRouting(); - this._didSetupRouter = true; }, /** @@ -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 ``'s that are rendered have the correct URL. - + @public */ setupRouter() { - if (this._didSetupRouter) { - return; - } - this._didSetupRouter = true; - this.router.setupRouter(); }, diff --git a/packages/@ember/application/lib/application.js b/packages/@ember/application/lib/application.js index 36ca0326acb..ed3fb7912e3 100644 --- a/packages/@ember/application/lib/application.js +++ b/packages/@ember/application/lib/application.js @@ -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); @@ -1167,7 +1167,6 @@ function commonSetupRegistry(registry) { }); registry.register('service:router', RouterService); - registry.injection('service:router', '_router', 'router:main'); } function registerLibraries() { diff --git a/packages/ember/tests/routing/router_service_test/non_application_test_test.js b/packages/ember/tests/routing/router_service_test/non_application_test_test.js new file mode 100644 index 00000000000..5705cd58760 --- /dev/null +++ b/packages/ember/tests/routing/router_service_test/non_application_test_test.js @@ -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']); + } + } +);