Skip to content

Commit 5691298

Browse files
authored
perf(@angular/build): avoid reading external component stylesheets twice
- Skip calling `this.readFile(filename)` in `host.readResource(filename)` when `!hasTemplateExtension(filename)` so external component stylesheets (`styleUrl` / `styleUrls`) are not read from disk on the Angular compiler worker thread and serialized over the worker `MessagePort` only to be ignored by `transformStylesheet`. - Only short-circuit empty `data` in `host.transformResource(data, context)` for inline styles (`!context.resourceFile`) so external stylesheets still proceed to `transformStylesheet`.
1 parent c32a690 commit 5691298

2 files changed

Lines changed: 105 additions & 2 deletions

File tree

‎packages/angular/build/src/tools/angular/angular-host.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,12 @@ export function createAngularCompilerHost(
182182
// The AOT compiler currently requires this hook to allow for a transformResource hook.
183183
// Once the AOT compiler allows only a transformResource hook, this can be reevaluated.
184184
host.readResource = async function (filename) {
185+
// Only template resources need to be read by the compiler host.
186+
// External stylesheets are read and bundled by the stylesheet bundler during `transformResource`.
187+
if (!hasTemplateExtension(filename)) {
188+
return '';
189+
}
190+
185191
return this.readFile(filename) ?? '';
186192
};
187193

@@ -197,8 +203,8 @@ export function createAngularCompilerHost(
197203
'External runtime stylesheets should not be transformed: ' + context.resourceFile,
198204
);
199205

200-
// No transformation required if the resource is empty
201-
if (data.trim().length === 0) {
206+
// No transformation required if an inline resource is empty
207+
if (!context.resourceFile && data.trim().length === 0) {
202208
return { content: '' };
203209
}
204210

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
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 ts from 'typescript';
10+
import { type AngularHostOptions, createAngularCompilerHost } from './angular-host';
11+
12+
describe('createAngularCompilerHost', () => {
13+
function createHost() {
14+
const transformStylesheet = jasmine
15+
.createSpy<AngularHostOptions['transformStylesheet']>('transformStylesheet')
16+
.and.resolveTo('.compiled {}');
17+
const processWebWorker = jasmine
18+
.createSpy<AngularHostOptions['processWebWorker']>('processWebWorker')
19+
.and.returnValue('worker.js');
20+
21+
const host = createAngularCompilerHost(
22+
ts,
23+
{},
24+
{
25+
transformStylesheet,
26+
processWebWorker,
27+
},
28+
undefined,
29+
);
30+
31+
return { host, transformStylesheet };
32+
}
33+
34+
describe('readResource', () => {
35+
it('should not call readFile for .css and .scss stylesheet files', async () => {
36+
const { host } = createHost();
37+
const readFileSpy = spyOn(host, 'readFile').and.returnValue('.btn { color: red; }');
38+
39+
expect(await host.readResource?.('/app/src/app.component.css')).toBe('');
40+
expect(await host.readResource?.('/app/src/app.component.scss')).toBe('');
41+
expect(readFileSpy).not.toHaveBeenCalled();
42+
});
43+
44+
it('should call readFile for .html and .svg template files', async () => {
45+
const { host } = createHost();
46+
const readFileSpy = spyOn(host, 'readFile').and.callFake(
47+
(fileName) => `<content of ${fileName}>`,
48+
);
49+
50+
expect(await host.readResource?.('/app/src/app.component.html')).toBe(
51+
'<content of /app/src/app.component.html>',
52+
);
53+
expect(await host.readResource?.('/app/src/icon.component.svg')).toBe(
54+
'<content of /app/src/icon.component.svg>',
55+
);
56+
expect(readFileSpy).toHaveBeenCalledTimes(2);
57+
});
58+
});
59+
60+
describe('transformResource', () => {
61+
it('should invoke transformStylesheet when context.resourceFile is set even if data is empty', async () => {
62+
const { host, transformStylesheet } = createHost();
63+
64+
const result = await host.transformResource?.('', {
65+
type: 'style',
66+
containingFile: '/app/src/app.component.ts',
67+
resourceFile: '/app/src/app.component.scss',
68+
order: 0,
69+
className: 'AppComponent',
70+
});
71+
72+
expect(result).toEqual({ content: '.compiled {}' });
73+
expect(transformStylesheet).toHaveBeenCalledOnceWith(
74+
'',
75+
'/app/src/app.component.ts',
76+
'/app/src/app.component.scss',
77+
0,
78+
'AppComponent',
79+
);
80+
});
81+
82+
it('should short-circuit empty inline styles without calling transformStylesheet', async () => {
83+
const { host, transformStylesheet } = createHost();
84+
85+
const result = await host.transformResource?.(' ', {
86+
type: 'style',
87+
containingFile: '/app/src/app.component.ts',
88+
resourceFile: null,
89+
order: 0,
90+
className: 'AppComponent',
91+
});
92+
93+
expect(result).toEqual({ content: '' });
94+
expect(transformStylesheet).not.toHaveBeenCalled();
95+
});
96+
});
97+
});

0 commit comments

Comments
 (0)