Skip to content

Commit b01f492

Browse files
alan-agius4alan-agius4
committed
fix(@angular/ssr): enhance dynamic route matching for better performance and accuracy
Updated route matching logic to prioritize closest matches, improving the accuracy of dynamic route resolution. Also we optimized performance by eliminating unnecessary recursive checks, reducing overhead during route matching. Closes #29452
1 parent f5dc745 commit b01f492

File tree

4 files changed

+194
-175
lines changed

4 files changed

+194
-175
lines changed

‎packages/angular/ssr/src/routes/ng-routes.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,13 @@ function buildServerConfigRouteTree({ routes, appShellRoute }: ServerRoutesConfi
458458
continue;
459459
}
460460

461+
if (path.includes('**') && 'getPrerenderParams' in metadata) {
462+
errors.push(
463+
`Invalid '${path}' route configuration: 'getPrerenderParams' cannot be used with a '**' route.`,
464+
);
465+
continue;
466+
}
467+
461468
serverConfigRouteTree.insert(path, metadata);
462469
}
463470

‎packages/angular/ssr/src/routes/route-tree.ts

Lines changed: 29 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import { addLeadingSlash, stripTrailingSlash } from '../utils/url';
9+
import { addLeadingSlash } from '../utils/url';
1010
import { RenderMode } from './route-config';
1111

1212
/**
@@ -78,13 +78,6 @@ export interface RouteTreeNodeMetadata {
7878
* The `AdditionalMetadata` type parameter allows for extending the node metadata with custom data.
7979
*/
8080
interface RouteTreeNode<AdditionalMetadata extends Record<string, unknown>> {
81-
/**
82-
* The index indicating the order in which the route was inserted into the tree.
83-
* This index helps determine the priority of routes during matching, with lower indexes
84-
* indicating earlier inserted routes.
85-
*/
86-
insertionIndex: number;
87-
8881
/**
8982
* A map of child nodes, keyed by their corresponding route segment or wildcard.
9083
*/
@@ -110,13 +103,6 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
110103
*/
111104
private readonly root = this.createEmptyRouteTreeNode();
112105

113-
/**
114-
* A counter that tracks the order of route insertion.
115-
* This ensures that routes are matched in the order they were defined,
116-
* with earlier routes taking precedence.
117-
*/
118-
private insertionIndexCounter = 0;
119-
120106
/**
121107
* Inserts a new route into the route tree.
122108
* The route is broken down into segments, and each segment is added to the tree.
@@ -149,8 +135,6 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
149135
...metadata,
150136
route: addLeadingSlash(normalizedSegments.join('/')),
151137
};
152-
153-
node.insertionIndex = this.insertionIndexCounter++;
154138
}
155139

156140
/**
@@ -222,7 +206,7 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
222206
* @returns An array of path segments.
223207
*/
224208
private getPathSegments(route: string): string[] {
225-
return stripTrailingSlash(route).split('/');
209+
return route.split('/').filter(Boolean);
226210
}
227211

228212
/**
@@ -232,74 +216,48 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
232216
* This function prioritizes exact segment matches first, followed by wildcard matches (`*`),
233217
* and finally deep wildcard matches (`**`) that consume all segments.
234218
*
235-
* @param remainingSegments - The remaining segments of the route path to match.
236-
* @param node - The current node in the route tree to start traversal from.
219+
* @param segments - The array of route path segments to match against the route tree.
220+
* @param node - The current node in the route tree to start traversal from. Defaults to the root node.
221+
* @param currentIndex - The index of the segment in `remainingSegments` currently being matched.
222+
* Defaults to `0` (the first segment).
237223
*
238224
* @returns The node that best matches the remaining segments or `undefined` if no match is found.
239225
*/
240226
private traverseBySegments(
241-
remainingSegments: string[],
227+
segments: string[],
242228
node = this.root,
229+
currentIndex = 0,
243230
): RouteTreeNode<AdditionalMetadata> | undefined {
244-
const { metadata, children } = node;
245-
246-
// If there are no remaining segments and the node has metadata, return this node
247-
if (!remainingSegments.length) {
248-
return metadata ? node : node.children.get('**');
231+
if (currentIndex >= segments.length) {
232+
return node.metadata ? node : node.children.get('**');
249233
}
250234

251-
// If the node has no children, end the traversal
252-
if (!children.size) {
253-
return;
235+
if (!node.children.size) {
236+
return undefined;
254237
}
255238

256-
const [segment, ...restSegments] = remainingSegments;
257-
let currentBestMatchNode: RouteTreeNode<AdditionalMetadata> | undefined;
258-
259-
// 1. Exact segment match
260-
const exactMatchNode = node.children.get(segment);
261-
currentBestMatchNode = this.getHigherPriorityNode(
262-
currentBestMatchNode,
263-
this.traverseBySegments(restSegments, exactMatchNode),
264-
);
265-
266-
// 2. Wildcard segment match (`*`)
267-
const wildcardNode = node.children.get('*');
268-
currentBestMatchNode = this.getHigherPriorityNode(
269-
currentBestMatchNode,
270-
this.traverseBySegments(restSegments, wildcardNode),
271-
);
239+
const segment = segments[currentIndex];
272240

273-
// 3. Deep wildcard segment match (`**`)
274-
const deepWildcardNode = node.children.get('**');
275-
currentBestMatchNode = this.getHigherPriorityNode(currentBestMatchNode, deepWildcardNode);
276-
277-
return currentBestMatchNode;
278-
}
279-
280-
/**
281-
* Compares two nodes and returns the node with higher priority based on insertion index.
282-
* A node with a lower insertion index is prioritized as it was defined earlier.
283-
*
284-
* @param currentBestMatchNode - The current best match node.
285-
* @param candidateNode - The node being evaluated for higher priority based on insertion index.
286-
* @returns The node with higher priority (i.e., lower insertion index). If one of the nodes is `undefined`, the other node is returned.
287-
*/
288-
private getHigherPriorityNode(
289-
currentBestMatchNode: RouteTreeNode<AdditionalMetadata> | undefined,
290-
candidateNode: RouteTreeNode<AdditionalMetadata> | undefined,
291-
): RouteTreeNode<AdditionalMetadata> | undefined {
292-
if (!candidateNode) {
293-
return currentBestMatchNode;
241+
// 1. Attempt exact match with the current segment.
242+
const exactMatch = node.children.get(segment);
243+
if (exactMatch) {
244+
const match = this.traverseBySegments(segments, exactMatch, currentIndex + 1);
245+
if (match) {
246+
return match;
247+
}
294248
}
295249

296-
if (!currentBestMatchNode) {
297-
return candidateNode;
250+
// 2. Attempt wildcard match ('*').
251+
const wildcardMatch = node.children.get('*');
252+
if (wildcardMatch) {
253+
const match = this.traverseBySegments(segments, wildcardMatch, currentIndex + 1);
254+
if (match) {
255+
return match;
256+
}
298257
}
299258

300-
return candidateNode.insertionIndex < currentBestMatchNode.insertionIndex
301-
? candidateNode
302-
: currentBestMatchNode;
259+
// 3. Attempt double wildcard match ('**').
260+
return node.children.get('**');
303261
}
304262

305263
/**
@@ -310,7 +268,6 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
310268
*/
311269
private createEmptyRouteTreeNode(): RouteTreeNode<AdditionalMetadata> {
312270
return {
313-
insertionIndex: -1,
314271
children: new Map(),
315272
};
316273
}

‎packages/angular/ssr/test/routes/ng-routes_spec.ts

Lines changed: 102 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,101 @@ describe('extractRoutesAndCreateRouteTree', () => {
5252
]);
5353
});
5454

55-
it('should handle invalid route configuration path', async () => {
56-
setAngularAppTestingManifest(
57-
[{ path: 'home', component: DummyComponent }],
58-
[
59-
// This path starts with a slash, which should trigger an error
60-
{ path: '/invalid', renderMode: RenderMode.Client },
61-
],
62-
);
55+
describe('route configuration validation', () => {
56+
it(`should error when a route starts with a '/'`, async () => {
57+
setAngularAppTestingManifest(
58+
[{ path: 'home', component: DummyComponent }],
59+
[
60+
// This path starts with a slash, which should trigger an error
61+
{ path: '/invalid', renderMode: RenderMode.Client },
62+
],
63+
);
6364

64-
const { errors } = await extractRoutesAndCreateRouteTree({ url });
65-
expect(errors[0]).toContain(
66-
`Invalid '/invalid' route configuration: the path cannot start with a slash.`,
67-
);
65+
const { errors } = await extractRoutesAndCreateRouteTree({ url });
66+
expect(errors[0]).toContain(
67+
`Invalid '/invalid' route configuration: the path cannot start with a slash.`,
68+
);
69+
});
70+
71+
it("should error when 'getPrerenderParams' is used with a '**' route", async () => {
72+
setAngularAppTestingManifest(
73+
[{ path: 'home', component: DummyComponent }],
74+
[
75+
{
76+
path: '**',
77+
renderMode: RenderMode.Prerender,
78+
getPrerenderParams() {
79+
return Promise.resolve([]);
80+
},
81+
},
82+
],
83+
);
84+
85+
const { errors } = await extractRoutesAndCreateRouteTree({ url });
86+
expect(errors[0]).toContain(
87+
"Invalid '**' route configuration: 'getPrerenderParams' cannot be used with a '**' route.",
88+
);
89+
});
90+
91+
it(`should not error when a catch-all route didn't match any Angular route.`, async () => {
92+
setAngularAppTestingManifest(
93+
[{ path: 'home', component: DummyComponent }],
94+
[
95+
{ path: 'home', renderMode: RenderMode.Server },
96+
{ path: '**', renderMode: RenderMode.Server },
97+
],
98+
);
99+
100+
const { errors } = await extractRoutesAndCreateRouteTree({
101+
url,
102+
invokeGetPrerenderParams: false,
103+
includePrerenderFallbackRoutes: false,
104+
});
105+
106+
expect(errors).toHaveSize(0);
107+
});
108+
109+
it('should error when a route is not defined in the server routing configuration', async () => {
110+
setAngularAppTestingManifest(
111+
[{ path: 'home', component: DummyComponent }],
112+
[
113+
{ path: 'home', renderMode: RenderMode.Server },
114+
{ path: 'invalid', renderMode: RenderMode.Server },
115+
],
116+
);
117+
118+
const { errors } = await extractRoutesAndCreateRouteTree({
119+
url,
120+
invokeGetPrerenderParams: false,
121+
includePrerenderFallbackRoutes: false,
122+
});
123+
124+
expect(errors).toHaveSize(1);
125+
expect(errors[0]).toContain(
126+
`The 'invalid' server route does not match any routes defined in the Angular routing configuration`,
127+
);
128+
});
129+
130+
it('should error when a server route is not defined in the Angular routing configuration', async () => {
131+
setAngularAppTestingManifest(
132+
[
133+
{ path: 'home', component: DummyComponent },
134+
{ path: 'invalid', component: DummyComponent },
135+
],
136+
[{ path: 'home', renderMode: RenderMode.Server }],
137+
);
138+
139+
const { errors } = await extractRoutesAndCreateRouteTree({
140+
url,
141+
invokeGetPrerenderParams: false,
142+
includePrerenderFallbackRoutes: false,
143+
});
144+
145+
expect(errors).toHaveSize(1);
146+
expect(errors[0]).toContain(
147+
`The 'invalid' route does not match any route defined in the server routing configuration`,
148+
);
149+
});
68150
});
69151

70152
describe('when `invokeGetPrerenderParams` is true', () => {
@@ -207,6 +289,13 @@ describe('extractRoutesAndCreateRouteTree', () => {
207289
return [{ param: 'some' }];
208290
},
209291
},
292+
{
293+
path: ':param/*',
294+
renderMode: RenderMode.Prerender,
295+
async getPrerenderParams() {
296+
return [{ param: 'some' }];
297+
},
298+
},
210299
{ path: '**', renderMode: RenderMode.Prerender },
211300
],
212301
);
@@ -216,6 +305,7 @@ describe('extractRoutesAndCreateRouteTree', () => {
216305
invokeGetPrerenderParams: true,
217306
includePrerenderFallbackRoutes: true,
218307
});
308+
219309
expect(errors).toHaveSize(0);
220310
expect(routeTree.toObject()).toEqual([
221311
{ route: '/', renderMode: RenderMode.Prerender, redirectTo: '/some' },
@@ -318,7 +408,6 @@ describe('extractRoutesAndCreateRouteTree', () => {
318408

319409
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
320410
url,
321-
322411
invokeGetPrerenderParams: true,
323412
includePrerenderFallbackRoutes: false,
324413
});
@@ -374,66 +463,6 @@ describe('extractRoutesAndCreateRouteTree', () => {
374463
]);
375464
});
376465

377-
it(`should not error when a catch-all route didn't match any Angular route.`, async () => {
378-
setAngularAppTestingManifest(
379-
[{ path: 'home', component: DummyComponent }],
380-
[
381-
{ path: 'home', renderMode: RenderMode.Server },
382-
{ path: '**', renderMode: RenderMode.Server },
383-
],
384-
);
385-
386-
const { errors } = await extractRoutesAndCreateRouteTree({
387-
url,
388-
invokeGetPrerenderParams: false,
389-
includePrerenderFallbackRoutes: false,
390-
});
391-
392-
expect(errors).toHaveSize(0);
393-
});
394-
395-
it('should error when a route is not defined in the server routing configuration', async () => {
396-
setAngularAppTestingManifest(
397-
[{ path: 'home', component: DummyComponent }],
398-
[
399-
{ path: 'home', renderMode: RenderMode.Server },
400-
{ path: 'invalid', renderMode: RenderMode.Server },
401-
],
402-
);
403-
404-
const { errors } = await extractRoutesAndCreateRouteTree({
405-
url,
406-
invokeGetPrerenderParams: false,
407-
includePrerenderFallbackRoutes: false,
408-
});
409-
410-
expect(errors).toHaveSize(1);
411-
expect(errors[0]).toContain(
412-
`The 'invalid' server route does not match any routes defined in the Angular routing configuration`,
413-
);
414-
});
415-
416-
it('should error when a server route is not defined in the Angular routing configuration', async () => {
417-
setAngularAppTestingManifest(
418-
[
419-
{ path: 'home', component: DummyComponent },
420-
{ path: 'invalid', component: DummyComponent },
421-
],
422-
[{ path: 'home', renderMode: RenderMode.Server }],
423-
);
424-
425-
const { errors } = await extractRoutesAndCreateRouteTree({
426-
url,
427-
invokeGetPrerenderParams: false,
428-
includePrerenderFallbackRoutes: false,
429-
});
430-
431-
expect(errors).toHaveSize(1);
432-
expect(errors[0]).toContain(
433-
`The 'invalid' route does not match any route defined in the server routing configuration`,
434-
);
435-
});
436-
437466
it('should use wildcard configuration when no Angular routes are defined', async () => {
438467
setAngularAppTestingManifest([], [{ path: '**', renderMode: RenderMode.Server, status: 201 }]);
439468

0 commit comments

Comments
 (0)