From 48657af8bbcc6d1121a99b54826d73e1f984febb Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Fri, 13 Oct 2023 07:27:08 +0000 Subject: [PATCH] fix(@angular-devkit/build-angular): resolve and load sourcemaps during prerendering to provide better stacktraces With this commit we running a build with sourcemaps enabled we enable Node.js Sourcemap support for stack traces. See: https://nodejs.org/dist/latest/docs/api/cli.html#--enable-source-maps for more information about this feature. Before ``` ERROR ReferenceError: window is not defined at new _AppComponent (file:///chunk-HFZN2HE2.mjs:41631:19) at NodeInjectorFactory.AppComponent_Factory [as factory] (file:///chunk-HFZN2HE2.mjs:41635:12) at getNodeInjectable (file:///chunk-HFZN2HE2.mjs:6104:38) at createRootComponent (file:///chunk-HFZN2HE2.mjs:10446:31) at ComponentFactory.create (file:///chunk-HFZN2HE2.mjs:10348:19) at _ApplicationRef.bootstrap (file:///chunk-HFZN2HE2.mjs:13247:40) at file:///chunk-HFZN2HE2.mjs:12938:20 at _ZoneDelegate.invoke (file:///chunk-HFZN2HE2.mjs:320:158) at Object.onInvoke (file:///chunk-HFZN2HE2.mjs:8562:25) at _ZoneDelegate.invoke (file:///chunk-HFZN2HE2.mjs:320:46) ``` Now ``` ERROR ReferenceError: window is not defined at constructor (/usr/xxxx/src/app/app.component.ts:16:17) at NodeInjectorFactory.AppComponent_Factory (/usr/xxxx/src/app/app.component.ts:17:3) at getNodeInjectable (/usr/xxxx/node_modules/@angular/core/fesm2022/core.mjs:4166:38) at createRootComponent (/usr/xxxx/node_modules/@angular/core/fesm2022/core.mjs:14064:31) at ComponentFactory.create (/usr/xxxx/node_modules/@angular/core/fesm2022/core.mjs:13931:19) at _ApplicationRef.bootstrap (/usr/xxxx/node_modules/@angular/core/fesm2022/core.mjs:30650:40) at (/usr/xxxx/node_modules/@angular/core/fesm2022/core.mjs:30163:20) at _ZoneDelegate.invoke (/usr/xxxx/node_modules/zone.js/fesm2015/zone-node.js:344:158) at Object.onInvoke (/usr/xxxx/node_modules/@angular/core/fesm2022/core.mjs:10964:25) at _ZoneDelegate.invoke (/usr/xxxx/node_modules/zone.js/fesm2015/zone-node.js:344:46) ``` Closes #26013 --- .../application/execute-post-bundle.ts | 2 + .../src/utils/server-rendering/prerender.ts | 50 ++++++++++++++++--- .../build/prerender/error-with-sourcemaps.ts | 41 +++++++++++++++ 3 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 tests/legacy-cli/e2e/tests/build/prerender/error-with-sourcemaps.ts diff --git a/packages/angular_devkit/build_angular/src/builders/application/execute-post-bundle.ts b/packages/angular_devkit/build_angular/src/builders/application/execute-post-bundle.ts index 0621c88eaa8b..cceb04d3b665 100644 --- a/packages/angular_devkit/build_angular/src/builders/application/execute-post-bundle.ts +++ b/packages/angular_devkit/build_angular/src/builders/application/execute-post-bundle.ts @@ -49,6 +49,7 @@ export async function executePostBundleSteps( serviceWorker, indexHtmlOptions, optimizationOptions, + sourcemapOptions, ssrOptions, prerenderOptions, appShellOptions, @@ -110,6 +111,7 @@ export async function executePostBundleSteps( prerenderOptions, outputFiles, indexContentOutputNoCssInlining, + sourcemapOptions.scripts, optimizationOptions.styles.inlineCritical, maxWorkers, verbose, diff --git a/packages/angular_devkit/build_angular/src/utils/server-rendering/prerender.ts b/packages/angular_devkit/build_angular/src/utils/server-rendering/prerender.ts index 66c052c34a55..83ca2cacdc4a 100644 --- a/packages/angular_devkit/build_angular/src/utils/server-rendering/prerender.ts +++ b/packages/angular_devkit/build_angular/src/utils/server-rendering/prerender.ts @@ -7,7 +7,7 @@ */ import { readFile } from 'node:fs/promises'; -import { extname, posix } from 'node:path'; +import { extname, join, posix } from 'node:path'; import Piscina from 'piscina'; import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context'; import { getESMLoaderArgs } from './esm-in-memory-loader/node-18-utils'; @@ -33,7 +33,8 @@ export async function prerenderPages( prerenderOptions: PrerenderOptions = {}, outputFiles: Readonly, document: string, - inlineCriticalCss?: boolean, + sourcemap = false, + inlineCriticalCss = false, maxThreads = 1, verbose = false, ): Promise<{ @@ -45,22 +46,48 @@ export async function prerenderPages( const warnings: string[] = []; const errors: string[] = []; const outputFilesForWorker: Record = {}; + const serverBundlesSourceMaps = new Map(); for (const { text, path, type } of outputFiles) { - if ( + const fileExt = extname(path); + if (type === BuildOutputFileType.Server && fileExt === '.map') { + serverBundlesSourceMaps.set(path.slice(0, -4), text); + } else if ( type === BuildOutputFileType.Server || // Contains the server runnable application code - (type === BuildOutputFileType.Browser && extname(path) === '.css') // Global styles for critical CSS inlining. + (type === BuildOutputFileType.Browser && fileExt === '.css') // Global styles for critical CSS inlining. ) { outputFilesForWorker[path] = text; } } + // Inline sourcemap into JS file. This is needed to make Node.js resolve sourcemaps + // when using `--enable-source-maps` when using in memory files. + for (const [filePath, map] of serverBundlesSourceMaps) { + const jsContent = outputFilesForWorker[filePath]; + if (jsContent) { + outputFilesForWorker[filePath] = + jsContent + + `\n//# sourceMappingURL=` + + `data:application/json;base64,${Buffer.from( + JSON.stringify({ + ...JSON.parse(map), + // Add a sourceRoot to enable navigation from the stacktrace to source file. + // This however causes absolute paths in stacktrace. + // TODO: clean stacktraces and rewrite paths to relative. + sourceRoot: join(workspaceRoot, '/'), + }), + ).toString('base64')}`; + } + } + serverBundlesSourceMaps.clear(); + const { routes: allRoutes, warnings: routesWarnings } = await getAllRoutes( workspaceRoot, outputFilesForWorker, document, appShellOptions, prerenderOptions, + sourcemap, verbose, ); @@ -76,6 +103,11 @@ export async function prerenderPages( }; } + const workerExecArgv = getESMLoaderArgs(); + if (sourcemap) { + workerExecArgv.push('--enable-source-maps'); + } + const renderWorker = new Piscina({ filename: require.resolve('./render-worker'), maxThreads: Math.min(allRoutes.size, maxThreads), @@ -85,7 +117,7 @@ export async function prerenderPages( inlineCriticalCss, document, } as RenderWorkerData, - execArgv: getESMLoaderArgs(), + execArgv: workerExecArgv, }); try { @@ -139,6 +171,7 @@ async function getAllRoutes( document: string, appShellOptions: AppShellOptions, prerenderOptions: PrerenderOptions, + sourcemap: boolean, verbose: boolean, ): Promise<{ routes: Set; warnings?: string[] }> { const { routesFile, discoverRoutes } = prerenderOptions; @@ -160,6 +193,11 @@ async function getAllRoutes( return { routes }; } + const workerExecArgv = getESMLoaderArgs(); + if (sourcemap) { + workerExecArgv.push('--enable-source-maps'); + } + const renderWorker = new Piscina({ filename: require.resolve('./routes-extractor-worker'), maxThreads: 1, @@ -169,7 +207,7 @@ async function getAllRoutes( document, verbose, } as RoutesExtractorWorkerData, - execArgv: getESMLoaderArgs(), + execArgv: workerExecArgv, }); const { routes: extractedRoutes, warnings }: RoutersExtractorWorkerResult = await renderWorker diff --git a/tests/legacy-cli/e2e/tests/build/prerender/error-with-sourcemaps.ts b/tests/legacy-cli/e2e/tests/build/prerender/error-with-sourcemaps.ts new file mode 100644 index 000000000000..1416afaa1d6d --- /dev/null +++ b/tests/legacy-cli/e2e/tests/build/prerender/error-with-sourcemaps.ts @@ -0,0 +1,41 @@ +import { ng } from '../../../utils/process'; +import { getGlobalVariable } from '../../../utils/env'; +import { rimraf, writeMultipleFiles } from '../../../utils/fs'; +import { match } from 'node:assert'; + +export default async function () { + const useWebpackBuilder = !getGlobalVariable('argv')['esbuild']; + if (useWebpackBuilder) { + return; + } + + // Forcibly remove in case another test doesn't clean itself up. + await rimraf('node_modules/@angular/ssr'); + await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install'); + + await writeMultipleFiles({ + 'src/app/app.component.ts': ` + import { Component } from '@angular/core'; + import { CommonModule } from '@angular/common'; + import { RouterOutlet } from '@angular/router'; + + @Component({ + selector: 'app-root', + standalone: true, + imports: [CommonModule, RouterOutlet], + templateUrl: './app.component.html', + styleUrls: ['./app.component.css'] + }) + export class AppComponent { + title = 'test-ssr'; + + constructor() { + console.log(window) + } + } + `, + }); + + const { stderr } = await ng('build', '--configuration', 'development', '--prerender'); + match(stderr, /window is not defined[.\s\S]*ngOnInit \(.*app\.component\.ts\:\d+:\d+\)/); +}