refactor(router): Move navigationId handling to the transition manager (angular#48202)

atscott · atscott · commit 0ff5d972cdd2 · 2022-11-29T14:55:25.000-08:00
The navigationId is really just a count of how many navigations have been processed through the navigation pipeline. This tracking should be done as part of the navigation transition handler PR Close angular#48202
diff --git a/packages/router/src/navigation_transition.ts b/packages/router/src/navigation_transition.ts
@@ -266,7 +266,6 @@ interface InternalRouterInterface {
   browserUrlTree: UrlTree;
   currentUrlTree: UrlTree;
   rawUrlTree: UrlTree;
-  navigationId: number;
   readonly routerState: RouterState;
   errorHandler: ErrorHandler;
   titleStrategy?: TitleStrategy;
@@ -297,10 +296,11 @@ export class NavigationTransitions {
   private readonly environmentInjector = inject(EnvironmentInjector);
   private readonly urlSerializer = inject(UrlSerializer);
   private readonly rootContexts = inject(ChildrenOutletContexts);
-  private _transitions?: BehaviorSubject<NavigationTransition>;
-  get transitions(): BehaviorSubject<NavigationTransition>|undefined {
-    return this._transitions;
+  navigationId = 0;
+  get hasRequestedNavigation() {
+    return this.navigationId !== 0;
   }
+  private transitions?: BehaviorSubject<NavigationTransition>;
 
   constructor() {
     const onLoadStart = (r: Route) => this.events.next(new RouteConfigLoadStart(r));
@@ -309,8 +309,21 @@ export class NavigationTransitions {
     this.configLoader.onLoadStartListener = onLoadStart;
   }
 
+  complete() {
+    this.transitions?.complete();
+  }
+
+  handleNavigationRequest(
+      request: Pick<
+          NavigationTransition,
+          'targetPageId'|'source'|'restoredState'|'currentUrlTree'|'currentRawUrl'|'rawUrl'|
+          'extras'|'resolve'|'reject'|'promise'|'currentSnapshot'|'currentRouterState'>) {
+    const id = ++this.navigationId;
+    this.transitions?.next({...this.transitions.value, ...request, id});
+  }
+
   setupNavigations(router: InternalRouterInterface): Observable<NavigationTransition> {
-    this._transitions = new BehaviorSubject<NavigationTransition>({
+    this.transitions = new BehaviorSubject<NavigationTransition>({
       id: 0,
       targetPageId: 0,
       currentUrlTree: router.currentUrlTree,
@@ -331,7 +344,7 @@ export class NavigationTransitions {
       guards: {canActivateChecks: [], canDeactivateChecks: []},
       guardsResult: null,
     });
-    return this._transitions.pipe(
+    return this.transitions.pipe(
                filter(t => t.id !== 0),
 
                // Extract URL
@@ -662,7 +675,7 @@ export class NavigationTransitions {
                                  `Navigation ID ${
                                      overallTransitionState
                                          .id} is not equal to the current navigation id ${
-                                     router.navigationId}` :
+                                     this.navigationId}` :
                                  '';
                              this.cancelNavigationTransition(
                                  overallTransitionState, cancelationReason,
diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
@@ -178,8 +178,11 @@ export class Router {
   private disposed = false;
 
   private locationSubscription?: SubscriptionLike;
-  /** @internal */
-  navigationId: number = 0;
+  // TODO(b/260747083): This should not exist and navigationId should be private in
+  // `NavigationTransitions`
+  private get navigationId() {
+    return this.navigationTransitions.navigationId;
+  }
 
   /**
    * The id of the currently active page in the router.
@@ -394,17 +397,12 @@ export class Router {
     this.routerState.root.component = this.rootComponentType;
   }
 
-  private setTransition(t: Partial<NavigationTransition>): void {
-    this.navigationTransitions.transitions?.next(
-        {...this.navigationTransitions.transitions?.value, ...t});
-  }
-
   /**
    * Sets up the location change listener and performs the initial navigation.
    */
   initialNavigation(): void {
     this.setUpLocationChangeListener();
-    if (this.navigationId === 0) {
+    if (!this.navigationTransitions.hasRequestedNavigation) {
       this.navigateByUrl(this.location.path(true), {replaceUrl: true});
     }
   }
@@ -499,7 +497,7 @@ export class Router {
 
   /** Disposes of the router. */
   dispose(): void {
-    this.navigationTransitions.transitions?.complete();
+    this.navigationTransitions.complete();
     if (this.locationSubscription) {
       this.locationSubscription.unsubscribe();
       this.locationSubscription = undefined;
@@ -736,7 +734,6 @@ export class Router {
       });
     }
 
-    const id = ++this.navigationId;
     let targetPageId: number;
     if (this.canceledNavigationResolution === 'computed') {
       const isInitialPage = this.currentPageId === 0;
@@ -762,12 +759,12 @@ export class Router {
       targetPageId = 0;
     }
 
-    this.setTransition({
-      id,
+    this.navigationTransitions.handleNavigationRequest({
       targetPageId,
       source,
       restoredState,
       currentUrlTree: this.currentUrlTree,
+      currentRawUrl: this.currentUrlTree,
       rawUrl,
       extras,
       resolve,
