Skip to content

Commit 12f4433

Browse files
clydinalan-agius4
clydin
authored and
alan-agius4
committed
fix(@angular-devkit/build-angular): cache loading of component resources in JIT mode
The load result caching capabilities of the Angular compiler plugin used within the `application` and `browser-esbuild` builders is now used for both stylesheet and template component resources when building in JIT mode. This limits the amount of file system access required during a rebuild in JIT mode and also more accurately captures the full set of watched files.
1 parent 4e1f0e4 commit 12f4433

File tree

3 files changed

+101
-86
lines changed

3 files changed

+101
-86
lines changed

‎packages/angular_devkit/build_angular/src/builders/application/tests/behavior/rebuild-component_styles_spec.ts

Lines changed: 49 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -18,58 +18,61 @@ export const BUILD_TIMEOUT = 30_000;
1818

1919
describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
2020
describe('Behavior: "Rebuilds when component stylesheets change"', () => {
21-
it('updates component when imported sass changes', async () => {
22-
harness.useTarget('build', {
23-
...BASE_OPTIONS,
24-
watch: true,
25-
});
21+
for (const aot of [true, false]) {
22+
it(`updates component when imported sass changes with ${aot ? 'AOT' : 'JIT'}`, async () => {
23+
harness.useTarget('build', {
24+
...BASE_OPTIONS,
25+
watch: true,
26+
aot,
27+
});
2628

27-
await harness.modifyFile('src/app/app.component.ts', (content) =>
28-
content.replace('app.component.css', 'app.component.scss'),
29-
);
30-
await harness.writeFile('src/app/app.component.scss', "@import './a';");
31-
await harness.writeFile('src/app/a.scss', '$primary: aqua;\\nh1 { color: $primary; }');
29+
await harness.modifyFile('src/app/app.component.ts', (content) =>
30+
content.replace('app.component.css', 'app.component.scss'),
31+
);
32+
await harness.writeFile('src/app/app.component.scss', "@import './a';");
33+
await harness.writeFile('src/app/a.scss', '$primary: aqua;\\nh1 { color: $primary; }');
3234

33-
const buildCount = await harness
34-
.execute()
35-
.pipe(
36-
timeout(30000),
37-
concatMap(async ({ result }, index) => {
38-
expect(result?.success).toBe(true);
35+
const buildCount = await harness
36+
.execute()
37+
.pipe(
38+
timeout(30000),
39+
concatMap(async ({ result }, index) => {
40+
expect(result?.success).toBe(true);
3941

40-
switch (index) {
41-
case 0:
42-
harness.expectFile('dist/browser/main.js').content.toContain('color: aqua');
43-
harness.expectFile('dist/browser/main.js').content.not.toContain('color: blue');
42+
switch (index) {
43+
case 0:
44+
harness.expectFile('dist/browser/main.js').content.toContain('color: aqua');
45+
harness.expectFile('dist/browser/main.js').content.not.toContain('color: blue');
4446

45-
await harness.writeFile(
46-
'src/app/a.scss',
47-
'$primary: blue;\\nh1 { color: $primary; }',
48-
);
49-
break;
50-
case 1:
51-
harness.expectFile('dist/browser/main.js').content.not.toContain('color: aqua');
52-
harness.expectFile('dist/browser/main.js').content.toContain('color: blue');
47+
await harness.writeFile(
48+
'src/app/a.scss',
49+
'$primary: blue;\\nh1 { color: $primary; }',
50+
);
51+
break;
52+
case 1:
53+
harness.expectFile('dist/browser/main.js').content.not.toContain('color: aqua');
54+
harness.expectFile('dist/browser/main.js').content.toContain('color: blue');
5355

54-
await harness.writeFile(
55-
'src/app/a.scss',
56-
'$primary: green;\\nh1 { color: $primary; }',
57-
);
58-
break;
59-
case 2:
60-
harness.expectFile('dist/browser/main.js').content.not.toContain('color: aqua');
61-
harness.expectFile('dist/browser/main.js').content.not.toContain('color: blue');
62-
harness.expectFile('dist/browser/main.js').content.toContain('color: green');
56+
await harness.writeFile(
57+
'src/app/a.scss',
58+
'$primary: green;\\nh1 { color: $primary; }',
59+
);
60+
break;
61+
case 2:
62+
harness.expectFile('dist/browser/main.js').content.not.toContain('color: aqua');
63+
harness.expectFile('dist/browser/main.js').content.not.toContain('color: blue');
64+
harness.expectFile('dist/browser/main.js').content.toContain('color: green');
6365

64-
break;
65-
}
66-
}),
67-
take(3),
68-
count(),
69-
)
70-
.toPromise();
66+
break;
67+
}
68+
}),
69+
take(3),
70+
count(),
71+
)
72+
.toPromise();
7173

72-
expect(buildCount).toBe(3);
73-
});
74+
expect(buildCount).toBe(3);
75+
});
76+
}
7477
});
7578
});

‎packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,7 @@ export function createCompilerPlugin(
409409
stylesheetBundler,
410410
additionalResults,
411411
styleOptions.inlineStyleLanguage,
412+
pluginOptions.loadResultCache,
412413
);
413414
}
414415

‎packages/angular_devkit/build_angular/src/tools/esbuild/angular/jit-plugin-callbacks.ts

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88

99
import type { Metafile, OutputFile, PluginBuild } from 'esbuild';
1010
import { readFile } from 'node:fs/promises';
11-
import path from 'node:path';
11+
import { dirname, join, relative } from 'node:path';
12+
import { LoadResultCache, createCachedLoad } from '../load-result-cache';
1213
import { ComponentStylesheetBundler } from './component-stylesheets';
1314
import {
1415
JIT_NAMESPACE_REGEXP,
@@ -34,7 +35,7 @@ async function loadEntry(
3435
skipRead?: boolean,
3536
): Promise<{ path: string; contents?: string }> {
3637
if (entry.startsWith('file:')) {
37-
const specifier = path.join(root, entry.slice(5));
38+
const specifier = join(root, entry.slice(5));
3839

3940
return {
4041
path: specifier,
@@ -44,7 +45,7 @@ async function loadEntry(
4445
const [importer, data] = entry.slice(7).split(';', 2);
4546

4647
return {
47-
path: path.join(root, importer),
48+
path: join(root, importer),
4849
contents: Buffer.from(data, 'base64').toString(),
4950
};
5051
} else {
@@ -66,6 +67,7 @@ export function setupJitPluginCallbacks(
6667
stylesheetBundler: ComponentStylesheetBundler,
6768
additionalResultFiles: Map<string, { outputFiles?: OutputFile[]; metafile?: Metafile }>,
6869
inlineStyleLanguage: string,
70+
loadCache?: LoadResultCache,
6971
): void {
7072
const root = build.initialOptions.absWorkingDir ?? '';
7173

@@ -84,12 +86,12 @@ export function setupJitPluginCallbacks(
8486
return {
8587
// Use a relative path to prevent fully resolved paths in the metafile (JSON stats file).
8688
// This is only necessary for custom namespaces. esbuild will handle the file namespace.
87-
path: 'file:' + path.relative(root, path.join(path.dirname(args.importer), specifier)),
89+
path: 'file:' + relative(root, join(dirname(args.importer), specifier)),
8890
namespace,
8991
};
9092
} else {
9193
// Inline data may need the importer to resolve imports/references within the content
92-
const importer = path.relative(root, args.importer);
94+
const importer = relative(root, args.importer);
9395

9496
return {
9597
path: `inline:${importer};${specifier}`,
@@ -99,45 +101,54 @@ export function setupJitPluginCallbacks(
99101
});
100102

101103
// Add a load callback to handle Component stylesheets (both inline and external)
102-
build.onLoad({ filter: /./, namespace: JIT_STYLE_NAMESPACE }, async (args) => {
103-
// skipRead is used here because the stylesheet bundling will read a file stylesheet
104-
// directly either via a preprocessor or esbuild itself.
105-
const entry = await loadEntry(args.path, root, true /* skipRead */);
104+
build.onLoad(
105+
{ filter: /./, namespace: JIT_STYLE_NAMESPACE },
106+
createCachedLoad(loadCache, async (args) => {
107+
// skipRead is used here because the stylesheet bundling will read a file stylesheet
108+
// directly either via a preprocessor or esbuild itself.
109+
const entry = await loadEntry(args.path, root, true /* skipRead */);
110+
111+
let stylesheetResult;
112+
113+
// Stylesheet contents only exist for internal stylesheets
114+
if (entry.contents === undefined) {
115+
stylesheetResult = await stylesheetBundler.bundleFile(entry.path);
116+
} else {
117+
stylesheetResult = await stylesheetBundler.bundleInline(
118+
entry.contents,
119+
entry.path,
120+
inlineStyleLanguage,
121+
);
122+
}
123+
124+
const { contents, resourceFiles, errors, warnings, metafile, referencedFiles } =
125+
stylesheetResult;
126+
127+
additionalResultFiles.set(entry.path, { outputFiles: resourceFiles, metafile });
106128

107-
let stylesheetResult;
108-
109-
// Stylesheet contents only exist for internal stylesheets
110-
if (entry.contents === undefined) {
111-
stylesheetResult = await stylesheetBundler.bundleFile(entry.path);
112-
} else {
113-
stylesheetResult = await stylesheetBundler.bundleInline(
114-
entry.contents,
115-
entry.path,
116-
inlineStyleLanguage,
117-
);
118-
}
119-
120-
const { contents, resourceFiles, errors, warnings, metafile } = stylesheetResult;
121-
122-
additionalResultFiles.set(entry.path, { outputFiles: resourceFiles, metafile });
123-
124-
return {
125-
errors,
126-
warnings,
127-
contents,
128-
loader: 'text',
129-
};
130-
});
129+
return {
130+
errors,
131+
warnings,
132+
contents,
133+
loader: 'text',
134+
watchFiles: referencedFiles && [...referencedFiles],
135+
};
136+
}),
137+
);
131138

132139
// Add a load callback to handle Component templates
133140
// NOTE: While this callback supports both inline and external templates, the transformer
134141
// currently only supports generating URIs for external templates.
135-
build.onLoad({ filter: /./, namespace: JIT_TEMPLATE_NAMESPACE }, async (args) => {
136-
const { contents } = await loadEntry(args.path, root);
142+
build.onLoad(
143+
{ filter: /./, namespace: JIT_TEMPLATE_NAMESPACE },
144+
createCachedLoad(loadCache, async (args) => {
145+
const { contents, path } = await loadEntry(args.path, root);
137146

138-
return {
139-
contents,
140-
loader: 'text',
141-
};
142-
});
147+
return {
148+
contents,
149+
loader: 'text',
150+
watchFiles: [path],
151+
};
152+
}),
153+
);
143154
}

0 commit comments

Comments
 (0)