Skip to content

Commit 05b3d8f

Browse files
committed
fix(@angular/ssr): substitute route parameters used in redirectTo
When a route redirected to a path using one of its parameters (e.g. `old/:id` to `new/:id`), the server responded with the literal placeholder, `Location: /new/:id`, while the Angular router navigates to `/new/5`. Substitute the matched values, and for prerendered routes the `getPrerenderParams` values. Fixes #34410
1 parent 0fd4404 commit 05b3d8f

5 files changed

Lines changed: 130 additions & 15 deletions

File tree

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

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { AngularAppManifest, getAngularAppManifest } from '../manifest';
3131
import { AngularBootstrap, isNgModule } from '../utils/ng';
3232
import { promiseWithAbort } from '../utils/promise';
3333
import { VALID_REDIRECT_RESPONSE_CODES, isValidRedirectResponseCode } from '../utils/redirect';
34-
import { joinUrlParts, stripLeadingSlash } from '../utils/url';
34+
import { addLeadingSlash, joinUrlParts, stripLeadingSlash } from '../utils/url';
3535
import {
3636
PrerenderFallback,
3737
RenderMode,
@@ -202,7 +202,7 @@ async function* handleRoute(options: {
202202
} else if (typeof redirectTo === 'string') {
203203
parentRoutes.set(metadata.route, {
204204
...metadata,
205-
redirectTo: resolveRedirectTo(parentRoute, redirectTo),
205+
redirectTo: resolveRedirectTo(parentRoute, redirectTo, currentRoutePath),
206206
relativeRedirect: redirectTo[0] !== '/' ? true : undefined,
207207
});
208208
} else {
@@ -452,7 +452,7 @@ async function* handleSSGRoute(
452452
}
453453

454454
if (redirectTo !== undefined) {
455-
meta.redirectTo = resolveRedirectTo(parentRoute, redirectTo);
455+
meta.redirectTo = resolveRedirectTo(parentRoute, redirectTo, currentRoutePath);
456456
meta.relativeRedirect = redirectTo[0] !== '/' ? true : undefined;
457457
}
458458

@@ -513,7 +513,10 @@ async function* handleSSGRoute(
513513
? undefined
514514
: resolveRedirectTo(
515515
parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer),
516-
redirectTo,
516+
redirectTo.replace(URL_PARAMETER_GLOBAL_REGEXP, (match) =>
517+
Object.hasOwn(params, match.slice(1)) ? replacer(match) : match,
518+
),
519+
currentRoutePath,
517520
),
518521
};
519522
}
@@ -570,23 +573,53 @@ function handlePrerenderParamsReplacement(
570573
*
571574
* This function processes the `redirectTo` property to ensure that it correctly
572575
* resolves relative to the parent route path. If `redirectTo` is an absolute path,
573-
* it is returned as is. If it is a relative path, it is resolved based on the parent route path,
576+
* it is used as is. If it is a relative path, it is resolved based on the parent route path,
574577
* and, like in the Angular router, its query and fragment are ignored.
575578
*
579+
* Like in the Angular router, route parameters (e.g. `:id`) used in `redirectTo` take the value
580+
* they have in the matched URL. They are emitted as `*` placeholders, or `*<n>` when the value
581+
* comes from a different segment position than the one it is used in.
582+
*
576583
* @param parentRoute - The path of the parent route.
577584
* @param redirectTo - The target path for redirection.
585+
* @param currentRoutePath - The full path of the route that redirects, including its parent route.
578586
* @returns The resolved redirect path as a string.
579587
*/
580-
function resolveRedirectTo(parentRoute: string, redirectTo: string): string {
588+
function resolveRedirectTo(
589+
parentRoute: string,
590+
redirectTo: string,
591+
currentRoutePath: string,
592+
): string {
593+
let target: string;
581594
if (redirectTo[0] === '/') {
582-
// If the redirectTo path is absolute, return it as is.
583-
return redirectTo;
595+
target = redirectTo;
596+
} else {
597+
// Match Angular router behavior: replace all segments matched by the route.
598+
const [path] = redirectTo.split(/[?#]/, 1);
599+
target = joinUrlParts(parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, '*'), path);
600+
}
601+
602+
if (!URL_PARAMETER_REGEXP.test(target)) {
603+
return target;
584604
}
585605

586-
// Match Angular router behavior: replace all segments matched by the route.
587-
const [path] = redirectTo.split(/[?#]/, 1);
606+
const sourceSegments = addLeadingSlash(currentRoutePath).split('/');
607+
608+
return target
609+
.split('/')
610+
.map((segment, index) => {
611+
if (segment[0] !== ':') {
612+
return segment;
613+
}
614+
615+
const sourceIndex = sourceSegments.indexOf(segment);
616+
if (sourceIndex === -1) {
617+
return segment;
618+
}
588619

589-
return joinUrlParts(parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, '*'), path);
620+
return sourceIndex === index ? '*' : `*${sourceIndex}`;
621+
})
622+
.join('/');
590623
}
591624

592625
/**

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,8 @@ export function stripIndexHtmlFromURL(url: URL): URL {
157157
* from a base path. This is useful for constructing paths dynamically based on a given base path.
158158
*
159159
* The function processes the `toPath` string, replacing each `*` placeholder with
160-
* the corresponding segment from the `fromPath`. If the `toPath` contains no placeholders,
160+
* the corresponding segment from the `fromPath`. A `*<n>` placeholder (e.g. `*3`) takes the
161+
* segment at position `n` of the `fromPath` instead of the one at its own position. If the `toPath` contains no placeholders,
161162
* it is returned as-is. Invalid `toPath` formats (not starting with `/`) will throw an error.
162163
*
163164
* @param toPath - A path template string that may contain `*` placeholders. Each `*` is replaced
@@ -196,9 +197,15 @@ export function buildPathWithParams(toPath: string, fromPath: string): string {
196197

197198
const fromPathParts = fromPath.split('/');
198199
const toPathParts = toPath.split('/');
199-
const resolvedParts = toPathParts.map((part, index) =>
200-
toPathParts[index] === '*' ? fromPathParts[index] : part,
201-
);
200+
const resolvedParts = toPathParts.map((part, index) => {
201+
// `*` takes the segment at the same position, `*<n>` the segment at position `n`.
202+
const match = /^\*(\d*)$/.exec(part);
203+
if (!match) {
204+
return part;
205+
}
206+
207+
return fromPathParts[match[1] ? Number(match[1]) : index];
208+
});
202209

203210
return joinUrlParts(...resolvedParts);
204211
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ 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/:id', redirectTo: 'redirect-target/:id' },
94+
{ path: 'redirect-param-deep/:id', redirectTo: 'redirect-target/deep/:id' },
95+
{
96+
path: 'redirect-nested-param/:parent',
97+
children: [{ path: 'old/:id', redirectTo: 'new/:id/:parent' }],
98+
},
9399
{
94100
path: 'redirect-to-function',
95101
redirectTo: () => 'home',
@@ -446,6 +452,24 @@ describe('AngularServerApp', () => {
446452
});
447453

448454
describe('SSR pages', () => {
455+
it('substitutes route parameters used in `redirectTo`', async () => {
456+
const response = await app.handle(new Request('http://localhost/redirect-param/5'));
457+
expect(response?.headers.get('location')).toBe('/redirect-target/5');
458+
expect(response?.status).toBe(302);
459+
});
460+
461+
it('substitutes route parameters used at a different position in `redirectTo`', async () => {
462+
const response = await app.handle(new Request('http://localhost/redirect-param-deep/5'));
463+
expect(response?.headers.get('location')).toBe('/redirect-target/deep/5');
464+
});
465+
466+
it('substitutes parameters of parent routes used in `redirectTo`', async () => {
467+
const response = await app.handle(
468+
new Request('http://localhost/redirect-nested-param/a/old/b'),
469+
);
470+
expect(response?.headers.get('location')).toBe('/redirect-nested-param/a/new/b/a');
471+
});
472+
449473
it('returns a 302 status and redirects to the correct location when `redirectTo` is a function', async () => {
450474
const response = await app.handle(new Request('http://localhost/redirect-to-function'));
451475
expect(response?.headers.get('location')).toBe('/home');

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,48 @@ describe('extractRoutesAndCreateRouteTree', () => {
448448
]);
449449
});
450450

451+
it('should substitute route parameters used in the `redirectTo` of prerendered routes', async () => {
452+
setAngularAppTestingManifest(
453+
[
454+
{ path: 'old/:id', redirectTo: 'new/:id' },
455+
{ path: 'new/:id', component: DummyComponent },
456+
],
457+
[
458+
{
459+
path: 'old/:id',
460+
renderMode: RenderMode.Prerender,
461+
async getPrerenderParams() {
462+
return [{ id: '5' }];
463+
},
464+
},
465+
{ path: '**', renderMode: RenderMode.Server },
466+
],
467+
);
468+
469+
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({
470+
url,
471+
invokeGetPrerenderParams: true,
472+
includePrerenderFallbackRoutes: true,
473+
});
474+
475+
expect(errors).toHaveSize(0);
476+
expect(routeTree.toObject()).toEqual([
477+
{
478+
route: '/old/5',
479+
renderMode: RenderMode.Prerender,
480+
redirectTo: '/new/5',
481+
relativeRedirect: true,
482+
},
483+
{
484+
route: '/old/*',
485+
renderMode: RenderMode.Server,
486+
redirectTo: '/new/*',
487+
relativeRedirect: true,
488+
},
489+
{ route: '/new/*', renderMode: RenderMode.Server },
490+
]);
491+
});
492+
451493
it('should resolve relative redirects of prerendered catch-all routes', async () => {
452494
setAngularAppTestingManifest(
453495
[

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,15 @@ describe('URL Utils', () => {
184184
expect(result.toString()).toBe('/base/path/details');
185185
});
186186

187+
it('should take the segment at position `n` for `*<n>` placeholders', () => {
188+
const fromPath = '/old/5';
189+
const toPath = '/new/deep/*2';
190+
191+
const result = buildPathWithParams(toPath, fromPath);
192+
193+
expect(result.toString()).toBe('/new/deep/5');
194+
});
195+
187196
it('should throw an error if the toPath does not start with "/"', () => {
188197
const fromPath = '/base/path';
189198
const toPath = 'details';

0 commit comments

Comments
 (0)