diff --git a/packages/angular/ssr/src/app.ts b/packages/angular/ssr/src/app.ts index 517cf1e31727..0255b8b3b136 100644 --- a/packages/angular/ssr/src/app.ts +++ b/packages/angular/ssr/src/app.ts @@ -24,7 +24,12 @@ import { ServerRouter } from './routes/router'; import { AngularBootstrap, renderAngular } from './utils/ng'; import { promiseWithAbort } from './utils/promise'; import { createRedirectResponse } from './utils/redirect'; -import { buildPathWithParams, joinUrlParts, stripLeadingSlash } from './utils/url'; +import { + buildPathWithParams, + buildQueryWithParams, + joinUrlParts, + stripLeadingSlash, +} from './utils/url'; /** * A set of well-known URLs that are not handled by Angular. @@ -173,7 +178,7 @@ export class AngularServerApp { // Match Angular router behavior: relative redirects keep the query string. return createRedirectResponse( - relativeRedirect ? location + url.search : location, + relativeRedirect ? location + url.search : buildQueryWithParams(location, url.searchParams), status, headers, ); diff --git a/packages/angular/ssr/src/routes/ng-routes.ts b/packages/angular/ssr/src/routes/ng-routes.ts index 915d7c2b9a47..698a6918eafa 100644 --- a/packages/angular/ssr/src/routes/ng-routes.ts +++ b/packages/angular/ssr/src/routes/ng-routes.ts @@ -485,9 +485,12 @@ async function* handleSSGRoute( redirectTo: redirectTo === undefined ? undefined - : resolveRedirectTo( - parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer), - redirectTo, + : replaceRedirectParams( + resolveRedirectTo( + parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer), + redirectTo, + ), + (parameter) => params[parameter.slice(1)], ), }; } @@ -563,6 +566,29 @@ function resolveRedirectTo(parentRoute: string, redirectTo: string): string { return joinUrlParts(parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, '*'), path); } +/** + * Replaces the route parameters, such as `:id`, in the path of `redirectTo`. + * + * Like in the Angular router, only whole path segments starting with `:` are parameters. + * Parameters for which `resolve` returns `undefined` are kept as is. + * + * @param redirectTo - The target path for redirection. + * @param resolve - A function that returns the replacement for a parameter, such as `:id`. + * @returns The redirect path with its route parameters replaced. + */ +function replaceRedirectParams( + redirectTo: string, + resolve: (parameter: string) => string | undefined, +): string { + const [path] = redirectTo.split(/[?#]/, 1); + const resolvedPath = path + .split('/') + .map((segment) => (segment[0] === ':' ? (resolve(segment) ?? segment) : segment)) + .join('/'); + + return resolvedPath + redirectTo.slice(path.length); +} + /** * Builds a server configuration route tree from the given server routes configuration. * @@ -836,8 +862,19 @@ export function extractRoutesAndCreateRouteTree(options: { ); for (const { route, ...metadata } of routes) { + const fullRoute = joinUrlParts(baseHref, route); + if (metadata.redirectTo !== undefined) { - metadata.redirectTo = joinUrlParts(baseHref, metadata.redirectTo); + // Reference the route parameters by their position in the URL (see `buildPathWithParams`). + const routeSegments = fullRoute.split('/'); + metadata.redirectTo = replaceRedirectParams( + joinUrlParts(baseHref, metadata.redirectTo), + (parameter) => { + const position = routeSegments.lastIndexOf(parameter); + + return position === -1 ? undefined : `*${position}`; + }, + ); } // Remove undefined fields @@ -849,7 +886,6 @@ export function extractRoutesAndCreateRouteTree(options: { } } - const fullRoute = joinUrlParts(baseHref, route); routeTree.insert(fullRoute, metadata); } diff --git a/packages/angular/ssr/src/utils/url.ts b/packages/angular/ssr/src/utils/url.ts index d5e7c9ac2814..ff80599a836e 100644 --- a/packages/angular/ssr/src/utils/url.ts +++ b/packages/angular/ssr/src/utils/url.ts @@ -152,12 +152,19 @@ export function stripIndexHtmlFromURL(url: URL): URL { return url; } +/** + * Matches a `*` placeholder that makes up a whole path segment, which may be followed by + * the query or fragment of the URL. + */ +const POSITIONAL_PLACEHOLDER_REGEXP = /^\*(\d+)(?=[?#]|$)/; + /** * Resolves `*` placeholders in a path template by mapping them to corresponding segments * from a base path. This is useful for constructing paths dynamically based on a given base path. * * The function processes the `toPath` string, replacing each `*` placeholder with - * the corresponding segment from the `fromPath`. If the `toPath` contains no placeholders, + * the corresponding segment from the `fromPath`, and each `*` placeholder with the + * segment at position `n` of the `fromPath`. If the `toPath` contains no placeholders, * it is returned as-is. Invalid `toPath` formats (not starting with `/`) will throw an error. * * @param toPath - A path template string that may contain `*` placeholders. Each `*` is replaced @@ -176,6 +183,10 @@ export function stripIndexHtmlFromURL(url: URL): URL { * const resolvedPath = buildPathWithParams('/*\/details', '/123/abc'); * console.log(resolvedPath); // Outputs: '/123/details' * + * // Example with a positional placeholder + * const positionalPath = buildPathWithParams('/user/*1', '/123/abc'); + * console.log(positionalPath); // Outputs: '/user/123' + * * // Example with a static path * const staticPath = buildPathWithParams('/static/path', '/base/unused'); * console.log(staticPath); // Outputs: '/static/path' @@ -197,12 +208,51 @@ export function buildPathWithParams(toPath: string, fromPath: string): string { const fromPathParts = fromPath.split('/'); const toPathParts = toPath.split('/'); const resolvedParts = toPathParts.map((part, index) => - toPathParts[index] === '*' ? fromPathParts[index] : part, + part === '*' + ? fromPathParts[index] + : part.replace(POSITIONAL_PLACEHOLDER_REGEXP, (_, position) => fromPathParts[position]), ); return joinUrlParts(...resolvedParts); } +/** + * Resolves `:name` placeholders in the query of a URL, like the Angular router does for redirects. + * + * Each query parameter whose value is a `:name` placeholder gets the values of the `name` + * query parameter of `fromParams`, and is removed if `fromParams` has no such parameter. + * + * @param toUrl - A URL whose query may contain `:name` placeholders. + * @param fromParams - The query parameters that provide the values for the placeholders. + * @returns The URL with its query placeholders resolved. + * + * @example + * ```typescript + * buildQueryWithParams('/search?q=:term', new URLSearchParams('term=cats')); // returns '/search?q=cats' + * ``` + */ +export function buildQueryWithParams(toUrl: string, fromParams: URLSearchParams): string { + const [, path, query, fragment] = /^([^?#]*)\?([^#]*)(.*)$/.exec(toUrl) ?? []; + if (query === undefined || !query.includes('=:')) { + return toUrl; + } + + const resolvedParams = new URLSearchParams(); + for (const [name, value] of new URLSearchParams(query)) { + if (value[0] === ':') { + for (const sourceValue of fromParams.getAll(value.slice(1))) { + resolvedParams.append(name, sourceValue); + } + } else { + resolvedParams.append(name, value); + } + } + + const resolvedQuery = resolvedParams.toString(); + + return path + (resolvedQuery ? `?${resolvedQuery}` : '') + fragment; +} + const MATRIX_PARAMS_REGEX = /;[^/]+/g; /** diff --git a/packages/angular/ssr/test/app_spec.ts b/packages/angular/ssr/test/app_spec.ts index 8d8ee9add00b..35c05ad15a79 100644 --- a/packages/angular/ssr/test/app_spec.ts +++ b/packages/angular/ssr/test/app_spec.ts @@ -90,6 +90,12 @@ describe('AngularServerApp', () => { { path: 'redirect/absolute', redirectTo: '/home' }, { path: 'redirect-with-query', redirectTo: 'home?param=value' }, { path: 'redirect-nested/:param', children: [{ path: 'old', redirectTo: 'new' }] }, + { path: 'redirect-param/:a/:b', redirectTo: '/home/:b/:a' }, + { path: 'redirect-param-relative/:id', redirectTo: 'target/:id' }, + { + path: 'redirect-query-placeholder', + redirectTo: '/home?q=:term&id=:missing&static=value', + }, { path: 'redirect-to-function', redirectTo: () => 'home', @@ -252,6 +258,28 @@ describe('AngularServerApp', () => { expect(response?.status).toBe(302); }); + it('should resolve route parameters in absolute redirect targets', async () => { + const response = await app.handle(new Request('http://localhost/redirect-param/x/y')); + expect(response?.headers.get('location')).toBe('/home/y/x'); + expect(response?.status).toBe(302); + }); + + it('should resolve route parameters in relative redirect targets', async () => { + const response = await app.handle( + new Request('http://localhost/redirect-param-relative/7?filter=test'), + ); + expect(response?.headers.get('location')).toBe('/target/7?filter=test'); + expect(response?.status).toBe(302); + }); + + it('should resolve query parameters in absolute redirect targets', async () => { + const response = await app.handle( + new Request('http://localhost/redirect-query-placeholder?term=cats&term=dogs'), + ); + expect(response?.headers.get('location')).toBe('/home?q=cats&q=dogs&static=value'); + expect(response?.status).toBe(302); + }); + it('should handle request abortion gracefully', async () => { const controller = new AbortController(); const request = new Request('http://localhost/home', { signal: controller.signal }); diff --git a/packages/angular/ssr/test/routes/ng-routes_spec.ts b/packages/angular/ssr/test/routes/ng-routes_spec.ts index 1af823f8f2ba..81de52bd07f7 100644 --- a/packages/angular/ssr/test/routes/ng-routes_spec.ts +++ b/packages/angular/ssr/test/routes/ng-routes_spec.ts @@ -489,6 +489,38 @@ describe('extractRoutesAndCreateRouteTree', () => { { route: '/new', renderMode: RenderMode.Server }, ]); }); + + it('should resolve route parameters of redirect targets of prerendered routes', async () => { + setAngularAppTestingManifest( + [ + { path: 'user/:id', redirectTo: '/profile/:id' }, + { path: 'profile/:id', component: DummyComponent }, + ], + [ + { + path: 'user/:id', + renderMode: RenderMode.Prerender, + async getPrerenderParams() { + return [{ id: '7' }]; + }, + }, + { path: '**', renderMode: RenderMode.Server }, + ], + ); + + const { routeTree, errors } = await extractRoutesAndCreateRouteTree({ + url, + invokeGetPrerenderParams: true, + includePrerenderFallbackRoutes: true, + }); + + expect(errors).toHaveSize(0); + expect(routeTree.toObject()).toEqual([ + { route: '/user/7', renderMode: RenderMode.Prerender, redirectTo: '/profile/7' }, + { route: '/user/*', renderMode: RenderMode.Server, redirectTo: '/profile/*2' }, + { route: '/profile/*', renderMode: RenderMode.Server }, + ]); + }); }); it('should extract routes with a route level matcher', async () => { @@ -652,6 +684,36 @@ describe('extractRoutesAndCreateRouteTree', () => { ]); }); + it('should reference route parameters of redirect targets by their position', async () => { + setAngularAppTestingManifest( + [ + { + path: ':lang', + children: [ + { path: 'user/:id', redirectTo: 'profile/:id' }, + { path: 'profile/:id', component: DummyComponent }, + ], + }, + { path: 'search/:term', redirectTo: '/find/:term' }, + ], + [{ path: '**', renderMode: RenderMode.Server }], + ); + + const { routeTree, errors } = await extractRoutesAndCreateRouteTree({ url }); + expect(errors).toHaveSize(0); + expect(routeTree.toObject()).toEqual([ + { route: '/*', renderMode: RenderMode.Server }, + { + route: '/*/user/*', + renderMode: RenderMode.Server, + redirectTo: '/*/profile/*3', + relativeRedirect: true, + }, + { route: '/*/profile/*', renderMode: RenderMode.Server }, + { route: '/search/*', renderMode: RenderMode.Server, redirectTo: '/find/*2' }, + ]); + }); + it('should resolve multi-segment relative redirects against the parent route', async () => { setAngularAppTestingManifest( [ diff --git a/packages/angular/ssr/test/utils/url_spec.ts b/packages/angular/ssr/test/utils/url_spec.ts index a108c7ff1df6..749fd3a29380 100644 --- a/packages/angular/ssr/test/utils/url_spec.ts +++ b/packages/angular/ssr/test/utils/url_spec.ts @@ -10,6 +10,7 @@ import { addLeadingSlash, addTrailingSlash, buildPathWithParams, + buildQueryWithParams, joinUrlParts, stripIndexHtmlFromURL, stripLeadingSlash, @@ -184,6 +185,15 @@ describe('URL Utils', () => { expect(result.toString()).toBe('/base/path/details'); }); + it('should replace positional placeholders with the segments at those positions', () => { + expect(buildPathWithParams('/user/*2/*1', '/a/b')).toBe('/user/b/a'); + }); + + it('should replace a positional placeholder followed by a query or fragment', () => { + expect(buildPathWithParams('/user/*1?tab=1', '/a')).toBe('/user/a?tab=1'); + expect(buildPathWithParams('/user/*1#top', '/a')).toBe('/user/a#top'); + }); + it('should throw an error if the toPath does not start with "/"', () => { const fromPath = '/base/path'; const toPath = 'details'; @@ -195,6 +205,21 @@ describe('URL Utils', () => { }); }); + describe('buildQueryWithParams', () => { + it('should replace query placeholders with the values of the source parameters', () => { + const fromParams = new URLSearchParams('term=cats&term=dogs&page=2'); + expect(buildQueryWithParams('/search?q=:term&p=:page&static=1#top', fromParams)).toBe( + '/search?q=cats&q=dogs&p=2&static=1#top', + ); + }); + + it('should remove query parameters whose source parameter is missing', () => { + const fromParams = new URLSearchParams(); + expect(buildQueryWithParams('/search?q=:term&static=1', fromParams)).toBe('/search?static=1'); + expect(buildQueryWithParams('/search?q=:term', fromParams)).toBe('/search'); + }); + }); + describe('stripMatrixParams', () => { it('should remove a single matrix parameter', () => { expect(stripMatrixParams('/path;param=value')).toBe('/path');