fixup! refactor(@angular/ssr): combine redirect URL parameter helpers and use DefaultUrlSerializer

Author: alan-agius4

packages/angular/ssr/src/app.ts

@@ -167,21 +167,11 @@ export class AngularServerApp {
     const { redirectTo, relativeRedirect, status, renderMode, headers } = matchedRoute;
 
     if (redirectTo !== undefined) {
-      const prefix = getFirstHeaderValue(request.headers.get('X-Forwarded-Prefix'));
-      let location = buildUrlWithParams(
-        redirectTo,
-        url.pathname,
-        relativeRedirect ? undefined : url.search,
+      const location = joinUrlParts(
+        getFirstHeaderValue(request.headers.get('X-Forwarded-Prefix')) ?? '',
+        buildUrlWithParams(redirectTo, url.pathname, relativeRedirect ? undefined : url.search),
       );
 
-      if (prefix) {
-        const searchIndex = location.search(/[?#]/);
-        location =
-          searchIndex === -1
-            ? joinUrlParts(prefix, location)
-            : joinUrlParts(prefix, location.slice(0, searchIndex)) + location.slice(searchIndex);
-      }
-
       // Match Angular router behavior: relative redirects keep the query string.
       return createRedirectResponse(
         relativeRedirect ? location + url.search : location,

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

@@ -206,15 +206,12 @@ async function* handleRoute(options: {
         let routeSegments: string[] | undefined;
         parentRoutes.set(metadata.route, {
           ...metadata,
-          redirectTo: resolveRedirectTo(
-            parentRoute,
-            // Reference the route parameters by their position in the URL (see `buildUrlWithParams`).
-            replaceRedirectParams(route, redirectTo, (parameter) => {
-              routeSegments ??= joinUrlParts(baseHref, currentRoutePath).split('/');
-
-              return `*${routeSegments.lastIndexOf(parameter)}`;
-            }),
-          ),
+          // Reference the route parameters by their position in the URL (see `buildUrlWithParams`).
+          redirectTo: resolveRedirectTo(parentRoute, route, redirectTo, (parameter) => {
+            routeSegments ??= joinUrlParts(baseHref, currentRoutePath).split('/');
+
+            return `*${routeSegments.lastIndexOf(parameter)}`;
+          }),
           relativeRedirect: redirectTo[0] !== '/' ? true : undefined,
         });
       } else {
@@ -469,15 +466,12 @@ async function* handleSSGRoute(
   const redirectTo = typeof route.redirectTo === 'string' ? route.redirectTo : undefined;
   if (redirectTo !== undefined) {
     let routeSegments: string[] | undefined;
-    meta.redirectTo = resolveRedirectTo(
-      parentRoute,
-      // Reference the route parameters by their position in the URL (see `buildUrlWithParams`).
-      replaceRedirectParams(route, redirectTo, (parameter) => {
-        routeSegments ??= joinUrlParts(baseHref, currentRoutePath).split('/');
+    // Reference the route parameters by their position in the URL (see `buildUrlWithParams`).
+    meta.redirectTo = resolveRedirectTo(parentRoute, route, redirectTo, (parameter) => {
+      routeSegments ??= joinUrlParts(baseHref, currentRoutePath).split('/');
 
-        return `*${routeSegments.lastIndexOf(parameter)}`;
-      }),
-    );
+      return `*${routeSegments.lastIndexOf(parameter)}`;
+    });
     meta.relativeRedirect = redirectTo[0] !== '/' ? true : undefined;
   }
 
@@ -538,11 +532,9 @@ async function* handleSSGRoute(
               ? undefined
               : resolveRedirectTo(
                   parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, replacer),
-                  replaceRedirectParams(
-                    route,
-                    redirectTo,
-                    (parameter) => params[parameter.slice(1)],
-                  ),
+                  route,
+                  redirectTo,
+                  (parameter) => params[parameter.slice(1)],
                 ),
         };
       }
@@ -595,73 +587,51 @@ function handlePrerenderParamsReplacement(
 }
 
 /**
- * Resolves the `redirectTo` property for a given route.
+ * Resolves the `redirectTo` property for a given route, replacing any `:id` route parameters
+ * in its path and resolving relative paths against `parentRoute`.
  *
- * This function processes the `redirectTo` property to ensure that it correctly
- * resolves relative to the parent route path. If `redirectTo` is an absolute path,
- * it is returned as is. If it is a relative path, it is resolved based on the parent route path,
- * and, like in the Angular router, its query and fragment are ignored.
+ * Like in the Angular router, only whole path segments starting with `:` are parameters, only
+ * parameters defined in the path of the redirecting route itself can be used, and the query and
+ * fragment of relative redirects are ignored.
  *
  * @param parentRoute - The path of the parent route.
- * @param redirectTo - The target path for redirection.
- * @returns The resolved redirect path as a string.
- */
-function resolveRedirectTo(parentRoute: string, redirectTo: string): string {
-  if (redirectTo[0] === '/') {
-    // If the redirectTo path is absolute, return it as is.
-    return redirectTo;
-  }
-
-  // Match Angular router behavior: replace all segments matched by the route.
-  const [path] = redirectTo.split(/[?#]/, 1);
-
-  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, and only
- * the parameters defined in the path of the redirecting route itself can be used.
- *
  * @param route - The redirecting route.
  * @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.
+ * @param resolveParam - A function that returns the replacement for a parameter, such as `:id`.
+ * @returns The resolved redirect path as a string.
  * @throws If `redirectTo` uses a parameter that is not defined in the path of the route.
  */
-function replaceRedirectParams(
+function resolveRedirectTo(
+  parentRoute: string,
   route: Route,
   redirectTo: string,
-  resolve: (parameter: string) => string,
+  resolveParam: (parameter: string) => string,
 ): string {
-  if (route.matcher || !redirectTo.includes(':')) {
-    // The parameters of a custom matcher are only known at runtime.
-    return redirectTo;
-  }
-
   const [path] = redirectTo.split(/[?#]/, 1);
-  if (!path.includes(':')) {
-    return redirectTo;
-  }
-
-  const routeSegments = route.path?.split('/') ?? [];
-  const resolvedPath = path
-    .split('/')
-    .map((segment) => {
-      if (segment[0] !== ':') {
-        return segment;
-      }
+  let resolvedPath = path;
+
+  // The parameters of a custom matcher are only known at runtime.
+  if (!route.matcher && path.includes(':')) {
+    const routeSegments = route.path?.split('/') ?? [];
+    resolvedPath = path
+      .split('/')
+      .map((segment) => {
+        if (segment[0] !== ':') {
+          return segment;
+        }
 
-      if (!routeSegments.includes(segment)) {
-        throw new Error(`Cannot redirect to '${redirectTo}'. Cannot find '${segment}'.`);
-      }
+        if (!routeSegments.includes(segment)) {
+          throw new Error(`Cannot redirect to '${redirectTo}'. Cannot find '${segment}'.`);
+        }
 
-      return resolve(segment);
-    })
-    .join('/');
+        return resolveParam(segment);
+      })
+      .join('/');
+  }
 
-  return resolvedPath + redirectTo.slice(path.length);
+  return redirectTo[0] === '/'
+    ? resolvedPath + redirectTo.slice(path.length)
+    : joinUrlParts(parentRoute.replace(URL_PARAMETER_GLOBAL_REGEXP, '*'), resolvedPath);
 }
 
 /**
@@ -939,12 +909,7 @@ export function extractRoutesAndCreateRouteTree(options: {
 
     for (const { route, ...metadata } of routes) {
       if (metadata.redirectTo !== undefined) {
-        const searchIndex = metadata.redirectTo.search(/[?#]/);
-        metadata.redirectTo =
-          searchIndex === -1
-            ? joinUrlParts(baseHref, metadata.redirectTo)
-            : joinUrlParts(baseHref, metadata.redirectTo.slice(0, searchIndex)) +
-              metadata.redirectTo.slice(searchIndex);
+        metadata.redirectTo = joinUrlParts(baseHref, metadata.redirectTo);
       }
 
       // Remove undefined fields

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

@@ -183,37 +183,26 @@ export function buildUrlWithParams(toUrl: string, fromPath: string, fromSearch?:
     throw new Error(`Invalid fromPath: The string must start with a '/'. Received: '${fromPath}'`);
   }
 
-  const hasPathParams = toUrl.includes('/*');
-  const hasQueryParams = Boolean(fromSearch !== undefined && toUrl.includes('=:'));
-  if (!hasPathParams && !hasQueryParams) {
+  if (!toUrl.includes('/*') && (fromSearch === undefined || !toUrl.includes('=:'))) {
     return toUrl;
   }
 
-  const searchIndex = toUrl.search(/[?#]/);
-  let path = searchIndex === -1 ? toUrl : toUrl.slice(0, searchIndex);
-  let suffix = searchIndex === -1 ? '' : toUrl.slice(searchIndex);
+  const [path] = toUrl.split(/[?#]/, 1);
+  let resolvedPath = path;
+  let suffix = toUrl.slice(path.length);
 
-  if (hasPathParams && path.includes('/*')) {
+  if (path.includes('/*')) {
     const fromPathParts = fromPath.split('/');
-    const toPathParts = path.split('/');
-    const resolvedParts = toPathParts.map((part, index) => {
-      if (part[0] !== '*') {
-        return part;
-      }
-
-      if (part.length === 1) {
-        return fromPathParts[index];
-      }
-
-      const position = Number(part.slice(1));
-
-      return Number.isInteger(position) ? fromPathParts[position] : part;
-    });
-
-    path = joinUrlParts(...resolvedParts);
+    resolvedPath = joinUrlParts(
+      ...path
+        .split('/')
+        .map((part, index) =>
+          part[0] === '*' ? fromPathParts[part.length === 1 ? index : +part.slice(1)] : part,
+        ),
+    );
   }
 
-  if (hasQueryParams && suffix.includes('=:')) {
+  if (fromSearch !== undefined && suffix.includes('=:')) {
     urlSerializer ??= new DefaultUrlSerializer();
     const toTree = urlSerializer.parse(suffix);
     const actualParams = fromSearch ? urlSerializer.parse(fromSearch).queryParams : {};
@@ -228,7 +217,7 @@ export function buildUrlWithParams(toUrl: string, fromPath: string, fromSearch?:
     suffix = urlSerializer.serialize(toTree).slice(1);
   }
 
-  return path + suffix;
+  return resolvedPath + suffix;
 }
 
 const MATRIX_PARAMS_REGEX = /;[^/]+/g;