fix(router): Do not unnecessarily run matcher twice on route matching (#57530)

atscott · alxhub · commit 804925b11492 · 2024-08-27T13:20:58.000-07:00
This commit makes a small update to the route matching algorithm to avoid running the matcher function of a route twice. fixes #57511 PR Close #57530
diff --git a/packages/router/src/recognize.ts b/packages/router/src/recognize.ts
@@ -37,7 +37,7 @@ import {PRIMARY_OUTLET} from './shared';
 import {UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree';
 import {getOutlet, sortByMatchingOutlets} from './utils/config';
 import {
-  isImmediateMatch,
+  emptyPathMatch,
   match,
   matchWithChecks,
   noLeftoversInUrl,
@@ -293,7 +293,23 @@ export class Recognizer {
     allowRedirects: boolean,
     parentRoute: ActivatedRouteSnapshot,
   ): Observable<TreeNode<ActivatedRouteSnapshot> | NoLeftoversInUrl> {
-    if (!isImmediateMatch(route, rawSegment, segments, outlet)) return noMatch(rawSegment);
+    // We allow matches to empty paths when the outlets differ so we can match a url like `/(b:b)` to
+    // a config like
+    // * `{path: '', children: [{path: 'b', outlet: 'b'}]}`
+    // or even
+    // * `{path: '', outlet: 'a', children: [{path: 'b', outlet: 'b'}]`
+    //
+    // The exception here is when the segment outlet is for the primary outlet. This would
+    // result in a match inside the named outlet because all children there are written as primary
+    // outlets. So we need to prevent child named outlet matches in a url like `/b` in a config like
+    // * `{path: '', outlet: 'x' children: [{path: 'b'}]}`
+    // This should only match if the url is `/(x:b)`.
+    if (
+      getOutlet(route) !== outlet &&
+      (outlet === PRIMARY_OUTLET || !emptyPathMatch(rawSegment, segments, route))
+    ) {
+      return noMatch(rawSegment);
+    }
 
     if (route.redirectTo === undefined) {
       return this.matchSegmentAgainstRoute(
diff --git a/packages/router/src/utils/config_matching.ts b/packages/router/src/utils/config_matching.ts
@@ -195,7 +195,7 @@ function containsEmptyPathMatches(
   return routes.some((r) => emptyPathMatch(segmentGroup, slicedSegments, r));
 }
 
-function emptyPathMatch(
+export function emptyPathMatch(
   segmentGroup: UrlSegmentGroup,
   slicedSegments: UrlSegment[],
   r: Route,
@@ -207,37 +207,6 @@ function emptyPathMatch(
   return r.path === '';
 }
 
-/**
- * Determines if `route` is a path match for the `rawSegment`, `segments`, and `outlet` without
- * verifying that its children are a full match for the remainder of the `rawSegment` children as
- * well.
- */
-export function isImmediateMatch(
-  route: Route,
-  rawSegment: UrlSegmentGroup,
-  segments: UrlSegment[],
-  outlet: string,
-): boolean {
-  // We allow matches to empty paths when the outlets differ so we can match a url like `/(b:b)` to
-  // a config like
-  // * `{path: '', children: [{path: 'b', outlet: 'b'}]}`
-  // or even
-  // * `{path: '', outlet: 'a', children: [{path: 'b', outlet: 'b'}]`
-  //
-  // The exception here is when the segment outlet is for the primary outlet. This would
-  // result in a match inside the named outlet because all children there are written as primary
-  // outlets. So we need to prevent child named outlet matches in a url like `/b` in a config like
-  // * `{path: '', outlet: 'x' children: [{path: 'b'}]}`
-  // This should only match if the url is `/(x:b)`.
-  if (
-    getOutlet(route) !== outlet &&
-    (outlet === PRIMARY_OUTLET || !emptyPathMatch(rawSegment, segments, route))
-  ) {
-    return false;
-  }
-  return match(rawSegment, route, segments).matched;
-}
-
 export function noLeftoversInUrl(
   segmentGroup: UrlSegmentGroup,
   segments: UrlSegment[],
diff --git a/packages/router/test/recognize.spec.ts b/packages/router/test/recognize.spec.ts
@@ -9,7 +9,7 @@
 import {EnvironmentInjector} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 
-import {Routes} from '../src/models';
+import {Routes, UrlMatcher} from '../src/models';
 import {Recognizer} from '../src/recognize';
 import {RouterConfigLoader} from '../src/router_config_loader';
 import {ActivatedRouteSnapshot, RouterStateSnapshot} from '../src/router_state';
@@ -700,6 +700,27 @@ describe('recognize', async () => {
   });
 
   describe('custom path matchers', async () => {
+    it('should run once', async () => {
+      let calls = 0;
+      const matcher: UrlMatcher = (s) => {
+        calls++;
+        return {consumed: s};
+      };
+
+      const s = await recognize(
+        [
+          {
+            matcher,
+            component: ComponentA,
+          },
+        ],
+        '/a/1/b',
+      );
+      const a = s.root.firstChild!;
+      checkActivatedRoute(a, 'a/1/b', {}, ComponentA);
+      expect(calls).toBe(1);
+    });
+
     it('should use custom path matcher', async () => {
       const matcher = (s: any, g: any, r: any) => {
         if (s[0].path === 'a') {
