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

This commit makes a small update to the route matching algorithm to avoid running the matcher function of a route twice. fixes angular#57511 PR Close angular#57530

Author: atscott

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(

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[],

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') {