From 94396dac5aa13b797edce1aa45fe46cac011d394 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Thu, 31 Oct 2024 08:11:05 +0000 Subject: [PATCH] fixup! fix(@angular/ssr): enable serving of prerendered pages in the App Engine --- .../application/execute-post-bundle.ts | 1 - .../src/utils/server-rendering/manifest.ts | 6 +- packages/angular/ssr/node/src/app-engine.ts | 5 +- packages/angular/ssr/src/app-engine.ts | 5 +- packages/angular/ssr/src/app.ts | 149 ++++++++++++------ packages/angular/ssr/src/manifest.ts | 7 +- packages/angular/ssr/test/testing-utils.ts | 4 +- 7 files changed, 113 insertions(+), 64 deletions(-) diff --git a/packages/angular/build/src/builders/application/execute-post-bundle.ts b/packages/angular/build/src/builders/application/execute-post-bundle.ts index f1eb812bde9f..bb2fb2e17b4d 100644 --- a/packages/angular/build/src/builders/application/execute-post-bundle.ts +++ b/packages/angular/build/src/builders/application/execute-post-bundle.ts @@ -183,7 +183,6 @@ export async function executePostBundleSteps( } case RouteRenderMode.Server: case RouteRenderMode.Client: - case RouteRenderMode.AppShell: serializableRouteTreeNodeForManifest.push(metadata); break; diff --git a/packages/angular/build/src/utils/server-rendering/manifest.ts b/packages/angular/build/src/utils/server-rendering/manifest.ts index 77c40edcc466..ae8afb8fc875 100644 --- a/packages/angular/build/src/utils/server-rendering/manifest.ts +++ b/packages/angular/build/src/utils/server-rendering/manifest.ts @@ -6,9 +6,8 @@ * found in the LICENSE file at https://angular.dev/license */ +import { extname } from 'node:path'; import { - INDEX_HTML_CSR, - INDEX_HTML_SERVER, NormalizedApplicationBuildOptions, getLocaleBaseHref, } from '../../builders/application/options'; @@ -135,7 +134,8 @@ export function generateAngularServerAppManifest( ): string { const serverAssetsContent: string[] = []; for (const file of [...additionalHtmlOutputFiles.values(), ...outputFiles]) { - if (file.path.endsWith('.html') || (inlineCriticalCss && file.path.endsWith('.css'))) { + const extension = extname(file.path); + if (extension === '.html' || (inlineCriticalCss && extension === '.css')) { serverAssetsContent.push(`['${file.path}', async () => \`${escapeUnsafeChars(file.text)}\`]`); } } diff --git a/packages/angular/ssr/node/src/app-engine.ts b/packages/angular/ssr/node/src/app-engine.ts index 08563ec520e0..e5f1f2614530 100644 --- a/packages/angular/ssr/node/src/app-engine.ts +++ b/packages/angular/ssr/node/src/app-engine.ts @@ -71,9 +71,6 @@ export class AngularNodeAppEngine { async process(request: IncomingMessage, requestContext?: unknown): Promise { const webRequest = createWebRequestFromNodeRequest(request); - return ( - (await this.angularAppEngine.serve(webRequest)) ?? - (await this.angularAppEngine.render(webRequest, requestContext)) - ); + return this.angularAppEngine.process(webRequest, requestContext); } } diff --git a/packages/angular/ssr/src/app-engine.ts b/packages/angular/ssr/src/app-engine.ts index 0d5804b61b68..b00108bab4aa 100644 --- a/packages/angular/ssr/src/app-engine.ts +++ b/packages/angular/ssr/src/app-engine.ts @@ -103,11 +103,8 @@ export class AngularAppEngine { */ async process(request: Request, requestContext?: unknown): Promise { const serverApp = await this.getAngularServerAppForRequest(request); - if (!serverApp) { - return null; - } - return (await serverApp.serve(request)) ?? (await serverApp.render(request, requestContext)); + return serverApp ? serverApp.process(request, requestContext) : null; } /** diff --git a/packages/angular/ssr/src/app.ts b/packages/angular/ssr/src/app.ts index 8614ab346743..b0d897a83e4e 100644 --- a/packages/angular/ssr/src/app.ts +++ b/packages/angular/ssr/src/app.ts @@ -12,6 +12,7 @@ import { ServerAssets } from './assets'; import { Hooks } from './hooks'; import { getAngularAppManifest } from './manifest'; import { RenderMode } from './routes/route-config'; +import { RouteTreeNodeMetadata } from './routes/route-tree'; import { ServerRouter } from './routes/router'; import { sha256 } from './utils/crypto'; import { InlineCriticalCssProcessor } from './utils/inline-critical-css'; @@ -21,9 +22,9 @@ import { joinUrlParts, stripIndexHtmlFromURL, stripLeadingSlash } from './utils/ /** * The default maximum age in seconds. - * Represents the total number of seconds in a 30-day period. + * Represents the total number of seconds in a 365-day period. */ -const DEFAULT_MAX_AGE = 30 * 24 * 60 * 60; +const DEFAULT_MAX_AGE = 365 * 24 * 60 * 60; /** * Maximum number of critical CSS entries the cache can store. @@ -107,10 +108,7 @@ export class AngularServerApp { * @returns A promise that resolves to the HTTP response object resulting from the rendering, or null if no match is found. */ render(request: Request, requestContext?: unknown): Promise { - return Promise.race([ - this.createAbortPromise(request), - this.handleRendering(request, /** isSsrMode */ true, requestContext), - ]); + return this.handleAbortableRendering(request, /** isSsrMode */ true, undefined, requestContext); } /** @@ -125,10 +123,7 @@ export class AngularServerApp { renderStatic(url: URL, signal?: AbortSignal): Promise { const request = new Request(url, { signal }); - return Promise.race([ - this.createAbortPromise(request), - this.handleRendering(request, /** isSsrMode */ false), - ]); + return this.handleAbortableRendering(request, /** isSsrMode */ false); } /** @@ -143,10 +138,62 @@ export class AngularServerApp { * if available, or `null` if the request does not match a prerendered route or asset. */ async serve(request: Request): Promise { - const url = stripIndexHtmlFromURL(new URL(request.url)); + return this.handleServe(request); + } + + /** + * Handles incoming HTTP requests by serving prerendered content or rendering the page. + * + * This method first attempts to serve a prerendered page. If the prerendered page is not available, + * it falls back to rendering the requested page using server-side rendering. The function returns + * a promise that resolves to the appropriate HTTP response. + * + * @param request - The incoming HTTP request to be processed. + * @param requestContext - Optional additional context for rendering, such as request metadata. + * @returns A promise that resolves to the HTTP response object resulting from the request handling, + * or null if no matching content is found. + */ + async process(request: Request, requestContext?: unknown): Promise { + const url = new URL(request.url); this.router ??= await ServerRouter.from(this.manifest, url); - const matchedRoute = this.router.match(new URL(request.url)); + const matchedRoute = this.router.match(url); + if (!matchedRoute) { + // Not a known Angular route. + return null; + } + + return ( + (await this.handleServe(request, matchedRoute)) ?? + this.handleAbortableRendering(request, /** isSsrMode */ true, matchedRoute, requestContext) + ); + } + + /** + * Retrieves the matched route for the incoming request based on the request URL. + * + * @param request - The incoming HTTP request to match against routes. + * @returns A promise that resolves to the matched route metadata or `undefined` if no route matches. + */ + private async getMatchedRoute(request: Request): Promise { + this.router ??= await ServerRouter.from(this.manifest, new URL(request.url)); + + return this.router.match(new URL(request.url)); + } + + /** + * Handles serving a prerendered static asset if available for the matched route. + * + * @param request - The incoming HTTP request for serving a static page. + * @param matchedRoute - Optional parameter representing the metadata of the matched route for rendering. + * If not provided, the method attempts to find a matching route based on the request URL. + * @returns A promise that resolves to a `Response` object if the prerendered page is found, or `null`. + */ + private async handleServe( + request: Request, + matchedRoute?: RouteTreeNodeMetadata, + ): Promise { + matchedRoute ??= await this.getMatchedRoute(request); if (!matchedRoute) { return null; } @@ -156,7 +203,8 @@ export class AngularServerApp { return null; } - const assetPath = stripLeadingSlash(joinUrlParts(url.pathname, 'index.html')); + const { pathname } = stripIndexHtmlFromURL(new URL(request.url)); + const assetPath = stripLeadingSlash(joinUrlParts(pathname, 'index.html')); if (!this.assets.hasServerAsset(assetPath)) { return null; } @@ -176,41 +224,43 @@ export class AngularServerApp { } /** - * Handles incoming HTTP requests by serving prerendered content or rendering the page. - * - * This method first attempts to serve a prerendered page. If the prerendered page is not available, - * it falls back to rendering the requested page using server-side rendering. The function returns - * a promise that resolves to the appropriate HTTP response. + * Handles the server-side rendering process for the given HTTP request, allowing for abortion + * of the rendering if the request is aborted. This method matches the request URL to a route + * and performs rendering if a matching route is found. * - * @param request - The incoming HTTP request to be processed. + * @param request - The incoming HTTP request to be processed. It includes a signal to monitor + * for abortion events. + * @param isSsrMode - A boolean indicating whether the rendering is performed in server-side + * rendering (SSR) mode. + * @param matchedRoute - Optional parameter representing the metadata of the matched route for + * rendering. If not provided, the method attempts to find a matching route based on the request URL. * @param requestContext - Optional additional context for rendering, such as request metadata. - * @returns A promise that resolves to the HTTP response object resulting from the request handling, - * or null if no matching content is found. - */ - async process(request: Request, requestContext?: unknown): Promise { - return (await this.serve(request)) ?? (await this.render(request, requestContext)); - } - - /** - * Creates a promise that rejects when the request is aborted. * - * @param request - The HTTP request to monitor for abortion. - * @returns A promise that never resolves but rejects with an `AbortError` if the request is aborted. + * @returns A promise that resolves to the rendered response, or null if no matching route is found. + * If the request is aborted, the promise will reject with an `AbortError`. */ - private createAbortPromise(request: Request): Promise { - return new Promise((_, reject) => { - request.signal.addEventListener( - 'abort', - () => { - const abortError = new Error( - `Request for: ${request.url} was aborted.\n${request.signal.reason}`, - ); - abortError.name = 'AbortError'; - reject(abortError); - }, - { once: true }, - ); - }); + private async handleAbortableRendering( + request: Request, + isSsrMode: boolean, + matchedRoute?: RouteTreeNodeMetadata, + requestContext?: unknown, + ): Promise { + return Promise.race([ + new Promise((_, reject) => { + request.signal.addEventListener( + 'abort', + () => { + const abortError = new Error( + `Request for: ${request.url} was aborted.\n${request.signal.reason}`, + ); + abortError.name = 'AbortError'; + reject(abortError); + }, + { once: true }, + ); + }), + this.handleRendering(request, isSsrMode, matchedRoute, requestContext), + ]); } /** @@ -219,6 +269,8 @@ export class AngularServerApp { * * @param request - The incoming HTTP request to be processed. * @param isSsrMode - A boolean indicating whether the rendering is performed in server-side rendering (SSR) mode. + * @param matchedRoute - Optional parameter representing the metadata of the matched route for rendering. + * If not provided, the method attempts to find a matching route based on the request URL. * @param requestContext - Optional additional context for rendering, such as request metadata. * * @returns A promise that resolves to the rendered response, or null if no matching route is found. @@ -226,18 +278,17 @@ export class AngularServerApp { private async handleRendering( request: Request, isSsrMode: boolean, + matchedRoute?: RouteTreeNodeMetadata, requestContext?: unknown, ): Promise { - const url = new URL(request.url); - this.router ??= await ServerRouter.from(this.manifest, url); - - const matchedRoute = this.router.match(url); + matchedRoute ??= await this.getMatchedRoute(request); if (!matchedRoute) { - // Not a known Angular route. return null; } const { redirectTo, status } = matchedRoute; + const url = new URL(request.url); + if (redirectTo !== undefined) { // Note: The status code is validated during route extraction. // 302 Found is used by default for redirections diff --git a/packages/angular/ssr/src/manifest.ts b/packages/angular/ssr/src/manifest.ts index d22396029ddd..004c7e82bbb5 100644 --- a/packages/angular/ssr/src/manifest.ts +++ b/packages/angular/ssr/src/manifest.ts @@ -9,6 +9,11 @@ import type { SerializableRouteTreeNode } from './routes/route-tree'; import { AngularBootstrap } from './utils/ng'; +/** + * A function that returns a promise resolving to the file contents of the asset. + */ +export type ServerAsset = () => Promise; + /** * Represents the exports of an Angular server application entry point. */ @@ -55,7 +60,7 @@ export interface AngularAppManifest { * - `key`: The path of the asset. * - `value`: A function returning a promise that resolves to the file contents of the asset. */ - readonly assets: ReadonlyMap Promise>; + readonly assets: ReadonlyMap; /** * The bootstrap mechanism for the server application. diff --git a/packages/angular/ssr/test/testing-utils.ts b/packages/angular/ssr/test/testing-utils.ts index 7c755c25e64e..005b108585d3 100644 --- a/packages/angular/ssr/test/testing-utils.ts +++ b/packages/angular/ssr/test/testing-utils.ts @@ -10,7 +10,7 @@ import { Component, provideExperimentalZonelessChangeDetection } from '@angular/ import { bootstrapApplication } from '@angular/platform-browser'; import { provideServerRendering } from '@angular/platform-server'; import { RouterOutlet, Routes, provideRouter } from '@angular/router'; -import { AngularAppManifest, setAngularAppManifest } from '../src/manifest'; +import { AngularAppManifest, ServerAsset, setAngularAppManifest } from '../src/manifest'; import { ServerRoute, provideServerRoutesConfig } from '../src/routes/route-config'; /** @@ -27,7 +27,7 @@ export function setAngularAppTestingManifest( routes: Routes, serverRoutes: ServerRoute[], baseHref = '', - additionalServerAssets: Record Promise> = {}, + additionalServerAssets: Record = {}, ): void { setAngularAppManifest({ inlineCriticalCss: false,