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

With angular#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 angular#27680. Fixes angular#28200 PR Close angular#28300
vetom · Jan 31, 2019 · 40328a1 · 40328a1
1 parent 3ca5ac2
commit 40328a1
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 4 deletions.
diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
@@ -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;
                 }
               }),

diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
@@ -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
   ]