Skip to content

Commit

Permalink
[BUGFIX release] fix router test regression in urlFor and recognize
Browse files Browse the repository at this point in the history
As part of the improvements made between 3.24 and 3.28, the router
microlib is now lazily loaded. When these changes were made, there were
a couple cases where it *should* be possible to access router state in
a non-application test (integration etc.) but it currently is not
because the router is not necessarily set up. Since `setupRouter` is
idempotent, call it in those functions so that if it is *not* set up,
it gets set up, and otherwise it will continue working as expected.

(cherry picked from commit 9f61c7a)
  • Loading branch information
chriskrycho authored and rwjblue committed Aug 30, 2021
1 parent f243a4f commit 4e097e6
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
3 changes: 3 additions & 0 deletions packages/@ember/-internals/routing/lib/services/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ export default class RouterService extends Service {
@public
*/
urlFor(routeName: string, ...args: any[]) {
this._router.setupRouter();
return this._router.generate(routeName, ...args);
}

Expand Down Expand Up @@ -375,6 +376,7 @@ export default class RouterService extends Service {
`You must pass a url that begins with the application's rootURL "${this.rootURL}"`,
url.indexOf(this.rootURL) === 0
);
this._router.setupRouter();
let internalURL = cleanURL(url, this.rootURL);
return this._router._routerMicrolib.recognize(internalURL);
}
Expand All @@ -395,6 +397,7 @@ export default class RouterService extends Service {
`You must pass a url that begins with the application's rootURL "${this.rootURL}"`,
url.indexOf(this.rootURL) === 0
);
this._router.setupRouter();
let internalURL = cleanURL(url, this.rootURL);
return this._router._routerMicrolib.recognizeAndLoad(internalURL);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,15 @@ moduleFor(
}

['@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);

// Callers who want to actually execute a transition in a non-application
// test are doing something weird and therefore should do
// `owner.setupRouter()` explicitly in their tests.
let componentInstance;
let router = this.owner.lookup('router:main');
router.setupRouter();
Expand Down Expand Up @@ -107,8 +108,6 @@ 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

0 comments on commit 4e097e6

Please sign in to comment.