From b182be8aa7ff5fd3cddc0bcac5f4e45e9ed9cf2e Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 2 Nov 2023 09:56:41 -0400 Subject: [PATCH] fix(@angular-devkit/build-angular): avoid in-memory prerendering ESM loader errors The in-memory ESM loader hooks have been adjusted to avoid potential errors when resolving and loading the bundled server code during prerendering. These errors could result in hard to diagnose build failures. E2E testing via the deprecated protractor builder, would silently exit, for instance. To ensure on disk files including node modules are resolved from the workspace root, a virtual file root is used for all in memory files. This path does not actually exist but is used to overlay the in memory files with the actual filesystem for resolution purposes. A custom URL schema (such as `memory://`) cannot be used for the resolve output because the in-memory files may use `import.meta.url` in ways that assume a file URL. `createRequire` from the Node.js `module` builtin is one example of this usage. (cherry picked from commit f06a760344ee5271296d5007bcf79ba5cc97c685) --- .../esm-in-memory-loader/loader-hooks.ts | 121 ++++++++++++------ .../utils/server-rendering/render-worker.ts | 2 + .../routes-extractor-worker.ts | 7 +- 3 files changed, 91 insertions(+), 39 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/utils/server-rendering/esm-in-memory-loader/loader-hooks.ts b/packages/angular_devkit/build_angular/src/utils/server-rendering/esm-in-memory-loader/loader-hooks.ts index 94e1dcde9e0f..d6a2448984f2 100644 --- a/packages/angular_devkit/build_angular/src/utils/server-rendering/esm-in-memory-loader/loader-hooks.ts +++ b/packages/angular_devkit/build_angular/src/utils/server-rendering/esm-in-memory-loader/loader-hooks.ts @@ -6,7 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -import { join, relative } from 'node:path'; +import assert from 'node:assert'; +import { randomUUID } from 'node:crypto'; +import { join } from 'node:path'; import { pathToFileURL } from 'node:url'; import { fileURLToPath } from 'url'; import { JavaScriptTransformer } from '../../../tools/esbuild/javascript-transformer'; @@ -17,14 +19,14 @@ import { callInitializeIfNeeded } from './node-18-utils'; * @see: https://nodejs.org/api/esm.html#loaders for more information about loaders. */ +const MEMORY_URL_SCHEME = 'memory://'; + export interface ESMInMemoryFileLoaderWorkerData { outputFiles: Record; workspaceRoot: string; } -const TRANSFORMED_FILES: Record = {}; -const CHUNKS_REGEXP = /file:\/\/\/((?:main|render-utils)\.server|chunk-\w+)\.mjs/; -let workspaceRootFile: string; +let memoryVirtualRootUrl: string; let outputFiles: Record; const javascriptTransformer = new JavaScriptTransformer( @@ -38,7 +40,14 @@ const javascriptTransformer = new JavaScriptTransformer( callInitializeIfNeeded(initialize); export function initialize(data: ESMInMemoryFileLoaderWorkerData) { - workspaceRootFile = pathToFileURL(join(data.workspaceRoot, 'index.mjs')).href; + // This path does not actually exist but is used to overlay the in memory files with the + // actual filesystem for resolution purposes. + // A custom URL schema (such as `memory://`) cannot be used for the resolve output because + // the in-memory files may use `import.meta.url` in ways that assume a file URL. + // `createRequire` is one example of this usage. + memoryVirtualRootUrl = pathToFileURL( + join(data.workspaceRoot, `.angular/prerender-root/${randomUUID()}/`), + ).href; outputFiles = data.outputFiles; } @@ -47,49 +56,93 @@ export function resolve( context: { parentURL: undefined | string }, nextResolve: Function, ) { - if (!isFileProtocol(specifier)) { - const normalizedSpecifier = specifier.replace(/^\.\//, ''); - if (normalizedSpecifier in outputFiles) { + // In-memory files loaded from external code will contain a memory scheme + if (specifier.startsWith(MEMORY_URL_SCHEME)) { + let memoryUrl; + try { + memoryUrl = new URL(specifier); + } catch { + assert.fail('External code attempted to use malformed memory scheme: ' + specifier); + } + + // Resolve with a URL based from the virtual filesystem root + return { + format: 'module', + shortCircuit: true, + url: new URL(memoryUrl.pathname.slice(1), memoryVirtualRootUrl).href, + }; + } + + // Use next/default resolve if the parent is not from the virtual root + if (!context.parentURL?.startsWith(memoryVirtualRootUrl)) { + return nextResolve(specifier, context); + } + + // Check for `./` and `../` relative specifiers + const isRelative = + specifier[0] === '.' && + (specifier[1] === '/' || (specifier[1] === '.' && specifier[2] === '/')); + + // Relative specifiers from memory file should be based from the parent memory location + if (isRelative) { + let specifierUrl; + try { + specifierUrl = new URL(specifier, context.parentURL); + } catch {} + + if ( + specifierUrl?.pathname && + Object.hasOwn(outputFiles, specifierUrl.href.slice(memoryVirtualRootUrl.length)) + ) { return { format: 'module', shortCircuit: true, - // File URLs need to absolute. In Windows these also need to include the drive. - // The `/` will be resolved to the drive letter. - url: pathToFileURL('/' + normalizedSpecifier).href, + url: specifierUrl.href, }; } + + assert.fail( + `In-memory ESM relative file should always exist: '${context.parentURL}' --> '${specifier}'`, + ); } + // Update the parent URL to allow for module resolution for the workspace. + // This handles bare specifiers (npm packages) and absolute paths. // Defer to the next hook in the chain, which would be the // Node.js default resolve if this is the last user-specified loader. - return nextResolve( - specifier, - isBundleEntryPointOrChunk(context) ? { ...context, parentURL: workspaceRootFile } : context, - ); + return nextResolve(specifier, { + ...context, + parentURL: new URL('index.js', memoryVirtualRootUrl).href, + }); } export async function load(url: string, context: { format?: string | null }, nextLoad: Function) { const { format } = context; - // CommonJs modules require no transformations and are not in memory. - if (format !== 'commonjs' && isFileProtocol(url)) { - const filePath = fileURLToPath(url); - // Remove '/' or drive letter for Windows that was added in the above 'resolve'. - let source = outputFiles[relative('/', filePath)] ?? TRANSFORMED_FILES[filePath]; + // Load the file from memory if the URL is based in the virtual root + if (url.startsWith(memoryVirtualRootUrl)) { + const source = outputFiles[url.slice(memoryVirtualRootUrl.length)]; + assert(source !== undefined, 'Resolved in-memory ESM file should always exist: ' + url); + + // In-memory files have already been transformer during bundling and can be returned directly + return { + format, + shortCircuit: true, + source, + }; + } - if (source === undefined) { - source = TRANSFORMED_FILES[filePath] = Buffer.from( - await javascriptTransformer.transformFile(filePath), - ).toString('utf-8'); - } + // Only module files potentially require transformation. Angular libraries that would + // need linking are ESM only. + if (format === 'module' && isFileProtocol(url)) { + const filePath = fileURLToPath(url); + const source = await javascriptTransformer.transformFile(filePath); - if (source !== undefined) { - return { - format, - shortCircuit: true, - source, - }; - } + return { + format, + shortCircuit: true, + source, + }; } // Let Node.js handle all other URLs. @@ -104,10 +157,6 @@ function handleProcessExit(): void { void javascriptTransformer.close(); } -function isBundleEntryPointOrChunk(context: { parentURL: undefined | string }): boolean { - return !!context.parentURL && CHUNKS_REGEXP.test(context.parentURL); -} - process.once('exit', handleProcessExit); process.once('SIGINT', handleProcessExit); process.once('uncaughtException', handleProcessExit); diff --git a/packages/angular_devkit/build_angular/src/utils/server-rendering/render-worker.ts b/packages/angular_devkit/build_angular/src/utils/server-rendering/render-worker.ts index 8007986454f9..e5c71d31d441 100644 --- a/packages/angular_devkit/build_angular/src/utils/server-rendering/render-worker.ts +++ b/packages/angular_devkit/build_angular/src/utils/server-rendering/render-worker.ts @@ -7,6 +7,7 @@ */ import { workerData } from 'node:worker_threads'; +import { loadEsmModule } from '../load-esm'; import type { ESMInMemoryFileLoaderWorkerData } from './esm-in-memory-loader/loader-hooks'; import { patchFetchToLoadInMemoryAssets } from './fetch-patch'; import { RenderResult, ServerContext, renderPage } from './render-page'; @@ -34,6 +35,7 @@ function render(options: RenderOptions): Promise { outputFiles, document, inlineCriticalCss, + loadBundle: async (path) => await loadEsmModule(new URL(path, 'memory://')), }); } diff --git a/packages/angular_devkit/build_angular/src/utils/server-rendering/routes-extractor-worker.ts b/packages/angular_devkit/build_angular/src/utils/server-rendering/routes-extractor-worker.ts index 91c177559f2a..36b46e3fcaed 100644 --- a/packages/angular_devkit/build_angular/src/utils/server-rendering/routes-extractor-worker.ts +++ b/packages/angular_devkit/build_angular/src/utils/server-rendering/routes-extractor-worker.ts @@ -31,10 +31,11 @@ const { document, verbose } = workerData as RoutesExtractorWorkerData; /** Renders an application based on a provided options. */ async function extractRoutes(): Promise { const { extractRoutes } = await loadEsmModule( - './render-utils.server.mjs', + new URL('./render-utils.server.mjs', 'memory://'), + ); + const { default: bootstrapAppFnOrModule } = await loadEsmModule( + new URL('./main.server.mjs', 'memory://'), ); - const { default: bootstrapAppFnOrModule } = - await loadEsmModule('./main.server.mjs'); const skippedRedirects: string[] = []; const skippedOthers: string[] = [];