Skip to content

Commit 8ad2547

Browse files
authored
fix(@angular/build): serialize server manifest asset paths
Serialize the values embedded in the generated server manifests with JSON.stringify, so a prerendered route such as ssg/customer's-choice no longer produces an invalid manifest.
1 parent 1816466 commit 8ad2547

4 files changed

Lines changed: 89 additions & 7 deletions

File tree

‎packages/angular/build/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ ts_project(
146146
":node_modules/@babel/core",
147147
"//:node_modules/@angular/compiler-cli",
148148
"//:node_modules/@types/jasmine",
149+
"//:node_modules/esbuild",
149150
"//:node_modules/prettier",
150151
"//:node_modules/typescript",
151152
"//packages/angular/build/private",

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export function generateAngularServerAppEngineManifest(
6969
for (const locale of i18nOptions.inlineLocales) {
7070
const { subPath } = i18nOptions.locales[locale];
7171
const importPath = `${subPath ? `${subPath}/` : ''}${MAIN_SERVER_OUTPUT_FILENAME}`;
72-
entryPoints[subPath] = `() => import('./${importPath}')`;
72+
entryPoints[subPath] = `() => import(${JSON.stringify(`./${importPath}`)})`;
7373
supportedLocales[locale] = subPath;
7474
}
7575
} else {
@@ -85,12 +85,12 @@ export function generateAngularServerAppEngineManifest(
8585

8686
const manifestContent = `
8787
export default {
88-
basePath: '${basePath}',
88+
basePath: ${JSON.stringify(basePath)},
8989
allowedHosts: ${JSON.stringify(allowedHosts, undefined, 2)},
9090
supportedLocales: ${JSON.stringify(supportedLocales, undefined, 2)},
9191
entryPoints: {
9292
${Object.entries(entryPoints)
93-
.map(([key, value]) => `'${key}': ${value}`)
93+
.map(([key, value]) => `${JSON.stringify(key)}: ${value}`)
9494
.join(',\n ')}
9595
},
9696
};
@@ -162,8 +162,11 @@ export function generateAngularServerAppManifest(
162162
// which can result in an incorrect byte length.
163163
const size = runInThisContext(`new TextEncoder().encode(\`${escapedContent}\`).byteLength`);
164164

165+
// Asset paths are derived from route paths and can contain arbitrary characters, so they are
166+
// serialized rather than interpolated into the generated executable manifest.
165167
serverAssets[file.path] =
166-
`{size: ${size}, hash: '${file.hash}', text: () => import('./${jsChunkFilePath}').then(m => m.default)}`;
168+
`{size: ${size}, hash: ${JSON.stringify(file.hash)}, ` +
169+
`text: () => import(${JSON.stringify(`./${jsChunkFilePath}`)}).then(m => m.default)}`;
167170
}
168171
}
169172

@@ -178,13 +181,13 @@ export function generateAngularServerAppManifest(
178181
export default {
179182
bootstrap: () => import('./main.server.mjs').then(m => m.default),
180183
inlineCriticalCss: ${inlineCriticalCss},
181-
baseHref: '${baseHref}',
184+
baseHref: ${JSON.stringify(baseHref)},
182185
locale: ${JSON.stringify(locale)},
183186
routes: ${JSON.stringify(routes, undefined, 2)},
184187
entryPointToBrowserMapping: ${JSON.stringify(entryPointToBrowserMapping, undefined, 2)},
185188
assets: {
186189
${Object.entries(serverAssets)
187-
.map(([key, value]) => `'${key}': ${value}`)
190+
.map(([key, value]) => `${JSON.stringify(key)}: ${value}`)
188191
.join(',\n ')}
189192
},
190193
};
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import type { Metafile } from 'esbuild';
10+
import { BuildOutputFileType } from '../../tools/esbuild/bundler-context';
11+
import { createOutputFile } from '../../tools/esbuild/utils';
12+
import { generateAngularServerAppManifest } from './manifest';
13+
14+
describe('generateAngularServerAppManifest', () => {
15+
const dummyMetafile = { inputs: {}, outputs: {} } as unknown as Metafile;
16+
17+
/**
18+
* Evaluates a generated manifest, which both asserts that it is syntactically valid JavaScript and
19+
* gives access to the values it declares. The dynamic imports it contains are never invoked.
20+
*/
21+
function evaluateManifest(manifestContent: string): unknown {
22+
return new Function(manifestContent.replace('export default', 'return'))();
23+
}
24+
25+
function generateManifest(
26+
htmlOutputFiles: Record<string, string>,
27+
baseHref = '/',
28+
): ReturnType<typeof generateAngularServerAppManifest> {
29+
const additionalHtmlOutputFiles = new Map(
30+
Object.entries(htmlOutputFiles).map(([path, content]) => [
31+
path,
32+
createOutputFile(path, content, BuildOutputFileType.Browser),
33+
]),
34+
);
35+
36+
return generateAngularServerAppManifest(
37+
additionalHtmlOutputFiles,
38+
[],
39+
false,
40+
undefined,
41+
undefined,
42+
baseHref,
43+
new Set(),
44+
dummyMetafile,
45+
undefined,
46+
);
47+
}
48+
49+
it('should serialize asset paths which contain JavaScript string delimiters', () => {
50+
const assetPath = "catalog/customer's-choice/index.html";
51+
const { manifestContent } = generateManifest({
52+
[assetPath]: '<main>Featured</main>',
53+
});
54+
55+
expect(evaluateManifest(manifestContent)).toEqual(
56+
jasmine.objectContaining({
57+
assets: {
58+
[assetPath]: jasmine.objectContaining({
59+
size: 21,
60+
hash: jasmine.any(String),
61+
text: jasmine.any(Function),
62+
}),
63+
},
64+
}),
65+
);
66+
});
67+
68+
it('should serialize a base href which contains JavaScript string delimiters', () => {
69+
const { manifestContent } = generateManifest({ 'index.html': '<main></main>' }, "/o'brien/");
70+
71+
expect(evaluateManifest(manifestContent)).toEqual(
72+
jasmine.objectContaining({
73+
baseHref: "/o'brien/",
74+
}),
75+
);
76+
});
77+
});

‎tests/e2e/tests/build/server-rendering/server-routes-output-mode-server.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export default async function () {
7777
path: 'ssg/:id',
7878
renderMode: RenderMode.Prerender,
7979
headers: { 'x-custom': 'ssg-with-params' },
80-
getPrerenderParams: async() => [{id: 'one'}, {id: 'two'}],
80+
getPrerenderParams: async() => [{id: 'one'}, {id: 'two'}, {id: "customer's-choice"}],
8181
},
8282
{
8383
path: 'ssr',
@@ -115,6 +115,7 @@ export default async function () {
115115
'ssg/index.html': 'ssg works!',
116116
'ssg/one/index.html': 'ssg-with-params works!',
117117
'ssg/two/index.html': 'ssg-with-params works!',
118+
"ssg/customer's-choice/index.html": 'ssg-with-params works!',
118119
};
119120

120121
for (const [filePath, fileMatch] of Object.entries(expects)) {

0 commit comments

Comments
 (0)