refactor(router): make sure redirect within NavigationStart event works (#25740)

PR Close #25740

packages/router/src/router.ts

@@ -370,11 +370,15 @@ export class Router {
     this.processNavigations();
   }
 
-  private setupNavigations(transitions: Observable<NavigationTransition>): Observable<NavigationTransition> {
+  private setupNavigations(transitions: Observable<NavigationTransition>):
+      Observable<NavigationTransition> {
     return transitions.pipe(
-        filter(t => t.id !== 0),
+        filter(t => t.id !== 0), mergeMap(t => Promise.resolve(t)),
         // Extract URL
-        map(t => ({...t, extractedUrl: this.urlHandlingStrategy.extract(t.rawUrl)}) as NavigationTransition),
+        map(t => ({
+              ...t,                                                          //
+                  extractedUrl: this.urlHandlingStrategy.extract(t.rawUrl),  //
+            } as NavigationTransition)),
 
         // Using switchMap so we cancel executing navigations when a new one comes in
         switchMap(t => {
@@ -391,10 +395,16 @@ export class Router {
                       tap(t => this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange &&
                               this.setBrowserUrl(t.rawUrl, !!t.extras.replaceUrl, t.id)),
                       // Fire NavigationStart event
-                      tap(t =>
-                              (this.events as Subject<Event>)
-                                  .next(new NavigationStart(
-                                      t.id, this.serializeUrl(t.extractedUrl), t.source, t.state))),
+                      mergeMap(t => {
+                        const transition = this.transitions.getValue();
+                        (this.events as Subject<Event>)
+                            .next(new NavigationStart(
+                                t.id, this.serializeUrl(t.extractedUrl), t.source, t.state));
+                        if (transition !== this.transitions.getValue()) {
+                          EMPTY;
+                        }
+                        return [t];
+                      }),
 
                       // This delay is required to match old behavior that forced navigation to
                       // always be async
@@ -450,11 +460,10 @@ export class Router {
                 preActivation.initialize(this.rootContexts);
                 return {...t, preActivation};
               }),
-              checkGuards(),
-              tap(t => this.triggerEvent(new GuardsCheckEnd(
-                      t.id, this.serializeUrl(t.extractedUrl),
-                      this.serializeUrl(t.urlAfterRedirects), t.targetSnapshot !,
-                      !!t.guardsResult))),
+              checkGuards(), tap(t => this.triggerEvent(new GuardsCheckEnd(
+                                     t.id, this.serializeUrl(t.extractedUrl),
+                                     this.serializeUrl(t.urlAfterRedirects), t.targetSnapshot !,
+                                     !!t.guardsResult))),
 
               mergeMapIf(
                   t => !t.guardsResult,
@@ -533,7 +542,7 @@ export class Router {
                 }
                 return EMPTY;
               }), );
-            })) as any as Observable<NavigationTransition>;
+        })) as any as Observable<NavigationTransition>;
 
     function activate(
         rootContexts: ChildrenOutletContexts, routeReuseStrategy: RouteReuseStrategy,

packages/router/test/integration.spec.ts

@@ -14,7 +14,7 @@ import {By} from '@angular/platform-browser/src/dom/debug/by';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, ParamMap, Params, PreloadAllModules, PreloadingStrategy, Resolve, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RouteReuseStrategy, Router, RouterEvent, RouterModule, RouterPreloader, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlSerializer, UrlTree} from '@angular/router';
 import {Observable, Observer, Subscription, of } from 'rxjs';
-import {filter, map} from 'rxjs/operators';
+import {filter, first, map, tap} from 'rxjs/operators';
 
 import {forEach} from '../src/utils/collection';
 import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing';
@@ -2970,6 +2970,35 @@ describe('Integration', () => {
            [NavigationEnd, '/user/fedor']
          ]);
        })));
+
+    it('should allow redirection in NavigationStart', fakeAsync(inject([Router], (router: Router) => {
+        const fixture = createRoot(router, RootCmp);
+
+        router.resetConfig([
+          {path: 'blank', component: UserCmp},
+          {path: 'user/:name', component: BlankCmp},
+        ]);
+
+        const navigateSpy = spyOn(router, 'navigate').and.callThrough();
+        const recordedEvents: any[] = [];
+
+        const navStart$ = router.events.pipe(
+          tap(e => recordedEvents.push(e)),
+          filter(e => e instanceof NavigationStart),
+          first()
+        );
+
+        navStart$.subscribe((e: NavigationStart | NavigationError) => {
+          router.navigate(['/blank'], {queryParams: {state: 'redirected'}, queryParamsHandling: 'merge'});
+          advance(fixture);
+        });
+
+        router.navigate(['/user/:fedor']);
+        advance(fixture);
+
+        expect(navigateSpy.calls.mostRecent().args[1].queryParams);
+
+       })));
   });
 
   describe('routerActiveLink', () => {