Skip to content

Commit 0c7035a

Browse files
committed
refactor(@angular/build): lazily evaluate sideEffects in javascript transformer
Previously, compiler-plugin.ts eagerly resolved sideEffects for every JavaScript file loaded during bundling by calling await hasSideEffects(request), which executes esbuild's asynchronous build.resolve to inspect package.json. Within the JavaScript transformer, sideEffects is only needed in two specific locations: when checking if a file belongs to node_modules/@angular/ to apply top-level pure function annotations, and when determining whether to wrap decorators in side-effect-free files that lack primary candidate tokens. The sideEffects check is now evaluated lazily. Method signatures on transformData and transformFile have been simplified to accept a TransformOptions interface with an optional async sideEffects resolver callback. Inside hasAdvancedOptimizationCandidates, the resolver is only evaluated if the file matches the node_modules/@angular/ path pattern or contains decorator tokens without primary optimization tokens. If the candidate check does not query sideEffects, the resolver is never invoked, eliminating unnecessary build.resolve calls for bypassed files, linker-only modules, and files matching primary optimization tokens.
1 parent 0a137f9 commit 0c7035a

3 files changed

Lines changed: 313 additions & 137 deletions

File tree

packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -526,15 +526,11 @@ export function createCompilerPlugin(
526526
} else if (typeof contents === 'string' && (useTypeScriptTranspilation || isJS)) {
527527
// A string indicates untransformed output from the TS/NG compiler.
528528
// This step is unneeded when using esbuild transpilation.
529-
const sideEffects = await hasSideEffects(request);
530-
const instrumentForCoverage = pluginOptions.instrumentForCoverage?.(request);
531-
contents = await javascriptTransformer.transformData(
532-
request,
533-
contents,
534-
true /* skipLinker */,
535-
sideEffects,
536-
instrumentForCoverage,
537-
);
529+
contents = await javascriptTransformer.transformData(request, contents, {
530+
skipLinker: true,
531+
sideEffects: () => hasSideEffects(request),
532+
instrumentForCoverage: pluginOptions.instrumentForCoverage?.(request),
533+
});
538534

539535
// Store as the returned Uint8Array to allow caching the fully transformed code
540536
typeScriptFileCache.set(request, contents);
@@ -573,12 +569,10 @@ export function createCompilerPlugin(
573569
return profileAsync(
574570
'NG_EMIT_JS*',
575571
async () => {
576-
const sideEffects = await hasSideEffects(request);
577-
const contents = await javascriptTransformer.transformFile(
578-
request,
579-
pluginOptions.jit,
580-
sideEffects,
581-
);
572+
const contents = await javascriptTransformer.transformFile(request, {
573+
skipLinker: pluginOptions.jit,
574+
sideEffects: () => hasSideEffects(request),
575+
});
582576

583577
return {
584578
contents,

packages/angular/build/src/tools/esbuild/javascript-transformer.ts

Lines changed: 94 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -33,53 +33,52 @@ const DECORATOR_TOKENS = ['__decorate', '__esDecorate'] as const;
3333
const DECORATOR_TOKEN_BYTES = DECORATOR_TOKENS.map((token) => Buffer.from(token, 'utf-8'));
3434

3535
const ADVANCED_OPTIMIZATION_REGEX = new RegExp(ADVANCED_OPTIMIZATION_TOKENS.join('|'));
36-
const ADVANCED_OPTIMIZATION_WITH_DECORATORS_REGEX = new RegExp(
37-
[...ADVANCED_OPTIMIZATION_TOKENS, ...DECORATOR_TOKENS].join('|'),
38-
);
36+
const DECORATOR_REGEX = new RegExp(DECORATOR_TOKENS.join('|'));
3937

4038
/**
4139
* Determines whether JavaScript code contains potential candidate constructs for advanced optimizations.
4240
* When false, advanced optimizations can be bypassed without worker dispatch or AST parsing.
4341
*
4442
* @param filename The full path to the file.
4543
* @param data The data (string or Buffer) of the file.
46-
* @param sideEffects If false, indicates the file is considered side-effect free.
44+
* @param sideEffects An optional lazy resolver callback that returns whether the file is considered side-effect free.
4745
* @returns True if the code may contain constructs that advanced optimizations can mutate.
4846
*/
49-
function hasAdvancedOptimizationCandidates(
47+
async function hasAdvancedOptimizationCandidates(
5048
filename: string,
5149
data: string | Uint8Array,
52-
sideEffects?: boolean,
53-
): boolean {
50+
sideEffects?: () => Promise<boolean | undefined>,
51+
): Promise<boolean> {
5452
// Side-effect-free @angular/ packages undergo top-level pure function annotations
55-
if (sideEffects === false && /[\\/]node_modules[\\/]@angular[\\/]/.test(filename)) {
53+
if (/[\\/]node_modules[\\/]@angular[\\/]/.test(filename) && (await sideEffects?.()) === false) {
5654
return true;
5755
}
5856

5957
if (typeof data === 'string') {
60-
const regex =
61-
sideEffects === false
62-
? ADVANCED_OPTIMIZATION_WITH_DECORATORS_REGEX
63-
: ADVANCED_OPTIMIZATION_REGEX;
58+
const hasDecorators = DECORATOR_REGEX.test(data);
59+
if (hasDecorators && (await sideEffects?.()) === false) {
60+
return true;
61+
}
6462

65-
return regex.test(data);
63+
return ADVANCED_OPTIMIZATION_REGEX.test(data);
6664
}
6765

6866
const dataBuffer = Buffer.isBuffer(data)
6967
? data
7068
: Buffer.from(data.buffer, data.byteOffset, data.byteLength);
7169

72-
for (const tokenBytes of ADVANCED_OPTIMIZATION_TOKEN_BYTES) {
70+
for (const tokenBytes of DECORATOR_TOKEN_BYTES) {
7371
if (dataBuffer.includes(tokenBytes)) {
74-
return true;
72+
if ((await sideEffects?.()) === false) {
73+
return true;
74+
}
75+
break;
7576
}
7677
}
7778

78-
if (sideEffects === false) {
79-
for (const tokenBytes of DECORATOR_TOKEN_BYTES) {
80-
if (dataBuffer.includes(tokenBytes)) {
81-
return true;
82-
}
79+
for (const tokenBytes of ADVANCED_OPTIMIZATION_TOKEN_BYTES) {
80+
if (dataBuffer.includes(tokenBytes)) {
81+
return true;
8382
}
8483
}
8584

@@ -121,6 +120,23 @@ export interface JavaScriptTransformerOptions {
121120
jit?: boolean;
122121
}
123122

123+
/**
124+
* Transformation options for an individual file or data transform request.
125+
*/
126+
export interface TransformOptions {
127+
/** If true, bypass all Angular linker processing; if false, attempt linking. */
128+
skipLinker?: boolean;
129+
130+
/**
131+
* An optional lazy resolver callback that returns whether the file has side-effects.
132+
* If it resolves to false, top-level pure function annotations and decorator wrapping may be applied.
133+
*/
134+
sideEffects?: () => Promise<boolean | undefined>;
135+
136+
/** If true, instrument the code for test coverage. */
137+
instrumentForCoverage?: boolean;
138+
}
139+
124140
/**
125141
* A class that performs transformation of JavaScript files and raw data.
126142
* A worker pool is used to distribute the transformation actions and allow
@@ -220,56 +236,14 @@ export class JavaScriptTransformer {
220236
* Performs JavaScript transformations on a file from the filesystem.
221237
* If no transformations are required, the data for the original file will be returned.
222238
* @param filename The full path to the file.
223-
* @param skipLinker If true, bypass all Angular linker processing; if false, attempt linking.
224-
* @param sideEffects If false, and `advancedOptimizations` is enabled tslib decorators are wrapped.
239+
* @param options Transformation options specific to this file.
225240
* @returns A promise that resolves to a UTF-8 encoded Uint8Array containing the result.
226241
*/
227-
async transformFile(
228-
filename: string,
229-
skipLinker?: boolean,
230-
sideEffects?: boolean,
231-
instrumentForCoverage?: boolean,
232-
): Promise<Uint8Array> {
242+
async transformFile(filename: string, options?: TransformOptions): Promise<Uint8Array> {
233243
return this.#runWithThrottle(async () => {
234244
const data = await readFile(filename);
235245

236-
let cacheKey: string | undefined;
237-
if (this.cache) {
238-
// Create a cache key from the file data and options that effect the output.
239-
// NOTE: If additional options are added, this may need to be updated.
240-
const hasher = createContentHash();
241-
hasher.update(`${!!skipLinker}--${!!sideEffects}`);
242-
hasher.update(data);
243-
hasher.update(this.#fileCacheKeyBase);
244-
cacheKey = hasher.digest();
245-
246-
try {
247-
const cached = await this.cache.get(cacheKey);
248-
if (cached !== undefined) {
249-
return cached;
250-
}
251-
} catch {
252-
// Failure to get the value should not fail the transform
253-
}
254-
}
255-
256-
const result = await this.transformData(
257-
filename,
258-
data,
259-
!!skipLinker,
260-
sideEffects,
261-
instrumentForCoverage,
262-
);
263-
264-
if (this.cache && cacheKey) {
265-
try {
266-
await this.cache.put(cacheKey, result);
267-
} catch {
268-
// Failure to store the value in the cache should not fail the transform
269-
}
270-
}
271-
272-
return result;
246+
return this.transformData(filename, data, options);
273247
});
274248
}
275249

@@ -278,25 +252,36 @@ export class JavaScriptTransformer {
278252
* to exist on the filesystem.
279253
* @param filename The full path of the file represented by the data.
280254
* @param data The data of the file that should be transformed.
281-
* @param skipLinker If true, bypass all Angular linker processing; if false, attempt linking.
282-
* @param sideEffects If false, and `advancedOptimizations` is enabled tslib decorators are wrapped.
255+
* @param options Transformation options specific to this file data.
283256
* @returns A promise that resolves to a UTF-8 encoded Uint8Array containing the result.
284257
*/
285258
async transformData(
286259
filename: string,
287260
data: string | Uint8Array,
288-
skipLinker: boolean,
289-
sideEffects?: boolean,
290-
instrumentForCoverage?: boolean,
261+
options?: TransformOptions,
291262
): Promise<Uint8Array> {
292-
const shouldLink = !skipLinker && requiresLinking(filename, data);
263+
let resolvedSideEffects: boolean | undefined;
264+
let sideEffectsQueried = false;
265+
266+
const sideEffectsGetter = options?.sideEffects
267+
? async () => {
268+
if (!sideEffectsQueried) {
269+
sideEffectsQueried = true;
270+
resolvedSideEffects = await options.sideEffects?.();
271+
}
272+
273+
return resolvedSideEffects;
274+
}
275+
: undefined;
276+
277+
const shouldLink = !options?.skipLinker && requiresLinking(filename, data);
293278
const shouldOptimize =
294279
this.#commonOptions.advancedOptimizations &&
295-
hasAdvancedOptimizationCandidates(filename, data, sideEffects);
280+
(await hasAdvancedOptimizationCandidates(filename, data, sideEffectsGetter));
296281

297282
// Perform a quick test to determine if the data needs any transformations.
298283
// This allows directly returning the data without the worker communication overhead.
299-
if (!shouldLink && !shouldOptimize && !instrumentForCoverage) {
284+
if (!shouldLink && !shouldOptimize && !options?.instrumentForCoverage) {
300285
const keepSourcemap =
301286
this.#commonOptions.sourcemap &&
302287
(!!this.#commonOptions.thirdPartySourcemaps || !/[\\/]node_modules[\\/]/.test(filename));
@@ -308,6 +293,28 @@ export class JavaScriptTransformer {
308293
return keepSourcemap ? data : removeSourceMappingURL(data);
309294
}
310295

296+
let cacheKey: string | undefined;
297+
if (this.cache) {
298+
// Create a cache key from the file data and options that affect the output.
299+
// NOTE: If additional options are added, this may need to be updated.
300+
const hasher = createContentHash();
301+
hasher.update(
302+
`${!options?.skipLinker}--${resolvedSideEffects === false}--${!!options?.instrumentForCoverage}`,
303+
);
304+
hasher.update(data);
305+
hasher.update(this.#fileCacheKeyBase);
306+
cacheKey = hasher.digest();
307+
308+
try {
309+
const cached = await this.cache.get(cacheKey);
310+
if (cached !== undefined) {
311+
return cached;
312+
}
313+
} catch {
314+
// Failure to get the value should not fail the transform
315+
}
316+
}
317+
311318
// Only standalone (non-pooled) ArrayBuffers can be transferred across worker threads.
312319
// Node.js shares an internal 8KB ArrayBuffer pool for small buffers, and transferring
313320
// a pooled buffer will throw a DataCloneError because detaching it invalidates other slices.
@@ -319,18 +326,28 @@ export class JavaScriptTransformer {
319326
data.byteLength === data.buffer.byteLength &&
320327
!process.versions.pnp;
321328

322-
return this.#ensureWorkerPool().run(
329+
const result = (await this.#ensureWorkerPool().run(
323330
{
324331
filename,
325332
data,
326333
skipLinker: !shouldLink,
327-
sideEffects,
328-
instrumentForCoverage,
334+
sideEffects: resolvedSideEffects,
335+
instrumentForCoverage: options?.instrumentForCoverage,
329336
},
330337
{
331338
transferList: isTransferable ? [data.buffer] : undefined,
332339
},
333-
);
340+
)) as Uint8Array;
341+
342+
if (this.cache && cacheKey) {
343+
try {
344+
await this.cache.put(cacheKey, result);
345+
} catch {
346+
// Failure to store the value in the cache should not fail the transform
347+
}
348+
}
349+
350+
return result;
334351
}
335352

336353
/**

0 commit comments

Comments
 (0)