Skip to content

Commit ed7e320

Browse files
committed
fixup! fix(@angular/build): serialize server manifest asset paths
1 parent e5bde66 commit ed7e320

2 files changed

Lines changed: 52 additions & 115 deletions

File tree

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

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
BuildOutputFileType,
1919
createOutputFile,
2020
} from '../../tools/esbuild/bundler-files';
21-
import { calculateHash } from '../hash';
2221
import { findNonce } from '../index-file/nonce';
2322
import { joinUrlParts } from '../url';
2423

@@ -61,37 +60,6 @@ function escapeUnsafeChars(str: string): string {
6160
return str.replace(/[$`\\]/g, (c) => UNSAFE_CHAR_MAP[c]);
6261
}
6362

64-
/**
65-
* Matches every character which is not safe in the name of a generated server asset chunk.
66-
*/
67-
const UNSAFE_CHUNK_NAME_CHARACTER_REGEXP = /[^a-zA-Z0-9_-]/g;
68-
69-
/**
70-
* The maximum number of characters of an asset path kept in the name of its generated chunk.
71-
* The appended digest is what makes the name unique, so the readable part can be truncated to
72-
* stay well within the file name length limits of all supported platforms.
73-
*/
74-
const MAX_CHUNK_NAME_LENGTH = 128;
75-
76-
/**
77-
* Builds the path of the generated chunk which holds the content of a server asset.
78-
*
79-
* Asset paths are derived from route paths and can therefore contain characters which are unusable
80-
* in a file name (`?`, `:` and `*` are invalid on Windows) or which change how the generated
81-
* dynamic import is resolved (`?`, `#` and `%` are URL syntax). Those characters are replaced, and
82-
* a digest of the asset path is appended so that two asset paths never share a chunk.
83-
*
84-
* @param assetPath - The path of the asset, for example `store/summer sale/index.html`.
85-
* @returns The path of the chunk to generate for the asset.
86-
*/
87-
function generateServerAssetChunkPath(assetPath: string): string {
88-
const name = assetPath
89-
.replace(UNSAFE_CHUNK_NAME_CHARACTER_REGEXP, '_')
90-
.slice(0, MAX_CHUNK_NAME_LENGTH);
91-
92-
return `assets-chunks/${name}-${calculateHash(assetPath)}.mjs`;
93-
}
94-
9563
/**
9664
* Generates the server manifest for the App Engine environment.
9765
*
@@ -202,7 +170,7 @@ export async function generateAngularServerAppManifest(
202170
for (const file of [...additionalHtmlOutputFiles.values(), ...outputFiles]) {
203171
const extension = extname(file.path);
204172
if (extension === '.html') {
205-
const jsChunkFilePath = generateServerAssetChunkPath(file.path);
173+
const jsChunkFilePath = `assets-chunks/${file.path.replace(/[./]/g, '_')}.mjs`;
206174
const escapedContent = escapeUnsafeChars(file.text);
207175

208176
serverAssetsChunks.push(

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

Lines changed: 51 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -11,48 +11,45 @@ import { BuildOutputFileType, createOutputFile } from '../../tools/esbuild/bundl
1111
import { initializeHash } from '../hash';
1212
import { generateAngularServerAppManifest } from './manifest';
1313

14-
/**
15-
* Evaluates a generated manifest, which both asserts that it is syntactically valid JavaScript and
16-
* gives access to the values it declares. The dynamic imports it contains are never invoked.
17-
*/
18-
function evaluateManifest(manifestContent: string): Record<string, unknown> {
19-
return new Function(manifestContent.replace('export default', 'return'))() as Record<
20-
string,
21-
unknown
22-
>;
23-
}
24-
25-
const dummyMetafile = { inputs: {}, outputs: {} } as unknown as Metafile;
26-
27-
function generateManifest(
28-
htmlOutputFiles: Record<string, string>,
29-
baseHref = '/',
30-
): ReturnType<typeof generateAngularServerAppManifest> {
31-
const additionalHtmlOutputFiles = new Map(
32-
Object.entries(htmlOutputFiles).map(([path, content]) => [
33-
path,
34-
createOutputFile(path, content, BuildOutputFileType.Browser),
35-
]),
36-
);
37-
38-
return generateAngularServerAppManifest(
39-
additionalHtmlOutputFiles,
40-
[],
41-
false,
42-
undefined,
43-
undefined,
44-
baseHref,
45-
new Set(),
46-
dummyMetafile,
47-
undefined,
48-
);
49-
}
50-
5114
describe('generateAngularServerAppManifest', () => {
5215
beforeAll(async () => {
5316
await initializeHash();
5417
});
5518

19+
const dummyMetafile = { inputs: {}, outputs: {} } as unknown as Metafile;
20+
21+
/**
22+
* Evaluates a generated manifest, which both asserts that it is syntactically valid JavaScript and
23+
* gives access to the values it declares. The dynamic imports it contains are never invoked.
24+
*/
25+
function evaluateManifest(manifestContent: string): unknown {
26+
return new Function(manifestContent.replace('export default', 'return'))();
27+
}
28+
29+
function generateManifest(
30+
htmlOutputFiles: Record<string, string>,
31+
baseHref = '/',
32+
): ReturnType<typeof generateAngularServerAppManifest> {
33+
const additionalHtmlOutputFiles = new Map(
34+
Object.entries(htmlOutputFiles).map(([path, content]) => [
35+
path,
36+
createOutputFile(path, content, BuildOutputFileType.Browser),
37+
]),
38+
);
39+
40+
return generateAngularServerAppManifest(
41+
additionalHtmlOutputFiles,
42+
[],
43+
false,
44+
undefined,
45+
undefined,
46+
baseHref,
47+
new Set(),
48+
dummyMetafile,
49+
undefined,
50+
);
51+
}
52+
5653
it('should include criticalCssPlans when inlineCriticalCss is true', async () => {
5754
const additionalHtml = new Map([
5855
[
@@ -176,63 +173,35 @@ describe('generateAngularServerAppManifest', () => {
176173
expect(manifestContent).not.toContain('"styles.css":');
177174
});
178175

179-
it('serializes asset paths which contain JavaScript string delimiters', async () => {
176+
it('should serialize asset paths which contain JavaScript string delimiters', async () => {
180177
const assetPath = "catalog/customer's-choice/index.html";
181178
const { manifestContent } = await generateManifest({
182179
[assetPath]: '<main>Featured</main>',
183180
});
184181

185-
const assets = evaluateManifest(manifestContent)['assets'] as Record<string, unknown>;
186-
expect(Object.keys(assets)).toEqual([assetPath]);
182+
expect(evaluateManifest(manifestContent)).toEqual(
183+
jasmine.objectContaining({
184+
assets: {
185+
[assetPath]: jasmine.objectContaining({
186+
size: 21,
187+
hash: jasmine.any(String),
188+
text: jasmine.any(Function),
189+
}),
190+
},
191+
}),
192+
);
187193
});
188194

189-
it('serializes a base href which contains JavaScript string delimiters', async () => {
195+
it('should serialize a base href which contains JavaScript string delimiters', async () => {
190196
const { manifestContent } = await generateManifest(
191197
{ 'index.html': '<main></main>' },
192198
"/o'brien/",
193199
);
194200

195-
expect(evaluateManifest(manifestContent)['baseHref']).toBe("/o'brien/");
196-
});
197-
198-
it('generates chunk names which are usable as a file name and as a module specifier', async () => {
199-
const assetPath = "catalog/customer's#featured?ratio=50%/index.html";
200-
const { serverAssetsChunks } = await generateManifest({
201-
[assetPath]: '<main>Featured</main>',
202-
});
203-
204-
expect(serverAssetsChunks).toHaveSize(1);
205-
expect(serverAssetsChunks[0].path).toMatch(/^assets-chunks\/[a-zA-Z0-9_-]+\.mjs$/);
206-
});
207-
208-
it('generates a dynamic import which resolves back to the emitted chunk', async () => {
209-
// The in-memory ESM loader used while prerendering resolves the specifier as a URL and looks the
210-
// result up by output file path, so the two have to match exactly.
211-
const assetPath = "catalog/customer's#featured?ratio=50%/index.html";
212-
const { manifestContent, serverAssetsChunks } = await generateManifest({
213-
[assetPath]: '<main>Featured</main>',
214-
});
215-
216-
const assets = evaluateManifest(manifestContent)['assets'] as Record<
217-
string,
218-
{ text: () => Promise<string> }
219-
>;
220-
const specifier = /import\("(.+?)"\)/.exec(assets[assetPath].text.toString())?.[1];
221-
222-
const root = 'file:///virtual/root/';
223-
expect(specifier).toBeDefined();
224-
expect(new URL(specifier as string, root).href.slice(root.length)).toBe(
225-
serverAssetsChunks[0].path,
201+
expect(evaluateManifest(manifestContent)).toEqual(
202+
jasmine.objectContaining({
203+
baseHref: "/o'brien/",
204+
}),
226205
);
227206
});
228-
229-
it('generates a distinct chunk for asset paths which map to the same name', async () => {
230-
const { serverAssetsChunks } = await generateManifest({
231-
'foo/bar/index.html': '<main>nested</main>',
232-
'foo_bar/index.html': '<main>flat</main>',
233-
});
234-
235-
expect(serverAssetsChunks).toHaveSize(2);
236-
expect(serverAssetsChunks[0].path).not.toBe(serverAssetsChunks[1].path);
237-
});
238207
});

0 commit comments

Comments
 (0)