Skip to content

fix(@angular/ssr): properly manage catch-all routes with base href #29414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/angular/ssr/src/routes/ng-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -583,13 +583,13 @@ export async function getRoutesFromAngularRouterConfig(

if (serverConfigRouteTree) {
for (const { route, presentInClientRouter } of serverConfigRouteTree.traverse()) {
if (presentInClientRouter || route === '**') {
if (presentInClientRouter || route.endsWith('/**')) {
// Skip if matched or it's the catch-all route.
continue;
}

errors.push(
`The '${route}' server route does not match any routes defined in the Angular ` +
`The '${stripLeadingSlash(route)}' server route does not match any routes defined in the Angular ` +
`routing configuration (typically provided as a part of the 'provideRouter' call). ` +
'Please make sure that the mentioned server route is present in the Angular routing configuration.',
);
Expand Down
18 changes: 7 additions & 11 deletions packages/angular/ssr/src/routes/route-tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { stripTrailingSlash } from '../utils/url';
import { addLeadingSlash } from '../utils/url';
import { RenderMode } from './route-config';

/**
Expand Down Expand Up @@ -116,7 +116,7 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
* The root node of the route tree.
* All routes are stored and accessed relative to this root node.
*/
private readonly root = this.createEmptyRouteTreeNode('');
private readonly root = this.createEmptyRouteTreeNode('<root>');

/**
* A counter that tracks the order of route insertion.
Expand Down Expand Up @@ -155,7 +155,7 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
// At the leaf node, store the full route and its associated metadata
node.metadata = {
...metadata,
route: normalizedSegments.join('/'),
route: addLeadingSlash(normalizedSegments.join('/')),
};

node.insertionIndex = this.insertionIndexCounter++;
Expand Down Expand Up @@ -230,7 +230,7 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
* @returns An array of path segments.
*/
private getPathSegments(route: string): string[] {
return stripTrailingSlash(route).split('/');
return route.split('/').filter(Boolean);
}

/**
Expand All @@ -246,18 +246,14 @@ export class RouteTree<AdditionalMetadata extends Record<string, unknown> = {}>
* @returns The node that best matches the remaining segments or `undefined` if no match is found.
*/
private traverseBySegments(
remainingSegments: string[] | undefined,
remainingSegments: string[],
node = this.root,
): RouteTreeNode<AdditionalMetadata> | undefined {
const { metadata, children } = node;

// If there are no remaining segments and the node has metadata, return this node
if (!remainingSegments?.length) {
if (metadata) {
return node;
}

return;
if (!remainingSegments.length) {
return metadata ? node : node.children.get('**');
}

// If the node has no children, end the traversal
Expand Down
10 changes: 9 additions & 1 deletion packages/angular/ssr/test/routes/route-tree_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('RouteTree', () => {
describe('match', () => {
it('should handle empty routes', () => {
routeTree.insert('', { renderMode: RenderMode.Server });
expect(routeTree.match('')).toEqual({ route: '', renderMode: RenderMode.Server });
expect(routeTree.match('')).toEqual({ route: '/', renderMode: RenderMode.Server });
});

it('should insert and match basic routes', () => {
Expand Down Expand Up @@ -274,5 +274,13 @@ describe('RouteTree', () => {
renderMode: RenderMode.Server,
});
});

it('should correctly match catch-all segments with a prefix', () => {
routeTree.insert('/de/**', { renderMode: RenderMode.Server });
expect(routeTree.match('/de')).toEqual({
route: '/de/**',
renderMode: RenderMode.Server,
});
});
});
});
Loading