From 32737a6bf7bfe69d6684fe5d184934990e1592a0 Mon Sep 17 00:00:00 2001
From: Jason Aden <jasonaden@google.com>
Date: Tue, 22 Jan 2019 14:04:55 -0800
Subject: [PATCH] fix(router): `skipLocationChange` with named outlets (#28301)

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 #28301
---
 packages/router/src/router.ts            | 13 +++++++----
 packages/router/test/integration.spec.ts | 28 ++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
index 1600fb9153..3261f1efec 100644
--- 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
index 9861880a1f..0cfd6a02d6 100644
--- a/packages/router/test/integration.spec.ts
+++ b/packages/router/test/integration.spec.ts
@@ -575,6 +575,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();
@@ -4890,6 +4911,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'); }
@@ -4941,6 +4966,7 @@ class LazyComponent {
     RootCmp,
     RelativeLinkInIfCmp,
     RootCmpWithTwoOutlets,
+    RootCmpWithNamedOutlet,
     EmptyQueryParamsCmp,
     ThrowingCmp
   ],
@@ -4971,6 +4997,7 @@ class LazyComponent {
     RootCmpWithOnInit,
     RelativeLinkInIfCmp,
     RootCmpWithTwoOutlets,
+    RootCmpWithNamedOutlet,
     EmptyQueryParamsCmp,
     ThrowingCmp
   ],
@@ -5002,6 +5029,7 @@ class LazyComponent {
     RootCmpWithOnInit,
     RelativeLinkInIfCmp,
     RootCmpWithTwoOutlets,
+    RootCmpWithNamedOutlet,
     EmptyQueryParamsCmp,
     ThrowingCmp
   ]