Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX release] Avoid instantiation errors when app/router.js injects the router service. #19405

Merged
merged 2 commits into from
Mar 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 0 additions & 37 deletions packages/@ember/-internals/routing/lib/services/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,12 @@ import { assert } from '@ember/debug';
import { readOnly } from '@ember/object/computed';
import { assign } from '@ember/polyfills';
import Service from '@ember/service';
import { DEBUG } from '@glimmer/env';
import { consumeTag, tagFor } from '@glimmer/validator';
import { Transition } from 'router_js';
import EmberRouter, { QueryParam } from '../system/router';
import { extractRouteArgs, resemblesURL, shallowEqual } from '../utils';

const ROUTER = symbol('ROUTER') as string;

let freezeRouteInfo: Function;
if (DEBUG) {
freezeRouteInfo = (transition: Transition) => {
if (transition.from !== null && !Object.isFrozen(transition.from)) {
Object.freeze(transition.from);
}

if (transition.to !== null && !Object.isFrozen(transition.to)) {
Object.freeze(transition.to);
}
};
}

function cleanURL(url: string, rootURL: string) {
if (rootURL === '/') {
return url;
Expand Down Expand Up @@ -73,31 +58,9 @@ export default class RouterService extends Service {
}
const owner = getOwner(this) as Owner;
router = owner.lookup('router:main') as EmberRouter;
router.setupRouter();
return (this[ROUTER] = router);
}

constructor(owner: Owner) {
super(owner);

const router = owner.lookup('router:main') as EmberRouter;

router.on('routeWillChange', (transition: Transition) => {
if (DEBUG) {
freezeRouteInfo(transition);
}
this.trigger('routeWillChange', transition);
});

router.on('routeDidChange', (transition: Transition) => {
if (DEBUG) {
freezeRouteInfo(transition);
}

this.trigger('routeDidChange', transition);
});
}

/**
Transition the application into another route. The route may
be either a single route or route path:
Expand Down
29 changes: 29 additions & 0 deletions packages/@ember/-internals/routing/lib/system/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { OutletState as GlimmerOutletState, OutletView } from '@ember/-internals
import { computed, get, notifyPropertyChange, set } from '@ember/-internals/metal';
import { FactoryClass, getOwner, Owner } from '@ember/-internals/owner';
import { BucketCache } from '@ember/-internals/routing';
import RouterService from '@ember/-internals/routing/lib/services/router';
import { A as emberA, Evented, Object as EmberObject, typeOf } from '@ember/-internals/runtime';
import Controller from '@ember/controller';
import { assert, deprecate, info } from '@ember/debug';
Expand All @@ -23,6 +24,7 @@ import Route, {
ROUTER_EVENT_DEPRECATIONS,
} from './route';
import RouterState from './router_state';

/**
@module @ember/routing
*/
Expand Down Expand Up @@ -78,6 +80,19 @@ function defaultWillTransition(
}
}

let freezeRouteInfo: Function;
if (DEBUG) {
freezeRouteInfo = (transition: Transition) => {
if (transition.from !== null && !Object.isFrozen(transition.from)) {
Object.freeze(transition.from);
}

if (transition.to !== null && !Object.isFrozen(transition.to)) {
Object.freeze(transition.to);
}
};
}

interface RenderOutletState {
name: string;
outlet: string;
Expand Down Expand Up @@ -146,6 +161,7 @@ class EmberRouter extends EmberObject {
_handledErrors = new Set();
_engineInstances: { [name: string]: { [id: string]: EngineInstance } } = Object.create(null);
_engineInfoByRoute = Object.create(null);
_routerService: RouterService | undefined;

_slowTransitionTimer: unknown;

Expand All @@ -156,6 +172,7 @@ class EmberRouter extends EmberObject {
if (owner) {
this.namespace = owner.lookup('application:main');
this._bucketCache = owner.lookup(P`-bucket-cache:main`);
this._routerService = owner.lookup('service:router');
}
}

Expand Down Expand Up @@ -286,6 +303,12 @@ class EmberRouter extends EmberObject {

routeWillChange(transition: Transition) {
router.trigger('routeWillChange', transition);
if (router._routerService) {
if (DEBUG) {
freezeRouteInfo(transition);
}
router._routerService.trigger('routeWillChange', transition);
}
// in case of intermediate transition we update the current route
// to make router.currentRoute.name consistent with router.currentRouteName
// see https://github.com/emberjs/ember.js/issues/19449
Expand All @@ -298,6 +321,12 @@ class EmberRouter extends EmberObject {
router.set('currentRoute', transition.to);
once(() => {
router.trigger('routeDidChange', transition);
if (router._routerService) {
if (DEBUG) {
freezeRouteInfo(transition);
}
router._routerService.trigger('routeDidChange', transition);
}
});
}

Expand Down
16 changes: 16 additions & 0 deletions packages/ember/tests/routing/router_service_test/basic_test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Route, NoneLocation } from '@ember/-internals/routing';
import { set } from '@ember/-internals/metal';
import { RouterTestCase, moduleFor } from 'internal-test-helpers';
import { inject as injectService } from '@ember/service';

moduleFor(
'Router Service - main',
Expand Down Expand Up @@ -150,5 +151,20 @@ moduleFor(
assert.ok(location instanceof NoneLocation);
});
}

['@test RouterService can be injected into router and accessed on init'](assert) {
assert.expect(1);

this.router.reopen({
routerService: injectService('router'),
init() {
this.routerService.one('routeDidChange', () => {
assert.ok(true, 'routeDidChange event listener called');
});
},
});

return this.visit('/');
}
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,36 @@ moduleFor(
assert.expect(5);
assert.equal(this.routerService.get('currentRouteName'), null);
assert.equal(this.routerService.get('currentURL'), null);
assert.equal(this.routerService.get('location'), 'none');
assert.equal(this.routerService.get('rootURL'), '/');
assert.equal(this.routerService.get('currentRoute'), null);
}

['@test RouterService properties of router can be accessed with default when router is present'](
assert
) {
assert.expect(5);
let router = this.owner.lookup('router:main');
router.setupRouter();
rwjblue marked this conversation as resolved.
Show resolved Hide resolved
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) {
let router = this.owner.lookup('router:main');
router.setupRouter();
assert.equal(this.routerService.urlFor('parent.child'), '/child');
}

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

let componentInstance;
let router = this.owner.lookup('router:main');
router.setupRouter();

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

Expand Down Expand Up @@ -90,6 +107,8 @@ moduleFor(
}

['@test RouterService#recognize recognize returns routeInfo'](assert) {
let router = this.owner.lookup('router:main');
router.setupRouter();
let routeInfo = this.routerService.recognize('/dynamic-with-child/123/1?a=b');
assert.ok(routeInfo);
let { name, localName, parent, child, params, queryParams, paramNames } = routeInfo;
Expand Down