fix(@angular/ssr): resolve route parameters in redirect targets (#34321) · angular/angular-cli@97ed36e · GitHub
Skip to content

Commit 97ed36e

Browse files
bschaeublinalan-agius4
authored andcommitted
fix(@angular/ssr): resolve route parameters in redirect targets (#34321)
Route parameters in the path of a `redirectTo` target, such as `:id` in `profile/:id`, were sent unchanged in the `Location` header by the server, while the router replaces them with the matching parameters of the current URL. The server now does the same. PR Close #34321
1 parent 6bae97b commit 97ed36e

5 files changed

Lines changed: 199 additions & 11 deletions

File tree

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

Lines changed: 78 additions & 6 deletions

‎packages/angular/ssr/src/utils/url.ts‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,18 @@ export function stripIndexHtmlFromURL(url: URL): URL {
152152
return url;
153153
}
154154

155+
/**
156+
* Matches a `*<n>` placeholder that makes up a whole path segment.
157+
*/
158+
const POSITIONAL_PLACEHOLDER_REGEXP = /^\*(\d+)$/;
159+
155160
/**
156161
* Resolves `*` placeholders in a path template by mapping them to corresponding segments
157162
* from a base path. This is useful for constructing paths dynamically based on a given base path.
158163
*
159164
* The function processes the `toPath` string, replacing each `*` placeholder with
160-
* the corresponding segment from the `fromPath`. If the `toPath` contains no placeholders,
165+
* the corresponding segment from the `fromPath`, and each `*<n>` placeholder with the
166+
* segment at position `n` of the `fromPath`. If the `toPath` contains no placeholders,
161167
* it is returned as-is. Invalid `toPath` formats (not starting with `/`) will throw an error.
162168
*
163169
* @param toPath - A path template string that may contain `*` placeholders. Each `*` is replaced
@@ -176,6 +182,10 @@ export function stripIndexHtmlFromURL(url: URL): URL {
176182
* const resolvedPath = buildPathWithParams('/*\/details', '/123/abc');
177183
* console.log(resolvedPath); // Outputs: '/123/details'
178184
*
185+
* // Example with a positional placeholder
186+
* const positionalPath = buildPathWithParams('/user/*1', '/123/abc');
187+
* console.log(positionalPath); // Outputs: '/user/123'
188+
*
179189
* // Example with a static path
180190
* const staticPath = buildPathWithParams('/static/path', '/base/unused');
181191
* console.log(staticPath); // Outputs: '/static/path'
@@ -190,17 +200,20 @@ export function buildPathWithParams(toPath: string, fromPath: string): string {
190200
throw new Error(`Invalid fromPath: The string must start with a '/'. Received: '${fromPath}'`);
191201
}
192202

193-
if (!toPath.includes('/*')) {
203+
const [path] = toPath.split(/[?#]/, 1);
204+
if (!path.includes('/*')) {
194205
return toPath;
195206
}
196207

197208
const fromPathParts = fromPath.split('/');
198-
const toPathParts = toPath.split('/');
209+
const toPathParts = path.split('/');
199210
const resolvedParts = toPathParts.map((part, index) =>
200-
toPathParts[index] === '*' ? fromPathParts[index] : part,
211+
part === '*'
212+
? fromPathParts[index]
213+
: part.replace(POSITIONAL_PLACEHOLDER_REGEXP, (_, position) => fromPathParts[position]),
201214
);
202215

203-
return joinUrlParts(...resolvedParts);
216+
return joinUrlParts(...resolvedParts) + toPath.slice(path.length);
204217
}
205218

206219
const MATRIX_PARAMS_REGEX = /;[^/]+/g;

‎packages/angular/ssr/test/app_spec.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ describe('AngularServerApp', () => {
9090
{ path: 'redirect/absolute', redirectTo: '/home' },
9191
{ path: 'redirect-with-query', redirectTo: 'home?param=value' },
9292
{ path: 'redirect-nested/:param', children: [{ path: 'old', redirectTo: 'new' }] },
93+
{ path: 'redirect-param/:a/:b', redirectTo: '/home/:b/:a' },
94+
{ path: 'redirect-param-relative/:id', redirectTo: 'target/:id' },
9395
{
9496
path: 'redirect-to-function',
9597
redirectTo: () => 'home',
@@ -252,6 +254,20 @@ describe('AngularServerApp', () => {
252254
expect(response?.status).toBe(302);
253255
});
254256

257+
it('should resolve route parameters in absolute redirect targets', async () => {
258+
const response = await app.handle(new Request('http://localhost/redirect-param/x/y'));
259+
expect(response?.headers.get('location')).toBe('/home/y/x');
260+
expect(response?.status).toBe(302);
261+
});
262+
263+
it('should resolve route parameters in relative redirect targets', async () => {
264+
const response = await app.handle(
265+
new Request('http://localhost/redirect-param-relative/7?filter=test'),
266+
);
267+
expect(response?.headers.get('location')).toBe('/target/7?filter=test');
268+
expect(response?.status).toBe(302);
269+
});
270+
255271
it('should handle request abortion gracefully', async () => {
256272
const controller = new AbortController();
257273
const request = new Request('http://localhost/home', { signal: controller.signal });

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,38 @@ describe('extractRoutesAndCreateRouteTree', () => {
489489
{ route: '/new', renderMode: RenderMode.Server },
490490
]);
491491
});
492+
493+
it('should resolve route parameters of redirect targets of prerendered routes', async () => {
494+
setAngularAppTestingManifest(
495+
[
496+
{ path: 'user/:id', redirectTo: '/profile/:id' },
497+
{ path: 'profile/:id', component: DummyComponent },
498+
],
499+
[
500+
{
501+
path: 'user/:id',
502+
renderMode: RenderMode.Prerender,
503+
async getPrerenderParams() {
504+
return [{ id: '7' }];
505+
},
506+
},
507+
{ path: '**', renderMode: RenderMode.Server },
508+
],
509+
);
510+
511+
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
512+
url,
513+
invokeGetPrerenderParams: true,
514+
includePrerenderFallbackRoutes: true,
515+
});
516+
517+
expect(errors).toHaveSize(0);
518+
expect(routeTree.toObject()).toEqual([
519+
{ route: '/user/7', renderMode: RenderMode.Prerender, redirectTo: '/profile/7' },
520+
{ route: '/user/*', renderMode: RenderMode.Server, redirectTo: '/profile/*2' },
521+
{ route: '/profile/*', renderMode: RenderMode.Server },
522+
]);
523+
});
492524
});
493525

494526
it('should extract routes with a route level matcher', async () => {
@@ -652,6 +684,52 @@ describe('extractRoutesAndCreateRouteTree', () => {
652684
]);
653685
});
654686

687+
it('should reference route parameters of redirect targets by their position', async () => {
688+
setAngularAppTestingManifest(
689+
[
690+
{
691+
path: ':lang',
692+
children: [
693+
{ path: 'user/:id', redirectTo: 'profile/:id' },
694+
{ path: 'profile/:id', component: DummyComponent },
695+
],
696+
},
697+
{ path: 'search/:term', redirectTo: '/find/:term' },
698+
],
699+
[{ path: '**', renderMode: RenderMode.Server }],
700+
);
701+
702+
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({ url });
703+
expect(errors).toHaveSize(0);
704+
expect(routeTree.toObject()).toEqual([
705+
{ route: '/*', renderMode: RenderMode.Server },
706+
{
707+
route: '/*/user/*',
708+
renderMode: RenderMode.Server,
709+
redirectTo: '/*/profile/*3',
710+
relativeRedirect: true,
711+
},
712+
{ route: '/*/profile/*', renderMode: RenderMode.Server },
713+
{ route: '/search/*', renderMode: RenderMode.Server, redirectTo: '/find/*2' },
714+
]);
715+
});
716+
717+
it('should error when a redirect target uses a parameter not defined by the route', async () => {
718+
setAngularAppTestingManifest(
719+
[
720+
{
721+
path: ':lang',
722+
children: [{ path: 'user/:id', redirectTo: 'profile/:lang' }],
723+
},
724+
],
725+
[{ path: '**', renderMode: RenderMode.Server }],
726+
);
727+
728+
const { errors } = await extractRoutesAndCreateRouteTree({ url });
729+
expect(errors).toHaveSize(1);
730+
expect(errors[0]).toContain(`Cannot redirect to 'profile/:lang'. Cannot find ':lang'.`);
731+
});
732+
655733
it('should resolve multi-segment relative redirects against the parent route', async () => {
656734
setAngularAppTestingManifest(
657735
[

‎packages/angular/ssr/test/utils/url_spec.ts‎

Lines changed: 9 additions & 0 deletions

0 commit comments

Comments
 (0)