Skip to content

Commit 5598ecb

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 f1afa60 commit 5598ecb

3 files changed

Lines changed: 247 additions & 135 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
@@ -518,15 +518,11 @@ export function createCompilerPlugin(
518518
} else if (typeof contents === 'string' && (useTypeScriptTranspilation || isJS)) {
519519
// A string indicates untransformed output from the TS/NG compiler.
520520
// This step is unneeded when using esbuild transpilation.
521-
const sideEffects = await hasSideEffects(request);
522-
const instrumentForCoverage = pluginOptions.instrumentForCoverage?.(request);
523-
contents = await javascriptTransformer.transformData(
524-
request,
525-
contents,
526-
true /* skipLinker */,
527-
sideEffects,
528-
instrumentForCoverage,
529-
);
521+
contents = await javascriptTransformer.transformData(request, contents, {
522+
skipLinker: true,
523+
sideEffects: () => hasSideEffects(request),
524+
instrumentForCoverage: pluginOptions.instrumentForCoverage?.(request),
525+
});
530526

531527
// Store as the returned Uint8Array to allow caching the fully transformed code
532528
typeScriptFileCache.set(request, contents);
@@ -565,12 +561,10 @@ export function createCompilerPlugin(
565561
return profileAsync(
566562
'NG_EMIT_JS*',
567563
async () => {
568-
const sideEffects = await hasSideEffects(request);
569-
const contents = await javascriptTransformer.transformFile(
570-
request,
571-
pluginOptions.jit,
572-
sideEffects,
573-
);
564+
const contents = await javascriptTransformer.transformFile(request, {
565+
skipLinker: pluginOptions.jit,
566+
sideEffects: () => hasSideEffects(request),
567+
});
574568

575569
return {
576570
contents,

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

Lines changed: 86 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -33,36 +33,33 @@ 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+
if (ADVANCED_OPTIMIZATION_REGEX.test(data)) {
59+
return true;
60+
}
6461

65-
return regex.test(data);
62+
return DECORATOR_REGEX.test(data) && (await sideEffects?.()) === false;
6663
}
6764

6865
const dataBuffer = Buffer.isBuffer(data)
@@ -75,11 +72,9 @@ function hasAdvancedOptimizationCandidates(
7572
}
7673
}
7774

78-
if (sideEffects === false) {
79-
for (const tokenBytes of DECORATOR_TOKEN_BYTES) {
80-
if (dataBuffer.includes(tokenBytes)) {
81-
return true;
82-
}
75+
for (const tokenBytes of DECORATOR_TOKEN_BYTES) {
76+
if (dataBuffer.includes(tokenBytes)) {
77+
return (await sideEffects?.()) === false;
8378
}
8479
}
8580

@@ -121,6 +116,23 @@ export interface JavaScriptTransformerOptions {
121116
jit?: boolean;
122117
}
123118

119+
/**
120+
* Transformation options for an individual file or data transform request.
121+
*/
122+
export interface TransformOptions {
123+
/** If true, bypass all Angular linker processing; if false, attempt linking. */
124+
skipLinker?: boolean;
125+
126+
/**
127+
* An optional lazy resolver callback that returns whether the file has side-effects.
128+
* If it resolves to false, top-level pure function annotations and decorator wrapping may be applied.
129+
*/
130+
sideEffects?: () => Promise<boolean | undefined>;
131+
132+
/** If true, instrument the code for test coverage. */
133+
instrumentForCoverage?: boolean;
134+
}
135+
124136
/**
125137
* A class that performs transformation of JavaScript files and raw data.
126138
* A worker pool is used to distribute the transformation actions and allow
@@ -220,56 +232,14 @@ export class JavaScriptTransformer {
220232
* Performs JavaScript transformations on a file from the filesystem.
221233
* If no transformations are required, the data for the original file will be returned.
222234
* @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.
235+
* @param options Transformation options specific to this file.
225236
* @returns A promise that resolves to a UTF-8 encoded Uint8Array containing the result.
226237
*/
227-
async transformFile(
228-
filename: string,
229-
skipLinker?: boolean,
230-
sideEffects?: boolean,
231-
instrumentForCoverage?: boolean,
232-
): Promise<Uint8Array> {
238+
async transformFile(filename: string, options?: TransformOptions): Promise<Uint8Array> {
233239
return this.#runWithThrottle(async () => {
234240
const data = await readFile(filename);
235241

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;
242+
return this.transformData(filename, data, options);
273243
});
274244
}
275245

@@ -278,25 +248,36 @@ export class JavaScriptTransformer {
278248
* to exist on the filesystem.
279249
* @param filename The full path of the file represented by the data.
280250
* @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.
251+
* @param options Transformation options specific to this file data.
283252
* @returns A promise that resolves to a UTF-8 encoded Uint8Array containing the result.
284253
*/
285254
async transformData(
286255
filename: string,
287256
data: string | Uint8Array,
288-
skipLinker: boolean,
289-
sideEffects?: boolean,
290-
instrumentForCoverage?: boolean,
257+
options?: TransformOptions,
291258
): Promise<Uint8Array> {
292-
const shouldLink = !skipLinker && requiresLinking(filename, data);
259+
let resolvedSideEffects: boolean | undefined;
260+
let sideEffectsQueried = false;
261+
262+
const sideEffectsGetter = options?.sideEffects
263+
? async () => {
264+
if (!sideEffectsQueried) {
265+
sideEffectsQueried = true;
266+
resolvedSideEffects = await options.sideEffects?.();
267+
}
268+
269+
return resolvedSideEffects;
270+
}
271+
: undefined;
272+
273+
const shouldLink = !options?.skipLinker && requiresLinking(filename, data);
293274
const shouldOptimize =
294275
this.#commonOptions.advancedOptimizations &&
295-
hasAdvancedOptimizationCandidates(filename, data, sideEffects);
276+
(await hasAdvancedOptimizationCandidates(filename, data, sideEffectsGetter));
296277

297278
// Perform a quick test to determine if the data needs any transformations.
298279
// This allows directly returning the data without the worker communication overhead.
299-
if (!shouldLink && !shouldOptimize && !instrumentForCoverage) {
280+
if (!shouldLink && !shouldOptimize && !options?.instrumentForCoverage) {
300281
const keepSourcemap =
301282
this.#commonOptions.sourcemap &&
302283
(!!this.#commonOptions.thirdPartySourcemaps || !/[\\/]node_modules[\\/]/.test(filename));
@@ -308,6 +289,26 @@ export class JavaScriptTransformer {
308289
return keepSourcemap ? data : removeSourceMappingURL(data);
309290
}
310291

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

322-
return this.#ensureWorkerPool().run(
323+
const result = (await this.#ensureWorkerPool().run(
323324
{
324325
filename,
325326
data,
326327
skipLinker: !shouldLink,
327-
sideEffects,
328-
instrumentForCoverage,
328+
sideEffects: resolvedSideEffects,
329+
instrumentForCoverage: options?.instrumentForCoverage,
329330
},
330331
{
331332
transferList: isTransferable ? [data.buffer] : undefined,
332333
},
333-
);
334+
)) as Uint8Array;
335+
336+
if (this.cache && cacheKey) {
337+
try {
338+
await this.cache.put(cacheKey, result);
339+
} catch {
340+
// Failure to store the value in the cache should not fail the transform
341+
}
342+
}
343+
344+
return result;
334345
}
335346

336347
/**

0 commit comments

Comments
 (0)