From 2640bf7a680300acf18cf6502c57a00e0a5bfda9 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Mon, 16 Sep 2024 19:07:52 +0000 Subject: [PATCH] fix(@angular/ssr): correct route extraction and error handling This commit introduces the following changes: - Disallows paths starting with a slash to match Angular router behavior. - Errors are now stored and displayed at a later stage, improving UX by avoiding unnecessary stack traces that are not useful in this context. --- .../src/utils/server-rendering/prerender.ts | 20 +- .../routes-extractor-worker.ts | 16 +- packages/angular/ssr/node/src/app-engine.ts | 2 +- packages/angular/ssr/src/routes/ng-routes.ts | 293 +++++++++++------- packages/angular/ssr/src/routes/route-tree.ts | 17 +- packages/angular/ssr/src/routes/router.ts | 11 +- packages/angular/ssr/src/utils/url.ts | 47 ++- packages/angular/ssr/test/app-engine_spec.ts | 4 +- packages/angular/ssr/test/app_spec.ts | 8 +- .../angular/ssr/test/routes/ng-routes_spec.ts | 62 +++- .../angular/ssr/test/routes/router_spec.ts | 4 +- packages/angular/ssr/test/utils/url_spec.ts | 46 ++- 12 files changed, 365 insertions(+), 165 deletions(-) diff --git a/packages/angular/build/src/utils/server-rendering/prerender.ts b/packages/angular/build/src/utils/server-rendering/prerender.ts index f6e1b877b819..1b53fd9ca6d9 100644 --- a/packages/angular/build/src/utils/server-rendering/prerender.ts +++ b/packages/angular/build/src/utils/server-rendering/prerender.ts @@ -15,8 +15,9 @@ import { BuildOutputAsset } from '../../tools/esbuild/bundler-execution-result'; import { urlJoin } from '../url'; import type { RenderWorkerData } from './render-worker'; import type { + RoutersExtractorWorkerResult, RoutesExtractorWorkerData, - RoutersExtractorWorkerResult as SerializableRouteTreeNode, + SerializableRouteTreeNode, } from './routes-extractor-worker'; interface PrerenderOptions { @@ -298,14 +299,15 @@ async function getAllRoutes( }); const errors: string[] = []; - const serializableRouteTreeNode: SerializableRouteTreeNode = await renderWorker - .run({}) - .catch((err) => { - errors.push(`An error occurred while extracting routes.\n\n${err.stack}`); - }) - .finally(() => { - void renderWorker.destroy(); - }); + const { serializedRouteTree: serializableRouteTreeNode }: RoutersExtractorWorkerResult = + await renderWorker + .run({}) + .catch((err) => { + errors.push(`An error occurred while extracting routes.\n\n${err.stack}`); + }) + .finally(() => { + void renderWorker.destroy(); + }); const skippedRedirects: string[] = []; const skippedOthers: string[] = []; diff --git a/packages/angular/build/src/utils/server-rendering/routes-extractor-worker.ts b/packages/angular/build/src/utils/server-rendering/routes-extractor-worker.ts index 80afa0cfdccd..f62f30649491 100644 --- a/packages/angular/build/src/utils/server-rendering/routes-extractor-worker.ts +++ b/packages/angular/build/src/utils/server-rendering/routes-extractor-worker.ts @@ -15,22 +15,30 @@ export interface RoutesExtractorWorkerData extends ESMInMemoryFileLoaderWorkerDa assetFiles: Record; } -export type RoutersExtractorWorkerResult = ReturnType< - Awaited>['toObject'] +export type SerializableRouteTreeNode = ReturnType< + Awaited>['routeTree']['toObject'] >; +export interface RoutersExtractorWorkerResult { + serializedRouteTree: SerializableRouteTreeNode; + errors: string[]; +} + /** Renders an application based on a provided options. */ async function extractRoutes(): Promise { const { ɵextractRoutesAndCreateRouteTree: extractRoutesAndCreateRouteTree } = await loadEsmModuleFromMemory('./main.server.mjs'); - const routeTree = await extractRoutesAndCreateRouteTree( + const { routeTree, errors } = await extractRoutesAndCreateRouteTree( new URL('http://local-angular-prerender/'), /** manifest */ undefined, /** invokeGetPrerenderParams */ true, ); - return routeTree.toObject(); + return { + errors, + serializedRouteTree: routeTree.toObject(), + }; } function initialize() { diff --git a/packages/angular/ssr/node/src/app-engine.ts b/packages/angular/ssr/node/src/app-engine.ts index 3ae8f16e10a4..abfbe2944650 100644 --- a/packages/angular/ssr/node/src/app-engine.ts +++ b/packages/angular/ssr/node/src/app-engine.ts @@ -59,7 +59,7 @@ export class AngularNodeAppEngine { * const headers = angularAppEngine.getPrerenderHeaders(res.req); * * // Apply the retrieved headers to the response - * for (const { key, value } of headers) { + * for (const [key, value] of headers) { * res.setHeader(key, value); * } * } diff --git a/packages/angular/ssr/src/routes/ng-routes.ts b/packages/angular/ssr/src/routes/ng-routes.ts index 583fa0000ece..b4b3ddda39ce 100644 --- a/packages/angular/ssr/src/routes/ng-routes.ts +++ b/packages/angular/ssr/src/routes/ng-routes.ts @@ -77,6 +77,11 @@ interface AngularRouterConfigResult { * If not provided, the default configuration or behavior will be used. */ serverRoutesConfig?: ServerRoute[] | null; + + /** + * A list of errors encountered during the route extraction process. + */ + errors: string[]; } /** @@ -86,9 +91,9 @@ interface AngularRouterConfigResult { * and lazy-loaded routes. It yields route metadata for each route and its potential variants. * * @param options - The configuration options for traversing routes. - * @returns An async iterable iterator of route tree node metadata. + * @returns An async iterable iterator yielding either route tree node metadata or an error object with an error message. */ -export async function* traverseRoutesConfig({ +async function* traverseRoutesConfig({ routes, compiler, parentInjector, @@ -102,108 +107,103 @@ export async function* traverseRoutesConfig({ parentRoute: string; serverConfigRouteTree: RouteTree | undefined; invokeGetPrerenderParams: boolean; -}): AsyncIterableIterator { +}): AsyncIterableIterator { for (const route of routes) { - const { path = '', redirectTo, loadChildren, children } = route; - const currentRoutePath = joinUrlParts(parentRoute, path); - - // Get route metadata from the server config route tree, if available - const metadata: ServerConfigRouteTreeNodeMetadata = { - ...(serverConfigRouteTree - ? getMatchedRouteMetadata(serverConfigRouteTree, currentRoutePath) - : undefined), - route: currentRoutePath, - }; - - // Handle redirects - if (typeof redirectTo === 'string') { - const redirectToResolved = resolveRedirectTo(currentRoutePath, redirectTo); - if (metadata.status && !VALID_REDIRECT_RESPONSE_CODES.has(metadata.status)) { - throw new Error( - `The '${metadata.status}' status code is not a valid redirect response code. ` + - `Please use one of the following redirect response codes: ${[...VALID_REDIRECT_RESPONSE_CODES.values()].join(', ')}.`, - ); + try { + const { path = '', redirectTo, loadChildren, children } = route; + const currentRoutePath = joinUrlParts(parentRoute, path); + + // Get route metadata from the server config route tree, if available + let matchedMetaData: ServerConfigRouteTreeNodeMetadata | undefined; + if (serverConfigRouteTree) { + matchedMetaData = serverConfigRouteTree.match(currentRoutePath); + if (!matchedMetaData) { + yield { + error: + `The '${currentRoutePath}' route does not match any route defined in the server routing configuration. ` + + 'Please ensure this route is added to the server routing configuration.', + }; + + continue; + } } - yield { ...metadata, redirectTo: redirectToResolved }; - } else if (metadata.renderMode === RenderMode.Prerender) { - // Handle SSG routes - yield* handleSSGRoute(metadata, parentInjector, invokeGetPrerenderParams); - } else { - yield metadata; - } - // Recursively process child routes - if (children?.length) { - yield* traverseRoutesConfig({ - routes: children, - compiler, - parentInjector, - parentRoute: currentRoutePath, - serverConfigRouteTree, - invokeGetPrerenderParams, - }); - } - - // Load and process lazy-loaded child routes - if (loadChildren) { - const loadedChildRoutes = await loadChildrenHelper( - route, - compiler, - parentInjector, - ).toPromise(); + const metadata: ServerConfigRouteTreeNodeMetadata = { + ...matchedMetaData, + route: currentRoutePath, + }; + + // Handle redirects + if (typeof redirectTo === 'string') { + const redirectToResolved = resolveRedirectTo(currentRoutePath, redirectTo); + if (metadata.status && !VALID_REDIRECT_RESPONSE_CODES.has(metadata.status)) { + yield { + error: + `The '${metadata.status}' status code is not a valid redirect response code. ` + + `Please use one of the following redirect response codes: ${[...VALID_REDIRECT_RESPONSE_CODES.values()].join(', ')}.`, + }; + continue; + } + yield { ...metadata, redirectTo: redirectToResolved }; + } else if (metadata.renderMode === RenderMode.Prerender) { + // Handle SSG routes + yield* handleSSGRoute(metadata, parentInjector, invokeGetPrerenderParams); + } else { + yield metadata; + } - if (loadedChildRoutes) { - const { routes: childRoutes, injector = parentInjector } = loadedChildRoutes; + // Recursively process child routes + if (children?.length) { yield* traverseRoutesConfig({ - routes: childRoutes, + routes: children, compiler, - parentInjector: injector, + parentInjector, parentRoute: currentRoutePath, serverConfigRouteTree, invokeGetPrerenderParams, }); } - } - } -} -/** - * Retrieves the matched route metadata from the server configuration route tree. - * - * @param serverConfigRouteTree - The server configuration route tree. - * @param currentRoutePath - The current route path being processed. - * @returns The metadata associated with the matched route. - */ -function getMatchedRouteMetadata( - serverConfigRouteTree: RouteTree, - currentRoutePath: string, -): ServerConfigRouteTreeNodeMetadata { - const metadata = serverConfigRouteTree.match(currentRoutePath); - - if (!metadata) { - throw new Error( - `The '${currentRoutePath}' route does not match any route defined in the server routing configuration. ` + - 'Please ensure this route is added to the server routing configuration.', - ); + // Load and process lazy-loaded child routes + if (loadChildren) { + const loadedChildRoutes = await loadChildrenHelper( + route, + compiler, + parentInjector, + ).toPromise(); + + if (loadedChildRoutes) { + const { routes: childRoutes, injector = parentInjector } = loadedChildRoutes; + yield* traverseRoutesConfig({ + routes: childRoutes, + compiler, + parentInjector: injector, + parentRoute: currentRoutePath, + serverConfigRouteTree, + invokeGetPrerenderParams, + }); + } + } + } catch (error) { + yield { error: `Error processing route '${route.path}': ${(error as Error).message}` }; + } } - - return metadata; } /** * Handles SSG (Static Site Generation) routes by invoking `getPrerenderParams` and yielding - * all parameterized paths. + * all parameterized paths, returning any errors encountered. * * @param metadata - The metadata associated with the route tree node. * @param parentInjector - The dependency injection container for the parent route. * @param invokeGetPrerenderParams - A flag indicating whether to invoke the `getPrerenderParams` function. - * @returns An async iterable iterator that yields route tree node metadata for each SSG path. + * @returns An async iterable iterator that yields route tree node metadata for each SSG path or errors. */ async function* handleSSGRoute( metadata: ServerConfigRouteTreeNodeMetadata, parentInjector: Injector, invokeGetPrerenderParams: boolean, -): AsyncIterableIterator { +): AsyncIterableIterator { if (metadata.renderMode !== RenderMode.Prerender) { throw new Error( `'handleSSGRoute' was called for a route which rendering mode is not prerender.`, @@ -217,34 +217,52 @@ async function* handleSSGRoute( delete meta['getPrerenderParams']; } - if (invokeGetPrerenderParams && URL_PARAMETER_REGEXP.test(currentRoutePath)) { + if (!URL_PARAMETER_REGEXP.test(currentRoutePath)) { + // Route has no parameters + yield { + ...meta, + route: currentRoutePath, + }; + + return; + } + + if (invokeGetPrerenderParams) { if (!getPrerenderParams) { - throw new Error( - `The '${currentRoutePath}' route uses prerendering and includes parameters, but 'getPrerenderParams' is missing. ` + + yield { + error: + `The '${currentRoutePath}' route uses prerendering and includes parameters, but 'getPrerenderParams' is missing. ` + `Please define 'getPrerenderParams' function for this route in your server routing configuration ` + `or specify a different 'renderMode'.`, - ); + }; + + return; } const parameters = await runInInjectionContext(parentInjector, () => getPrerenderParams()); + try { + for (const params of parameters) { + const routeWithResolvedParams = currentRoutePath.replace(URL_PARAMETER_REGEXP, (match) => { + const parameterName = match.slice(1); + const value = params[parameterName]; + if (typeof value !== 'string') { + throw new Error( + `The 'getPrerenderParams' function defined for the '${currentRoutePath}' route ` + + `returned a non-string value for parameter '${parameterName}'. ` + + `Please make sure the 'getPrerenderParams' function returns values for all parameters ` + + 'specified in this route.', + ); + } + + return value; + }); - for (const params of parameters) { - const routeWithResolvedParams = currentRoutePath.replace(URL_PARAMETER_REGEXP, (match) => { - const parameterName = match.slice(1); - const value = params[parameterName]; - if (typeof value !== 'string') { - throw new Error( - `The 'getPrerenderParams' function defined for the '${currentRoutePath}' route ` + - `returned a non-string value for parameter '${parameterName}'. ` + - `Please make sure the 'getPrerenderParams' function returns values for all parameters ` + - 'specified in this route.', - ); - } - - return value; - }); + yield { ...meta, route: routeWithResolvedParams }; + } + } catch (error) { + yield { error: `${(error as Error).message}` }; - yield { ...meta, route: routeWithResolvedParams }; + return; } } @@ -286,17 +304,31 @@ function resolveRedirectTo(routePath: string, redirectTo: string): string { * Builds a server configuration route tree from the given server routes configuration. * * @param serverRoutesConfig - The array of server routes to be used for configuration. - * @returns A `RouteTree` populated with the server routes and their metadata. + + * @returns An object containing: + * - `serverConfigRouteTree`: A populated `RouteTree` instance, which organizes the server routes + * along with their additional metadata. + * - `errors`: An array of strings that list any errors encountered during the route tree construction + * process, such as invalid paths. */ -function buildServerConfigRouteTree( - serverRoutesConfig: ServerRoute[], -): RouteTree { +function buildServerConfigRouteTree(serverRoutesConfig: ServerRoute[]): { + errors: string[]; + serverConfigRouteTree: RouteTree; +} { const serverConfigRouteTree = new RouteTree(); + const errors: string[] = []; + for (const { path, ...metadata } of serverRoutesConfig) { + if (path[0] === '/') { + errors.push(`Invalid '${path}' route configuration: the path cannot start with a slash.`); + + continue; + } + serverConfigRouteTree.insert(path, metadata); } - return serverConfigRouteTree; + return { serverConfigRouteTree, errors }; } /** @@ -304,7 +336,7 @@ function buildServerConfigRouteTree( * * This function initializes an Angular platform, bootstraps the application or module, * and retrieves routes from the Angular router configuration. It handles both module-based - * and function-based bootstrapping. It yields the resulting routes as `RouteTreeNodeMetadata` objects. + * and function-based bootstrapping. It yields the resulting routes as `RouteTreeNodeMetadata` objects or errors. * * @param bootstrap - A function that returns a promise resolving to an `ApplicationRef` or an Angular module to bootstrap. * @param document - The initial HTML document used for server-side rendering. @@ -313,10 +345,7 @@ function buildServerConfigRouteTree( * for ensuring that API requests for relative paths succeed, which is essential for accurate route extraction. * @param invokeGetPrerenderParams - A boolean flag indicating whether to invoke `getPrerenderParams` for parameterized SSG routes * to handle prerendering paths. Defaults to `false`. - * See: - * - https://github.com/angular/angular/blob/d608b857c689d17a7ffa33bbb510301014d24a17/packages/platform-server/src/location.ts#L51 - * - https://github.com/angular/angular/blob/6882cc7d9eed26d3caeedca027452367ba25f2b9/packages/platform-server/src/http.ts#L44 - * @returns A promise that resolves to an object of type `AngularRouterConfigResult`. + * @returns A promise that resolves to an object of type `AngularRouterConfigResult` or errors. */ export async function getRoutesFromAngularRouterConfig( bootstrap: AngularBootstrap, @@ -355,14 +384,31 @@ export async function getRoutesFromAngularRouterConfig( const injector = applicationRef.injector; const router = injector.get(Router); const routesResults: RouteTreeNodeMetadata[] = []; + const errors: string[] = []; + + const baseHref = + injector.get(APP_BASE_HREF, null, { optional: true }) ?? + injector.get(PlatformLocation).getBaseHrefFromDOM(); if (router.config.length) { const compiler = injector.get(Compiler); const serverRoutesConfig = injector.get(SERVER_ROUTES_CONFIG, null, { optional: true }); - const serverConfigRouteTree = serverRoutesConfig - ? buildServerConfigRouteTree(serverRoutesConfig) - : undefined; + let serverConfigRouteTree: RouteTree | undefined; + + if (serverRoutesConfig) { + const result = buildServerConfigRouteTree(serverRoutesConfig); + serverConfigRouteTree = result.serverConfigRouteTree; + errors.push(...result.errors); + } + + if (errors.length) { + return { + baseHref, + routes: routesResults, + errors, + }; + } // Retrieve all routes from the Angular router configuration. const traverseRoutes = traverseRoutesConfig({ @@ -375,19 +421,20 @@ export async function getRoutesFromAngularRouterConfig( }); for await (const result of traverseRoutes) { - routesResults.push(result); + if ('error' in result) { + errors.push(result.error); + } else { + routesResults.push(result); + } } } else { routesResults.push({ route: '', renderMode: RenderMode.Prerender }); } - const baseHref = - injector.get(APP_BASE_HREF, null, { optional: true }) ?? - injector.get(PlatformLocation).getBaseHrefFromDOM(); - return { baseHref, routes: routesResults, + errors, }; } finally { platformRef.destroy(); @@ -407,17 +454,20 @@ export async function getRoutesFromAngularRouterConfig( * If not provided, the default manifest is retrieved using `getAngularAppManifest()`. * @param invokeGetPrerenderParams - A boolean flag indicating whether to invoke `getPrerenderParams` for parameterized SSG routes * to handle prerendering paths. Defaults to `false`. - * @returns A promise that resolves to a populated `RouteTree` containing all extracted routes from the Angular application. + * + * @returns A promise that resolves to an object containing: + * - `routeTree`: A populated `RouteTree` containing all extracted routes from the Angular application. + * - `errors`: An array of strings representing any errors encountered during the route extraction process. */ export async function extractRoutesAndCreateRouteTree( url: URL, manifest: AngularAppManifest = getAngularAppManifest(), invokeGetPrerenderParams = false, -): Promise { +): Promise<{ routeTree: RouteTree; errors: string[] }> { const routeTree = new RouteTree(); const document = await new ServerAssets(manifest).getIndexServerHtml(); const bootstrap = await manifest.bootstrap(); - const { baseHref, routes } = await getRoutesFromAngularRouterConfig( + const { baseHref, routes, errors } = await getRoutesFromAngularRouterConfig( bootstrap, document, url, @@ -433,5 +483,8 @@ export async function extractRoutesAndCreateRouteTree( routeTree.insert(fullRoute, metadata); } - return routeTree; + return { + routeTree, + errors, + }; } diff --git a/packages/angular/ssr/src/routes/route-tree.ts b/packages/angular/ssr/src/routes/route-tree.ts index f3b07859cc28..8b8db243b21e 100644 --- a/packages/angular/ssr/src/routes/route-tree.ts +++ b/packages/angular/ssr/src/routes/route-tree.ts @@ -131,8 +131,7 @@ export class RouteTree = {}> */ insert(route: string, metadata: RouteTreeNodeMetadataWithoutRoute & AdditionalMetadata): void { let node = this.root; - const normalizedRoute = stripTrailingSlash(route); - const segments = normalizedRoute.split('/'); + const segments = this.getPathSegments(route); for (const segment of segments) { // Replace parameterized segments (e.g., :id) with a wildcard (*) for matching @@ -150,7 +149,7 @@ export class RouteTree = {}> // At the leaf node, store the full route and its associated metadata node.metadata = { ...metadata, - route: normalizedRoute, + route: segments.join('/'), }; node.insertionIndex = this.insertionIndexCounter++; @@ -165,7 +164,7 @@ export class RouteTree = {}> * @returns The metadata of the best matching route or `undefined` if no match is found. */ match(route: string): (RouteTreeNodeMetadata & AdditionalMetadata) | undefined { - const segments = stripTrailingSlash(route).split('/'); + const segments = this.getPathSegments(route); return this.traverseBySegments(segments)?.metadata; } @@ -218,6 +217,16 @@ export class RouteTree = {}> } } + /** + * Extracts the path segments from a given route string. + * + * @param route - The route string from which to extract segments. + * @returns An array of path segments. + */ + private getPathSegments(route: string): string[] { + return stripTrailingSlash(route).split('/'); + } + /** * Recursively traverses the route tree from a given node, attempting to match the remaining route segments. * If the node is a leaf node (no more segments to match) and contains metadata, the node is yielded. diff --git a/packages/angular/ssr/src/routes/router.ts b/packages/angular/ssr/src/routes/router.ts index b708356b331d..bd63fa729a82 100644 --- a/packages/angular/ssr/src/routes/router.ts +++ b/packages/angular/ssr/src/routes/router.ts @@ -55,7 +55,16 @@ export class ServerRouter { // Create and store a new promise for the build process. // This prevents concurrent builds by re-using the same promise. ServerRouter.#extractionPromise ??= extractRoutesAndCreateRouteTree(url, manifest) - .then((routeTree) => new ServerRouter(routeTree)) + .then(({ routeTree, errors }) => { + if (errors.length > 0) { + throw new Error( + 'Error(s) occurred while extracting routes:\n' + + errors.map((error) => `- ${error}`).join('\n'), + ); + } + + return new ServerRouter(routeTree); + }) .finally(() => { ServerRouter.#extractionPromise = undefined; }); diff --git a/packages/angular/ssr/src/utils/url.ts b/packages/angular/ssr/src/utils/url.ts index 9d3d9dc4620d..db68ce3570e6 100644 --- a/packages/angular/ssr/src/utils/url.ts +++ b/packages/angular/ssr/src/utils/url.ts @@ -16,11 +16,49 @@ * ```js * stripTrailingSlash('path/'); // 'path' * stripTrailingSlash('/path'); // '/path' + * stripTrailingSlash('/'); // '/' + * stripTrailingSlash(''); // '' * ``` */ export function stripTrailingSlash(url: string): string { // Check if the last character of the URL is a slash - return url[url.length - 1] === '/' ? url.slice(0, -1) : url; + return url.length > 1 && url[url.length - 1] === '/' ? url.slice(0, -1) : url; +} + +/** + * Removes the leading slash from a URL if it exists. + * + * @param url - The URL string from which to remove the leading slash. + * @returns The URL string without a leading slash. + * + * @example + * ```js + * stripLeadingSlash('/path'); // 'path' + * stripLeadingSlash('/path/'); // 'path/' + * stripLeadingSlash('/'); // '/' + * stripLeadingSlash(''); // '' + * ``` + */ +export function stripLeadingSlash(url: string): string { + // Check if the first character of the URL is a slash + return url.length > 1 && url[0] === '/' ? url.slice(1) : url; +} + +/** + * Adds a leading slash to a URL if it does not already have one. + * + * @param url - The URL string to which the leading slash will be added. + * @returns The URL string with a leading slash. + * + * @example + * ```js + * addLeadingSlash('path'); // '/path' + * addLeadingSlash('/path'); // '/path' + * ``` + */ +export function addLeadingSlash(url: string): string { + // Check if the URL already starts with a slash + return url[0] === '/' ? url : `/${url}`; } /** @@ -36,12 +74,11 @@ export function stripTrailingSlash(url: string): string { * ```js * joinUrlParts('path/', '/to/resource'); // '/path/to/resource' * joinUrlParts('/path/', 'to/resource'); // '/path/to/resource' + * joinUrlParts('', ''); // '/' * ``` */ export function joinUrlParts(...parts: string[]): string { - // Initialize an array with an empty string to always add a leading slash - const normalizeParts: string[] = ['']; - + const normalizeParts: string[] = []; for (const part of parts) { if (part === '') { // Skip any empty parts @@ -60,7 +97,7 @@ export function joinUrlParts(...parts: string[]): string { } } - return normalizeParts.join('/'); + return addLeadingSlash(normalizeParts.join('/')); } /** diff --git a/packages/angular/ssr/test/app-engine_spec.ts b/packages/angular/ssr/test/app-engine_spec.ts index 7384f62456c1..0e27776a368e 100644 --- a/packages/angular/ssr/test/app-engine_spec.ts +++ b/packages/angular/ssr/test/app-engine_spec.ts @@ -41,7 +41,7 @@ describe('AngularAppEngine', () => { setAngularAppTestingManifest( [{ path: 'home', component: HomeComponent }], - [{ path: '/**', renderMode: RenderMode.Server }], + [{ path: '**', renderMode: RenderMode.Server }], locale, ); @@ -150,7 +150,7 @@ describe('AngularAppEngine', () => { setAngularAppTestingManifest( [{ path: 'home', component: HomeComponent }], - [{ path: '/**', renderMode: RenderMode.Server }], + [{ path: '**', renderMode: RenderMode.Server }], ); return { diff --git a/packages/angular/ssr/test/app_spec.ts b/packages/angular/ssr/test/app_spec.ts index 82a70bd0219e..7e2df85fffa7 100644 --- a/packages/angular/ssr/test/app_spec.ts +++ b/packages/angular/ssr/test/app_spec.ts @@ -41,16 +41,16 @@ describe('AngularServerApp', () => { ], [ { - path: '/home-csr', + path: 'home-csr', renderMode: RenderMode.Client, }, { - path: '/page-with-status', + path: 'page-with-status', renderMode: RenderMode.Server, status: 201, }, { - path: '/page-with-headers', + path: 'page-with-headers', renderMode: RenderMode.Server, headers: { 'Cache-Control': 'no-cache', @@ -58,7 +58,7 @@ describe('AngularServerApp', () => { }, }, { - path: '/**', + path: '**', renderMode: RenderMode.Server, }, ], diff --git a/packages/angular/ssr/test/routes/ng-routes_spec.ts b/packages/angular/ssr/test/routes/ng-routes_spec.ts index da347f9768f0..556d051f7927 100644 --- a/packages/angular/ssr/test/routes/ng-routes_spec.ts +++ b/packages/angular/ssr/test/routes/ng-routes_spec.ts @@ -29,25 +29,61 @@ describe('extractRoutesAndCreateRouteTree', () => { it('should extract routes and create a route tree', async () => { setAngularAppTestingManifest( [ + { path: '', component: DummyComponent }, { path: 'home', component: DummyComponent }, { path: 'redirect', redirectTo: 'home' }, { path: 'user/:id', component: DummyComponent }, ], [ - { path: '/home', renderMode: RenderMode.Client }, - { path: '/redirect', renderMode: RenderMode.Server, status: 301 }, - { path: '/**', renderMode: RenderMode.Server }, + { path: 'home', renderMode: RenderMode.Client }, + { path: 'redirect', renderMode: RenderMode.Server, status: 301 }, + { path: '**', renderMode: RenderMode.Server }, ], ); - const routeTree = await extractRoutesAndCreateRouteTree(url); + const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url); + expect(errors).toHaveSize(0); expect(routeTree.toObject()).toEqual([ + { route: '/', renderMode: RenderMode.Server }, { route: '/home', renderMode: RenderMode.Client }, { route: '/redirect', renderMode: RenderMode.Server, status: 301, redirectTo: '/home' }, { route: '/user/:id', renderMode: RenderMode.Server }, ]); }); + it('should handle invalid route configuration path', async () => { + setAngularAppTestingManifest( + [{ path: 'home', component: DummyComponent }], + [ + // This path starts with a slash, which should trigger an error + { path: '/invalid', renderMode: RenderMode.Client }, + ], + ); + + const { errors } = await extractRoutesAndCreateRouteTree(url); + expect(errors[0]).toContain( + `Invalid '/invalid' route configuration: the path cannot start with a slash.`, + ); + }); + + it('should handle route not matching server routing configuration', async () => { + setAngularAppTestingManifest( + [ + { path: 'home', component: DummyComponent }, + { path: 'about', component: DummyComponent }, // This route is not in the server configuration + ], + [ + { path: 'home', renderMode: RenderMode.Client }, + // 'about' route is missing here + ], + ); + + const { errors } = await extractRoutesAndCreateRouteTree(url); + expect(errors[0]).toContain( + `The '/about' route does not match any route defined in the server routing configuration.`, + ); + }); + describe('when `invokeGetPrerenderParams` is true', () => { it('should resolve parameterized routes for SSG and add a fallback route if fallback is Server', async () => { setAngularAppTestingManifest( @@ -67,7 +103,8 @@ describe('extractRoutesAndCreateRouteTree', () => { ], ); - const routeTree = await extractRoutesAndCreateRouteTree(url, undefined, true); + const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url, undefined, true); + expect(errors).toHaveSize(0); expect(routeTree.toObject()).toEqual([ { route: '/user/joe/role/admin', renderMode: RenderMode.Prerender }, { @@ -96,11 +133,12 @@ describe('extractRoutesAndCreateRouteTree', () => { ]; }, }, - { path: '/**', renderMode: RenderMode.Server }, + { path: '**', renderMode: RenderMode.Server }, ], ); - const routeTree = await extractRoutesAndCreateRouteTree(url, undefined, true); + const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url, undefined, true); + expect(errors).toHaveSize(0); expect(routeTree.toObject()).toEqual([ { route: '/home', renderMode: RenderMode.Server }, { route: '/user/joe/role/admin', renderMode: RenderMode.Prerender }, @@ -130,11 +168,12 @@ describe('extractRoutesAndCreateRouteTree', () => { ]; }, }, - { path: '/**', renderMode: RenderMode.Server }, + { path: '**', renderMode: RenderMode.Server }, ], ); - const routeTree = await extractRoutesAndCreateRouteTree(url, undefined, true); + const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url, undefined, true); + expect(errors).toHaveSize(0); expect(routeTree.toObject()).toEqual([ { route: '/home', renderMode: RenderMode.Server }, { route: '/user/joe/role/admin', renderMode: RenderMode.Prerender }, @@ -163,11 +202,12 @@ describe('extractRoutesAndCreateRouteTree', () => { ]; }, }, - { path: '/**', renderMode: RenderMode.Server }, + { path: '**', renderMode: RenderMode.Server }, ], ); - const routeTree = await extractRoutesAndCreateRouteTree(url, undefined, false); + const { routeTree, errors } = await extractRoutesAndCreateRouteTree(url, undefined, false); + expect(errors).toHaveSize(0); expect(routeTree.toObject()).toEqual([ { route: '/home', renderMode: RenderMode.Server }, { route: '/user/:id/role/:role', renderMode: RenderMode.Server }, diff --git a/packages/angular/ssr/test/routes/router_spec.ts b/packages/angular/ssr/test/routes/router_spec.ts index dc87a8e593e6..4a64e0713eaf 100644 --- a/packages/angular/ssr/test/routes/router_spec.ts +++ b/packages/angular/ssr/test/routes/router_spec.ts @@ -37,8 +37,8 @@ describe('ServerRouter', () => { { path: 'user/:id', component: DummyComponent }, ], [ - { path: '/redirect', renderMode: RenderMode.Server, status: 301 }, - { path: '/**', renderMode: RenderMode.Server }, + { path: 'redirect', renderMode: RenderMode.Server, status: 301 }, + { path: '**', renderMode: RenderMode.Server }, ], ); diff --git a/packages/angular/ssr/test/utils/url_spec.ts b/packages/angular/ssr/test/utils/url_spec.ts index a081b064af27..2cebcf0c5b53 100644 --- a/packages/angular/ssr/test/utils/url_spec.ts +++ b/packages/angular/ssr/test/utils/url_spec.ts @@ -6,7 +6,13 @@ * found in the LICENSE file at https://angular.dev/license */ -import { joinUrlParts, stripIndexHtmlFromURL, stripTrailingSlash } from '../../src/utils/url'; // Adjust the import path as needed +import { + addLeadingSlash, + joinUrlParts, + stripIndexHtmlFromURL, + stripLeadingSlash, + stripTrailingSlash, +} from '../../src/utils/url'; describe('URL Utils', () => { describe('stripTrailingSlash', () => { @@ -23,7 +29,39 @@ describe('URL Utils', () => { }); it('should handle URL with only a trailing slash', () => { - expect(stripTrailingSlash('/')).toBe(''); + expect(stripTrailingSlash('/')).toBe('/'); + }); + }); + + describe('stripLeadingSlash', () => { + it('should remove leading slash from URL', () => { + expect(stripLeadingSlash('/path/')).toBe('path/'); + }); + + it('should not modify URL if no leading slash is present', () => { + expect(stripLeadingSlash('path/')).toBe('path/'); + }); + + it('should handle empty URL', () => { + expect(stripLeadingSlash('')).toBe(''); + }); + + it('should handle URL with only a leading slash', () => { + expect(stripLeadingSlash('/')).toBe('/'); + }); + }); + + describe('addLeadingSlash', () => { + it('should add a leading slash to a URL without one', () => { + expect(addLeadingSlash('path/')).toBe('/path/'); + }); + + it('should not modify URL if it already has a leading slash', () => { + expect(addLeadingSlash('/path/')).toBe('/path/'); + }); + + it('should handle empty URL', () => { + expect(addLeadingSlash('')).toBe('/'); }); }); @@ -39,6 +77,10 @@ describe('URL Utils', () => { it('should handle empty URL parts', () => { expect(joinUrlParts('', '', 'path', '', 'to/resource')).toBe('/path/to/resource'); }); + + it('should handle an all-empty URL parts', () => { + expect(joinUrlParts('', '')).toBe('/'); + }); }); describe('stripIndexHtmlFromURL', () => {