fix(router): `skipLocationChange` with named outlets (#28300)

With #27680, a bug was fixed where multiple redirects using `eager` URL update could cause navigation to fail. However, that fix introduced a problem where with `skipLocationChange` enabled, the URL tree rendered was not properly stored for reference. This specifically caused an issue with named router outlets and subsequent navigations not being recognized.

This PR stores the correct `UrlTree` for reference with later navigations. It fixes the regression introdued with #27680.

Fixes #28200

PR Close #28300
This commit is contained in:
Jason Aden 2019-01-22 14:04:55 -08:00 committed by Alex Rickabaugh
parent 33e49c2894
commit 50df897fdc
2 changed files with 37 additions and 4 deletions

View File

@ -505,8 +505,10 @@ export class Router {
// Update URL if in `eager` update mode
tap(t => {
if (this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange) {
this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id);
if (this.urlUpdateStrategy === 'eager') {
if (!t.extras.skipLocationChange) {
this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id);
}
this.browserUrlTree = t.urlAfterRedirects;
}
}),
@ -669,8 +671,11 @@ export class Router {
(this as{routerState: RouterState}).routerState = t.targetRouterState !;
if (this.urlUpdateStrategy === 'deferred' && !t.extras.skipLocationChange) {
this.setBrowserUrl(this.rawUrlTree, !!t.extras.replaceUrl, t.id, t.extras.state);
if (this.urlUpdateStrategy === 'deferred') {
if (!t.extras.skipLocationChange) {
this.setBrowserUrl(
this.rawUrlTree, !!t.extras.replaceUrl, t.id, t.extras.state);
}
this.browserUrlTree = t.urlAfterRedirects;
}
}),

View File

@ -571,6 +571,27 @@ describe('Integration', () => {
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
})));
it('should navigate after navigation with skipLocationChange',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = TestBed.createComponent(RootCmpWithNamedOutlet);
advance(fixture);
router.resetConfig([{path: 'show', outlet: 'main', component: SimpleCmp}]);
router.navigate([{outlets: {main: 'show'}}], {skipLocationChange: true});
advance(fixture);
expect(location.path()).toEqual('');
expect(fixture.nativeElement).toHaveText('main [simple]');
router.navigate([{outlets: {main: null}}], {skipLocationChange: true});
advance(fixture);
expect(location.path()).toEqual('');
expect(fixture.nativeElement).toHaveText('main []');
})));
describe('"eager" urlUpdateStrategy', () => {
beforeEach(() => {
const serializer = new DefaultUrlSerializer();
@ -4879,6 +4900,10 @@ class RootCmpWithOnInit {
class RootCmpWithTwoOutlets {
}
@Component({selector: 'root-cmp', template: `main [<router-outlet name="main"></router-outlet>]`})
class RootCmpWithNamedOutlet {
}
@Component({selector: 'throwing-cmp', template: ''})
class ThrowingCmp {
constructor() { throw new Error('Throwing Cmp'); }
@ -4930,6 +4955,7 @@ class LazyComponent {
RootCmp,
RelativeLinkInIfCmp,
RootCmpWithTwoOutlets,
RootCmpWithNamedOutlet,
EmptyQueryParamsCmp,
ThrowingCmp
],
@ -4960,6 +4986,7 @@ class LazyComponent {
RootCmpWithOnInit,
RelativeLinkInIfCmp,
RootCmpWithTwoOutlets,
RootCmpWithNamedOutlet,
EmptyQueryParamsCmp,
ThrowingCmp
],
@ -4991,6 +5018,7 @@ class LazyComponent {
RootCmpWithOnInit,
RelativeLinkInIfCmp,
RootCmpWithTwoOutlets,
RootCmpWithNamedOutlet,
EmptyQueryParamsCmp,
ThrowingCmp
]