refactor(router): refactor and simplify router RxJS chains (#40290)

Refactor and simplifiy RxJS usage in the router package in order to reduce its size and increase performance. PR Close #40290
angular · Jan 11, 2021 · 9105005 · 9105005
1 parent 4a7a649
commit 9105005
Show file tree

Hide file tree

Showing 11 changed files with 70 additions and 105 deletions.
diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json
@@ -66,4 +66,4 @@
       }
     }
   }
-}
+}
diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json
@@ -1238,12 +1238,6 @@
   {
     "name": "findPath"
   },
-  {
-    "name": "fireActivationStart"
-  },
-  {
-    "name": "fireChildActivationStart"
-  },
   {
     "name": "first"
   },
@@ -1832,12 +1826,6 @@
   {
     "name": "routerNgProbeToken"
   },
-  {
-    "name": "runCanActivate"
-  },
-  {
-    "name": "runCanActivateChild"
-  },
   {
     "name": "rxSubscriber"
   },

diff --git a/packages/router/src/directives/router_link_active.ts b/packages/router/src/directives/router_link_active.ts
@@ -105,12 +105,10 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
   /** @nodoc */
   ngAfterContentInit(): void {
     // `of(null)` is used to force subscribe body to execute once immediately (like `startWith`).
-    from([this.links.changes, this.linksWithHrefs.changes, of(null)])
-        .pipe(mergeAll())
-        .subscribe(_ => {
-          this.update();
-          this.subscribeToEachLinkOnChanges();
-        });
+    of(this.links.changes, this.linksWithHrefs.changes, of(null)).pipe(mergeAll()).subscribe(_ => {
+      this.update();
+      this.subscribeToEachLinkOnChanges();
+    });
   }
 
   private subscribeToEachLinkOnChanges() {

diff --git a/packages/router/src/operators/apply_redirects.ts b/packages/router/src/operators/apply_redirects.ts
@@ -7,7 +7,7 @@
  */
 
 import {Injector} from '@angular/core';
-import {MonoTypeOperatorFunction, Observable} from 'rxjs';
+import {MonoTypeOperatorFunction} from 'rxjs';
 import {map, switchMap} from 'rxjs/operators';
 
 import {applyRedirects as applyRedirectsFn} from '../apply_redirects';
@@ -19,9 +19,7 @@ import {UrlSerializer} from '../url_tree';
 export function applyRedirects(
     moduleInjector: Injector, configLoader: RouterConfigLoader, urlSerializer: UrlSerializer,
     config: Routes): MonoTypeOperatorFunction<NavigationTransition> {
-  return function(source: Observable<NavigationTransition>) {
-    return source.pipe(switchMap(
-        t => applyRedirectsFn(moduleInjector, configLoader, urlSerializer, t.extractedUrl, config)
-                 .pipe(map(urlAfterRedirects => ({...t, urlAfterRedirects})))));
-  };
+  return switchMap(
+      t => applyRedirectsFn(moduleInjector, configLoader, urlSerializer, t.extractedUrl, config)
+               .pipe(map(urlAfterRedirects => ({...t, urlAfterRedirects}))));
 }
diff --git a/packages/router/src/operators/check_guards.ts b/packages/router/src/operators/check_guards.ts
@@ -7,8 +7,8 @@
  */
 
 import {Injector} from '@angular/core';
-import {defer, from, MonoTypeOperatorFunction, Observable, of} from 'rxjs';
-import {concatAll, concatMap, first, map, mergeMap} from 'rxjs/operators';
+import {concat, defer, from, MonoTypeOperatorFunction, Observable, of} from 'rxjs';
+import {concatMap, first, map, mergeMap} from 'rxjs/operators';
 
 import {ActivationStart, ChildActivationStart, Event} from '../events';
 import {CanActivateChildFn, CanActivateFn, CanDeactivateFn} from '../interfaces';
@@ -23,25 +23,23 @@ import {prioritizedGuardValue} from './prioritized_guard_value';
 
 export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event) => void):
     MonoTypeOperatorFunction<NavigationTransition> {
-  return function(source: Observable<NavigationTransition>) {
-    return source.pipe(mergeMap(t => {
-      const {targetSnapshot, currentSnapshot, guards: {canActivateChecks, canDeactivateChecks}} = t;
-      if (canDeactivateChecks.length === 0 && canActivateChecks.length === 0) {
-        return of({...t, guardsResult: true});
-      }
+  return mergeMap(t => {
+    const {targetSnapshot, currentSnapshot, guards: {canActivateChecks, canDeactivateChecks}} = t;
+    if (canDeactivateChecks.length === 0 && canActivateChecks.length === 0) {
+      return of({...t, guardsResult: true});
+    }
 
-      return runCanDeactivateChecks(
-                 canDeactivateChecks, targetSnapshot!, currentSnapshot, moduleInjector)
-          .pipe(
-              mergeMap(canDeactivate => {
-                return canDeactivate && isBoolean(canDeactivate) ?
-                    runCanActivateChecks(
-                        targetSnapshot!, canActivateChecks, moduleInjector, forwardEvent) :
-                    of(canDeactivate);
-              }),
-              map(guardsResult => ({...t, guardsResult})));
-    }));
-  };
+    return runCanDeactivateChecks(
+               canDeactivateChecks, targetSnapshot!, currentSnapshot, moduleInjector)
+        .pipe(
+            mergeMap(canDeactivate => {
+              return canDeactivate && isBoolean(canDeactivate) ?
+                  runCanActivateChecks(
+                      targetSnapshot!, canActivateChecks, moduleInjector, forwardEvent) :
+                  of(canDeactivate);
+            }),
+            map(guardsResult => ({...t, guardsResult})));
+  });
 }
 
 function runCanDeactivateChecks(
@@ -61,15 +59,11 @@ function runCanActivateChecks(
     forwardEvent?: (evt: Event) => void) {
   return from(checks).pipe(
       concatMap((check: CanActivate) => {
-        return from([
-                 fireChildActivationStart(check.route.parent, forwardEvent),
-                 fireActivationStart(check.route, forwardEvent),
-                 runCanActivateChild(futureSnapshot, check.path, moduleInjector),
-                 runCanActivate(futureSnapshot, check.route, moduleInjector)
-               ])
-            .pipe(concatAll(), first(result => {
-                    return result !== true;
-                  }, true as boolean | UrlTree));
+        return concat(
+            fireChildActivationStart(check.route.parent, forwardEvent),
+            fireActivationStart(check.route, forwardEvent),
+            runCanActivateChild(futureSnapshot, check.path, moduleInjector),
+            runCanActivate(futureSnapshot, check.route, moduleInjector));
       }),
       first(result => {
         return result !== true;

diff --git a/packages/router/src/operators/prioritized_guard_value.ts b/packages/router/src/operators/prioritized_guard_value.ts
@@ -18,8 +18,7 @@ declare type INTERIM_VALUES = typeof INITIAL_VALUE | boolean | UrlTree;
 export function prioritizedGuardValue():
     OperatorFunction<Observable<boolean|UrlTree>[], boolean|UrlTree> {
   return switchMap(obs => {
-    return combineLatest(
-               ...obs.map(o => o.pipe(take(1), startWith(INITIAL_VALUE as INTERIM_VALUES))))
+    return combineLatest(obs.map(o => o.pipe(take(1), startWith(INITIAL_VALUE as INTERIM_VALUES))))
                .pipe(
                    scan(
                        (acc: INTERIM_VALUES, list: INTERIM_VALUES[]) => {

diff --git a/packages/router/src/operators/recognize.ts b/packages/router/src/operators/recognize.ts
@@ -7,7 +7,7 @@
  */
 
 import {Type} from '@angular/core';
-import {MonoTypeOperatorFunction, Observable} from 'rxjs';
+import {MonoTypeOperatorFunction} from 'rxjs';
 import {map, mergeMap} from 'rxjs/operators';
 
 import {Route} from '../config';
@@ -19,11 +19,9 @@ export function recognize(
     rootComponentType: Type<any>|null, config: Route[], serializer: (url: UrlTree) => string,
     paramsInheritanceStrategy: 'emptyOnly'|'always',
     relativeLinkResolution: 'legacy'|'corrected'): MonoTypeOperatorFunction<NavigationTransition> {
-  return function(source: Observable<NavigationTransition>) {
-    return source.pipe(mergeMap(
-        t => recognizeFn(
-                 rootComponentType, config, t.urlAfterRedirects, serializer(t.urlAfterRedirects),
-                 paramsInheritanceStrategy, relativeLinkResolution)
-                 .pipe(map(targetSnapshot => ({...t, targetSnapshot})))));
-  };
+  return mergeMap(
+      t => recognizeFn(
+               rootComponentType, config, t.urlAfterRedirects, serializer(t.urlAfterRedirects),
+               paramsInheritanceStrategy, relativeLinkResolution)
+               .pipe(map(targetSnapshot => ({...t, targetSnapshot}))));
 }
diff --git a/packages/router/src/operators/resolve_data.ts b/packages/router/src/operators/resolve_data.ts
@@ -19,25 +19,23 @@ import {getToken} from '../utils/preactivation';
 export function resolveData(
     paramsInheritanceStrategy: 'emptyOnly'|'always',
     moduleInjector: Injector): MonoTypeOperatorFunction<NavigationTransition> {
-  return function(source: Observable<NavigationTransition>) {
-    return source.pipe(mergeMap(t => {
-      const {targetSnapshot, guards: {canActivateChecks}} = t;
+  return mergeMap(t => {
+    const {targetSnapshot, guards: {canActivateChecks}} = t;
 
-      if (!canActivateChecks.length) {
-        return of(t);
-      }
-      let canActivateChecksResolved = 0;
-      return from(canActivateChecks)
-          .pipe(
-              concatMap(
-                  check => runResolve(
-                      check.route, targetSnapshot!, paramsInheritanceStrategy, moduleInjector)),
-              tap(() => canActivateChecksResolved++),
-              takeLast(1),
-              mergeMap(_ => canActivateChecksResolved === canActivateChecks.length ? of(t) : EMPTY),
-          );
-    }));
-  };
+    if (!canActivateChecks.length) {
+      return of(t);
+    }
+    let canActivateChecksResolved = 0;
+    return from(canActivateChecks)
+        .pipe(
+            concatMap(
+                check => runResolve(
+                    check.route, targetSnapshot!, paramsInheritanceStrategy, moduleInjector)),
+            tap(() => canActivateChecksResolved++),
+            takeLast(1),
+            mergeMap(_ => canActivateChecksResolved === canActivateChecks.length ? of(t) : EMPTY),
+        );
+  });
 }
 
 function runResolve(

diff --git a/packages/router/src/operators/switch_tap.ts b/packages/router/src/operators/switch_tap.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {from, MonoTypeOperatorFunction, ObservableInput} from 'rxjs';
+import {from, MonoTypeOperatorFunction, ObservableInput, of} from 'rxjs';
 import {map, switchMap} from 'rxjs/operators';
 
 /**
@@ -17,13 +17,11 @@ import {map, switchMap} from 'rxjs/operators';
  */
 export function switchTap<T>(next: (x: T) => void|ObservableInput<any>):
     MonoTypeOperatorFunction<T> {
-  return function(source) {
-    return source.pipe(switchMap(v => {
-      const nextResult = next(v);
-      if (nextResult) {
-        return from(nextResult).pipe(map(() => v));
-      }
-      return from([v]);
-    }));
-  };
+  return switchMap(v => {
+    const nextResult = next(v);
+    if (nextResult) {
+      return from(nextResult).pipe(map(() => v));
+    }
+    return of(v);
+  });
 }
diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
@@ -573,12 +573,11 @@ export class Router {
                                if (transition !== this.transitions.getValue()) {
                                  return EMPTY;
                                }
-                               return [t];
-                             }),
 
-                             // This delay is required to match old behavior that forced navigation
-                             // to always be async
-                             switchMap(t => Promise.resolve(t)),
+                               // This delay is required to match old behavior that forced
+                               // navigation to always be async
+                               return Promise.resolve(t);
+                             }),
 
                              // ApplyRedirects
                              applyRedirects(
@@ -609,10 +608,8 @@ export class Router {
                                  }
                                  this.browserUrlTree = t.urlAfterRedirects;
                                }
-                             }),
 
-                             // Fire RoutesRecognized
-                             tap(t => {
+                               // Fire RoutesRecognized
                                const routesRecognized = new RoutesRecognized(
                                    t.id, this.serializeUrl(t.extractedUrl),
                                    this.serializeUrl(t.urlAfterRedirects), t.targetSnapshot!);
@@ -692,9 +689,7 @@ export class Router {
                          error.url = t.guardsResult;
                          throw error;
                        }
-                     }),
 
-                     tap(t => {
                        const guardsEnd = new GuardsCheckEnd(
                            t.id, this.serializeUrl(t.extractedUrl),
                            this.serializeUrl(t.urlAfterRedirects), t.targetSnapshot!,
@@ -875,7 +870,7 @@ export class Router {
                                replaceUrl: this.urlUpdateStrategy === 'eager'
                              };
 
-                             return this.scheduleNavigation(
+                             this.scheduleNavigation(
                                  mergedTree, 'imperative', null, extras,
                                  {resolve: t.resolve, reject: t.reject, promise: t.promise});
                            }, 0);

diff --git a/packages/router/src/utils/collection.ts b/packages/router/src/utils/collection.ts
@@ -8,9 +8,8 @@
 
 import {ɵisObservable as isObservable, ɵisPromise as isPromise} from '@angular/core';
 import {from, Observable, of} from 'rxjs';
-import {concatAll, last as lastValue, map} from 'rxjs/operators';
 
-import {Params, PRIMARY_OUTLET} from '../shared';
+import {Params} from '../shared';
 
 export function shallowEqualArrays(a: any[], b: any[]): boolean {
   if (a.length !== b.length) return false;