FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(@angular/build): fail build and exclude routes when prerendering … · angular/angular-cli@5b850fd · GitHub

  1. angular
  2. angular-cli

Repository navigation

Commit 5b850fd

Browse files
alan-agius4
committed
fix(@angular/build): fail build and exclude routes when prerendering fails
When a prerendered route failed to render (such as when a component throws during route activation), the render worker returned null content which was silently skipped. Consequently, no HTML file was written, but the build still reported the route in prerender statistics, included it in prerendered-routes.json, and exited with code 0. Now: - The render worker throws an error if content is null ('The content returned was empty.'). - Prerendering records the error so the build fails with a non-zero exit code. - Prerendered routes recorded for manifest and statistics are derived strictly from routes that produced output files. Closes #33965
1 parent 4fa81d4 commit 5b850fd

4 files changed

Lines changed: 138 additions & 33 deletions

File tree

‎packages/angular/build/src/builders/application/execute-post-bundle.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,13 @@ export async function executePostBundleSteps(
157157
'The "index" option is required when using the "ssg" or "appShell" options.',
158158
);
159159

160-
const { output, warnings, errors, serializableRouteTreeNode } = await prerenderPages(
160+
const {
161+
output,
162+
warnings,
163+
errors,
164+
serializableRouteTreeNode,
165+
prerenderedRoutes: generatedPrerenderedRoutes,
166+
} = await prerenderPages(
161167
workspaceRoot,
162168
baseHref,
163169
appShellOptions,
@@ -171,6 +177,7 @@ export async function executePostBundleSteps(
171177

172178
allErrors.push(...errors);
173179
allWarnings.push(...warnings);
180+
Object.assign(prerenderedRoutes, generatedPrerenderedRoutes);
174181

175182
const indexHasBeenPrerendered = output[indexHtmlOptions.output];
176183
for (const [path, { content, appShellRoute }] of Object.entries(output)) {
@@ -195,10 +202,6 @@ export async function executePostBundleSteps(
195202
const serializableRouteTreeNodeForManifest: WritableSerializableRouteTreeNode = [];
196203
for (const metadata of serializableRouteTreeNode) {
197204
serializableRouteTreeNodeForManifest.push(metadata);
198-
199-
if (metadata.renderMode === RouteRenderMode.Prerender && !metadata.route.includes('*')) {
200-
prerenderedRoutes[metadata.route] = { headers: metadata.headers };
201-
}
202205
}
203206

204207
if (outputMode === OutputMode.Server) {

‎packages/angular/build/src/utils/server-rendering/prerender.ts‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import { readFile } from 'node:fs/promises';
1010
import { extname, posix } from 'node:path';
1111
import { NormalizedApplicationBuildOptions } from '../../builders/application/options';
1212
import { OutputMode } from '../../builders/application/schema';
13-
import { BuildOutputAsset } from '../../tools/esbuild/bundler-execution-result';
13+
import {
14+
BuildOutputAsset,
15+
PrerenderedRoutesRecord,
16+
} from '../../tools/esbuild/bundler-execution-result';
1417
import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-files';
1518
import { assertIsError } from '../error';
1619
import { toPosixPath } from '../path';
@@ -65,6 +68,7 @@ export async function prerenderPages(
6568
output: PrerenderOutput;
6669
warnings: string[];
6770
errors: string[];
71+
prerenderedRoutes: PrerenderedRoutesRecord;
6872
serializableRouteTreeNode: SerializableRouteTreeNode;
6973
}> {
7074
const rawOutputFiles: Record<string, string> = {};
@@ -167,6 +171,7 @@ export async function prerenderPages(
167171
errors,
168172
warnings,
169173
output: {},
174+
prerenderedRoutes: {},
170175
serializableRouteTreeNode,
171176
};
172177
}
@@ -200,10 +205,22 @@ export async function prerenderPages(
200205

201206
errors.push(...renderingErrors);
202207

208+
const prerenderedRoutes: PrerenderedRoutesRecord = {};
209+
const baseHrefPathnameWithLeadingSlash = new URL(baseHref, 'http://localhost').pathname;
210+
211+
for (const metadata of serializableRouteTreeNodeForPrerender) {
212+
const outPath = getRouteOutPath(metadata.route, baseHrefPathnameWithLeadingSlash);
213+
214+
if (output[outPath]) {
215+
prerenderedRoutes[metadata.route] = { headers: metadata.headers };
216+
}
217+
}
218+
203219
return {
204220
errors,
205221
warnings,
206222
output,
223+
prerenderedRoutes,
207224
serializableRouteTreeNode,
208225
};
209226
}
@@ -227,22 +244,15 @@ async function renderPages(
227244

228245
const baseHrefPathnameWithLeadingSlash = new URL(baseHref, 'http://localhost').pathname;
229246
const appShellRouteWithoutBaseHref = appShellRoute
230-
? addTrailingSlash(appShellRoute).startsWith(baseHrefPathnameWithLeadingSlash)
231-
? addLeadingSlash(appShellRoute.slice(baseHrefPathnameWithLeadingSlash.length))
232-
: addLeadingSlash(appShellRoute)
247+
? addLeadingSlash(getRouteWithoutBaseHref(appShellRoute, baseHrefPathnameWithLeadingSlash))
233248
: undefined;
234249

235250
const routesToRender: { route: string; outPath: string; isAppShell: boolean }[] = [];
236251

237252
for (const { route, redirectTo } of serializableRouteTreeNode) {
238253
// Remove the base href from the file output path.
239-
const routeWithoutBaseHref = addTrailingSlash(route).startsWith(
240-
baseHrefPathnameWithLeadingSlash,
241-
)
242-
? addLeadingSlash(route.slice(baseHrefPathnameWithLeadingSlash.length))
243-
: route;
244-
245-
const outPath = stripLeadingSlash(posix.join(routeWithoutBaseHref, 'index.html'));
254+
const routeWithoutBaseHref = getRouteWithoutBaseHref(route, baseHrefPathnameWithLeadingSlash);
255+
const outPath = getRouteOutPath(route, baseHrefPathnameWithLeadingSlash);
246256

247257
if (typeof redirectTo === 'string') {
248258
output[outPath] = { content: generateRedirectStaticPage(redirectTo), appShellRoute: false };
@@ -305,20 +315,20 @@ async function renderPages(
305315
const renderBatchPromise: Promise<RenderResult> = renderWorker.run(urls);
306316
const batchResultPromise = renderBatchPromise
307317
.then((results) => {
308-
for (const { url, content, error } of results) {
309-
if (error) {
310-
errors.push(`An error occurred while prerendering route '${url}'.\n\n${error}`);
318+
for (const result of results) {
319+
if ('error' in result) {
320+
errors.push(
321+
`An error occurred while prerendering route '${result.url}'.\n\n${result.error}`,
322+
);
311323
continue;
312324
}
313325

314-
if (content !== null) {
315-
const routeInfo = routeOutPathMap.get(url);
316-
if (routeInfo) {
317-
output[routeInfo.outPath] = {
318-
content,
319-
appShellRoute: routeInfo.isAppShell,
320-
};
321-
}
326+
const routeInfo = routeOutPathMap.get(result.url);
327+
if (routeInfo) {
328+
output[routeInfo.outPath] = {
329+
content: result.content,
330+
appShellRoute: routeInfo.isAppShell,
331+
};
322332
}
323333
}
324334
})
@@ -439,3 +449,15 @@ async function getAllRoutes(
439449
void renderWorker.destroy();
440450
}
441451
}
452+
453+
function getRouteWithoutBaseHref(route: string, baseHrefPathname: string): string {
454+
return addTrailingSlash(route).startsWith(baseHrefPathname)
455+
? addLeadingSlash(route.slice(baseHrefPathname.length))
456+
: route;
457+
}
458+
459+
function getRouteOutPath(route: string, baseHrefPathname: string): string {
460+
const routeWithoutBaseHref = getRouteWithoutBaseHref(route, baseHrefPathname);
461+
462+
return stripLeadingSlash(posix.join(routeWithoutBaseHref, 'index.html'));
463+
}

‎packages/angular/build/src/utils/server-rendering/render-worker.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@ export interface RenderWorkerData extends ESMInMemoryFileLoaderWorkerData {
2121
hasSsrEntry: boolean;
2222
}
2323

24-
export interface RenderResultItem {
25-
url: string;
26-
content: string | null;
27-
error?: string;
28-
}
24+
export type RenderResultItem =
25+
| {
26+
url: string;
27+
content: string;
28+
}
29+
| {
30+
url: string;
31+
error: string;
32+
};
2933

3034
export type RenderResult = RenderResultItem[];
3135

@@ -74,12 +78,16 @@ async function renderPages(urls: string[]): Promise<RenderResult> {
7478
for (const currentUrl of urls) {
7579
try {
7680
const content = await renderPage(currentUrl, angularServerApp);
81+
82+
if (content === null) {
83+
throw new Error('The content returned was empty.');
84+
}
85+
7786
results.push({ url: currentUrl, content });
7887
} catch (err) {
7988
assertIsError(err);
8089
results.push({
8190
url: currentUrl,
82-
content: null,
8391
error: err.stack ?? err.message ?? err.code ?? `${err}`,
8492
});
8593
}

‎tests/e2e/tests/build/prerender/error-component-render.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { existsSync } from 'node:fs';
2+
import assert, { match } from 'node:assert';
3+
import { getGlobalVariable } from '../../../utils/env';
4+
import { expectFileNotToExist, readFile, rimraf, writeMultipleFiles } from '../../../utils/fs';
5+
import { installWorkspacePackages } from '../../../utils/packages';
6+
import { ng } from '../../../utils/process';
7+
import { useSha } from '../../../utils/project';
8+
import { expectToFail } from '../../../utils/utils';
9+
10+
export default async function () {
11+
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
12+
if (useWebpackBuilder) {
13+
return;
14+
}
15+
16+
// Forcibly remove in case another test doesn't clean itself up.
17+
await rimraf('node_modules/@angular/ssr');
18+
await ng('add', '@angular/ssr', '--skip-confirmation');
19+
await useSha();
20+
await installWorkspacePackages();
21+
22+
await writeMultipleFiles({
23+
'src/app/app.routes.ts': `
24+
import { Routes } from '@angular/router';
25+
import { Component } from '@angular/core';
26+
27+
@Component({
28+
selector: 'app-home',
29+
standalone: true,
30+
template: '<p>home works!</p>',
31+
})
32+
export class HomeRoute {}
33+
34+
@Component({
35+
selector: 'app-second',
36+
standalone: true,
37+
template: '<p>second works!</p>',
38+
})
39+
export class SecondRoute {
40+
constructor() {
41+
throw new Error('render failure');
42+
}
43+
}
44+
45+
export const routes: Routes = [
46+
{ path: '', component: HomeRoute },
47+
{ path: 'second', component: SecondRoute },
48+
];
49+
`,
50+
'src/app/app.routes.server.ts': `
51+
import { RenderMode, ServerRoute } from '@angular/ssr';
52+
53+
export const serverRoutes: ServerRoute[] = [
54+
{ path: 'second', renderMode: RenderMode.Prerender },
55+
{ path: '**', renderMode: RenderMode.Prerender },
56+
];
57+
`,
58+
});
59+
60+
const { message } = await expectToFail(() => ng('build', '--output-mode=server'));
61+
62+
match(message, /An error occurred while prerendering route '\/second'\./);
63+
64+
await expectFileNotToExist('dist/test-project/browser/second/index.html');
65+
66+
// prerendered-routes.json should only contain successfully prerendered routes if emitted
67+
const statsPath = 'dist/test-project/prerendered-routes.json';
68+
if (existsSync(statsPath)) {
69+
const stats = JSON.parse(await readFile(statsPath));
70+
assert.strictEqual(stats.routes['/second'], undefined);
71+
}
72+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL