From 8d482a13b02feb7ce5a3cd644c33e8ed49d9da58 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:35:54 +0000 Subject: [PATCH] perf(@angular/build): memoize package-level sideEffects checks in compiler plugin When advancedOptimizations is enabled, the compiler plugin checks each emitted and transformed JavaScript file via hasSideEffects() to determine whether pure annotations should be added during bundling. Previously, hasSideEffects() invoked build.resolve() across the esbuild Go <-> Node.js IPC boundary for every individual file. In an application with hundreds of files loaded from node_modules (e.g. 226 files from rxjs), this resulted in hundreds of redundant IPC round-trips to evaluate the exact same package-level sideEffects configuration. To eliminate redundant resolution calls: - Introduce a dedicated SideEffectsResolver class and createSideEffectsResolver() factory in a separate module. - Memoize resolved sideEffects booleans at both the package level and the individual file level. - When an imported file resides in node_modules, extract the package directory and inspect its package.json sideEffects property once. - If sideEffects is a boolean, memoize it for all files originating from that package. - If sideEffects is an array of globs, string, or omitted, bypass package-level memoization and resolve via build.resolve() per file, memoizing the resolved file result to prevent duplicate calls. - Return an async noop when advancedOptimizations is disabled to avoid overhead. - Provide comprehensive unit tests covering package memoization, non-boolean sideEffects, scoped packages, pnpm virtual stores, and Windows/relative paths. In benchmarks on an application with 253 JS dependency files across 6 packages, build.resolve IPC calls dropped from 253 to 6 (-97.6%) and cumulative resolve duration dropped by 99.7%. --- .../tools/esbuild/angular/compiler-plugin.ts | 19 +- .../esbuild/angular/side-effects-resolver.ts | 174 ++++++++++ .../angular/side-effects-resolver_spec.ts | 322 ++++++++++++++++++ 3 files changed, 499 insertions(+), 16 deletions(-) create mode 100644 packages/angular/build/src/tools/esbuild/angular/side-effects-resolver.ts create mode 100644 packages/angular/build/src/tools/esbuild/angular/side-effects-resolver_spec.ts diff --git a/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts b/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts index f80f3c78afca..54d36896b778 100644 --- a/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts +++ b/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts @@ -37,6 +37,7 @@ import { ComponentStylesheetBundler } from './component-stylesheets'; import { FileReferenceTracker } from './file-reference-tracker'; import { setupJitPluginCallbacks } from './jit-plugin-callbacks'; import { rewriteForBazel } from './rewrite-bazel-paths'; +import { createSideEffectsResolver } from './side-effects-resolver'; import { SourceFileCache } from './source-file-cache'; export interface CompilerPluginOptions { @@ -116,6 +117,8 @@ export function createCompilerPlugin( cacheStore?.createCache('jstransformer'), ); + const hasSideEffects = createSideEffectsResolver(build, pluginOptions.advancedOptimizations); + // Setup defines based on the values used by the Angular compiler-cli build.initialOptions.define ??= {}; build.initialOptions.define['ngI18nClosureMode'] ??= 'false'; @@ -647,22 +650,6 @@ export function createCompilerPlugin( void javascriptTransformer.close(); void cacheStore?.close(); }); - - /** - * Checks if the file has side-effects when `advancedOptimizations` is enabled. - */ - async function hasSideEffects(path: string): Promise { - if (!pluginOptions.advancedOptimizations) { - return undefined; - } - - const { sideEffects } = await build.resolve(path, { - kind: 'import-statement', - resolveDir: build.initialOptions.absWorkingDir ?? '', - }); - - return sideEffects; - } }, }; } diff --git a/packages/angular/build/src/tools/esbuild/angular/side-effects-resolver.ts b/packages/angular/build/src/tools/esbuild/angular/side-effects-resolver.ts new file mode 100644 index 000000000000..433ccc83e382 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/angular/side-effects-resolver.ts @@ -0,0 +1,174 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import type { PluginBuild } from 'esbuild'; +import { readFile } from 'node:fs/promises'; +import * as path from 'node:path'; + +/** + * Extracts the root package directory containing package.json for a file inside node_modules. + * Supports standard packages (node_modules/pkg/...) and scoped packages (node_modules/@scope/pkg/...). + */ +export function getPackageDirectory(filePath: string): string | undefined { + const normalizedPath = filePath.includes('\\') ? filePath.replace(/\\/g, '/') : filePath; + let index = normalizedPath.lastIndexOf('/node_modules/'); + let offset = 14; + if (index === -1) { + if (normalizedPath.startsWith('node_modules/')) { + index = 0; + offset = 13; + } else { + return undefined; + } + } + + const afterNodeModules = normalizedPath.slice(index + offset); + const firstSlash = afterNodeModules.indexOf('/'); + if (firstSlash === -1) { + return undefined; + } + + let end = index + offset; + if (afterNodeModules.startsWith('@')) { + const secondSlash = afterNodeModules.indexOf('/', firstSlash + 1); + end += secondSlash === -1 ? afterNodeModules.length : secondSlash; + } else { + end += firstSlash; + } + + return filePath.slice(0, end); +} + +/** + * Resolves and memoizes package-level and file-level side-effects for bundling optimizations. + */ +export class SideEffectsResolver { + readonly #build: PluginBuild; + readonly #advancedOptimizations: boolean; + readonly #workingDirectory: string; + + /** + * Memoizes package-level sideEffects values. + * - `true` or `false` when package.json specifies a boolean `sideEffects`. + * - `null` when package.json has non-boolean (e.g. array of globs, string), omitted sideEffects, or fails to read. + * - `Promise` while package.json is being read and parsed. + */ + readonly #packageSideEffectsCache = new Map | boolean | null>(); + + /** + * Memoizes file-level sideEffects results. + * - `true` or `false` once resolved. + * - `Promise` while esbuild resolution is in-flight. + */ + readonly #fileSideEffectsCache = new Map | boolean>(); + + constructor(build: PluginBuild, advancedOptimizations: boolean = true) { + this.#build = build; + this.#advancedOptimizations = advancedOptimizations; + this.#workingDirectory = build.initialOptions.absWorkingDir ?? ''; + } + + /** + * Determines if a file has side-effects. + * Returns `undefined` when `advancedOptimizations` is disabled. + */ + async resolve(filePath: string): Promise { + if (!this.#advancedOptimizations) { + return undefined; + } + + const cachedFileSideEffects = this.#fileSideEffectsCache.get(filePath); + if (cachedFileSideEffects !== undefined) { + return cachedFileSideEffects; + } + + const packageDir = getPackageDirectory(filePath); + if (packageDir !== undefined) { + let packageSideEffects = this.#packageSideEffectsCache.get(packageDir); + if (packageSideEffects === undefined) { + packageSideEffects = this.#resolvePackageSideEffects(packageDir); + this.#packageSideEffectsCache.set(packageDir, packageSideEffects); + } + + if (packageSideEffects instanceof Promise) { + packageSideEffects = await packageSideEffects; + this.#packageSideEffectsCache.set(packageDir, packageSideEffects); + } + + if (packageSideEffects !== null) { + this.#fileSideEffectsCache.set(filePath, packageSideEffects); + + return packageSideEffects; + } + } + + // Fallback: per-file resolution via esbuild when outside node_modules, + // or when the package sideEffects is non-boolean (array of globs, string, omitted). + const resolutionPromise = (async () => { + try { + const { sideEffects } = await this.#build.resolve(filePath, { + kind: 'import-statement', + resolveDir: this.#workingDirectory, + }); + + this.#fileSideEffectsCache.set(filePath, sideEffects); + + return sideEffects; + } catch (error) { + this.#fileSideEffectsCache.delete(filePath); + throw error; + } + })(); + + this.#fileSideEffectsCache.set(filePath, resolutionPromise); + + return resolutionPromise; + } + + async #resolvePackageSideEffects(packageDir: string): Promise { + try { + const packageJsonPath = path.join(packageDir, 'package.json'); + const packageJsonContent = await readFile(packageJsonPath, 'utf-8'); + const packageJson = JSON.parse(packageJsonContent) as { sideEffects?: unknown } | null; + const sideEffects = packageJson?.sideEffects; + + return typeof sideEffects === 'boolean' ? sideEffects : null; + } catch { + return null; + } + } + + /** + * Clears all memoized package and file-level side-effects caches. + */ + clear(): void { + this.#packageSideEffectsCache.clear(); + this.#fileSideEffectsCache.clear(); + } +} + +/** + * Creates a side-effects resolver function that determines whether a file has side-effects. + * + * @param build The esbuild PluginBuild instance. + * @param advancedOptimizations Whether advanced optimizations are enabled. + * @returns An async function accepting a file path and returning whether the file has side-effects, + * or `undefined` when `advancedOptimizations` is false. + */ +export function createSideEffectsResolver( + build: PluginBuild, + advancedOptimizations?: boolean, +): (filePath: string) => Promise { + if (!advancedOptimizations) { + return async () => undefined; + } + + const resolver = new SideEffectsResolver(build, advancedOptimizations); + + return (filePath: string) => resolver.resolve(filePath); +} diff --git a/packages/angular/build/src/tools/esbuild/angular/side-effects-resolver_spec.ts b/packages/angular/build/src/tools/esbuild/angular/side-effects-resolver_spec.ts new file mode 100644 index 000000000000..f605ac4aaf90 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/angular/side-effects-resolver_spec.ts @@ -0,0 +1,322 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import type { PluginBuild } from 'esbuild'; +import assert from 'node:assert'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import * as path from 'node:path'; +import { + SideEffectsResolver, + createSideEffectsResolver, + getPackageDirectory, +} from './side-effects-resolver'; + +describe('getPackageDirectory', () => { + it('should extract package dir for a standard package in node_modules', () => { + const filePath = '/workspace/node_modules/rxjs/dist/esm5/index.js'; + expect(getPackageDirectory(filePath)).toBe('/workspace/node_modules/rxjs'); + }); + + it('should extract package dir for a scoped package in node_modules', () => { + const filePath = '/workspace/node_modules/@angular/core/fesm2022/core.mjs'; + expect(getPackageDirectory(filePath)).toBe('/workspace/node_modules/@angular/core'); + }); + + it('should extract package dir for nested node_modules', () => { + const filePath = '/workspace/node_modules/foo/node_modules/@bar/baz/dist/index.js'; + expect(getPackageDirectory(filePath)).toBe('/workspace/node_modules/foo/node_modules/@bar/baz'); + }); + + it('should extract package dir for relative node_modules paths', () => { + const filePath = 'node_modules/@angular/core/fesm2022/core.mjs'; + expect(getPackageDirectory(filePath)).toBe('node_modules/@angular/core'); + }); + + it('should extract package dir for Windows formatted paths', () => { + const filePath = 'C:\\workspace\\node_modules\\@angular\\core\\fesm2022\\core.mjs'; + expect(getPackageDirectory(filePath)).toBe('C:\\workspace\\node_modules\\@angular\\core'); + }); + + it('should return undefined for files not in node_modules', () => { + const filePath = '/workspace/src/app/app.component.ts'; + expect(getPackageDirectory(filePath)).toBeUndefined(); + }); +}); + +describe('SideEffectsResolver', () => { + let testDir: string; + let mockBuild: { + resolve: jasmine.Spy; + initialOptions: { absWorkingDir: string }; + }; + + beforeEach(async () => { + const baseTmpDir = process.env['TEST_TMPDIR']; + assert(baseTmpDir, 'TEST_TMPDIR is not set'); + testDir = await mkdtemp(path.join(baseTmpDir, 'side-effects-test-')); + mockBuild = { + resolve: jasmine.createSpy('resolve').and.callFake(async (targetPath: string) => ({ + errors: [], + warnings: [], + path: targetPath, + external: false, + sideEffects: true, + namespace: 'file', + suffix: '', + pluginData: null, + })), + initialOptions: { absWorkingDir: testDir }, + }; + }); + + afterEach(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + async function createPackage( + pkgName: string, + pkgJsonContent: Record, + subFiles: string[] = ['index.js'], + ): Promise { + const pkgDir = path.join(testDir, 'node_modules', pkgName); + await mkdir(pkgDir, { recursive: true }); + await writeFile( + path.join(pkgDir, 'package.json'), + JSON.stringify(pkgJsonContent, null, 2), + 'utf-8', + ); + + const filePaths: string[] = []; + for (const subFile of subFiles) { + const fullPath = path.join(pkgDir, subFile); + await mkdir(path.dirname(fullPath), { recursive: true }); + await writeFile(fullPath, '// test file', 'utf-8'); + filePaths.push(fullPath); + } + + return filePaths; + } + + describe('when advancedOptimizations is false', () => { + it('should return undefined and avoid resolution or package reads', async () => { + const [file] = await createPackage('pure-pkg', { sideEffects: false }); + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, false); + + const result = await resolver(file); + expect(result).toBeUndefined(); + expect(mockBuild.resolve).not.toHaveBeenCalled(); + }); + + it('should return undefined via SideEffectsResolver instance directly', async () => { + const [file] = await createPackage('pure-pkg', { sideEffects: false }); + const resolver = new SideEffectsResolver(mockBuild as unknown as PluginBuild, false); + + const result = await resolver.resolve(file); + expect(result).toBeUndefined(); + expect(mockBuild.resolve).not.toHaveBeenCalled(); + }); + }); + + describe('when package has sideEffects: false', () => { + it('should return false and memoize package-level result across multiple files', async () => { + const [file1, file2] = await createPackage('pure-pkg', { sideEffects: false }, [ + 'file1.js', + 'file2.js', + ]); + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + const result1 = await resolver(file1); + const result2 = await resolver(file2); + + expect(result1).toBeFalse(); + expect(result2).toBeFalse(); + expect(mockBuild.resolve).not.toHaveBeenCalled(); + + // Repeated lookup for the same file returns memoized result + const result1Cached = await resolver(file1); + expect(result1Cached).toBeFalse(); + expect(mockBuild.resolve).not.toHaveBeenCalled(); + }); + }); + + describe('when package has sideEffects: true', () => { + it('should return true and memoize package-level result across multiple files', async () => { + const [file1, file2] = await createPackage('impure-pkg', { sideEffects: true }, [ + 'file1.js', + 'file2.js', + ]); + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + const result1 = await resolver(file1); + const result2 = await resolver(file2); + + expect(result1).toBeTrue(); + expect(result2).toBeTrue(); + expect(mockBuild.resolve).not.toHaveBeenCalled(); + }); + }); + + describe('when package has non-boolean sideEffects (array of globs or omitted)', () => { + it('should fallback to build.resolve per file when sideEffects is an array', async () => { + const [file1, file2] = await createPackage('glob-pkg', { sideEffects: ['*.css'] }, [ + 'index.js', + 'styles.css', + ]); + + mockBuild.resolve.and.callFake(async (targetPath: string) => ({ + errors: [], + warnings: [], + path: targetPath, + external: false, + sideEffects: targetPath.endsWith('.css'), + namespace: 'file', + suffix: '', + pluginData: null, + })); + + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + const result1 = await resolver(file1); + const result2 = await resolver(file2); + + expect(result1).toBeFalse(); + expect(result2).toBeTrue(); + expect(mockBuild.resolve).toHaveBeenCalledTimes(2); + + // Memoization of file-level result prevents second build.resolve call for the same file + const result1Cached = await resolver(file1); + expect(result1Cached).toBeFalse(); + expect(mockBuild.resolve).toHaveBeenCalledTimes(2); + }); + + it('should fallback to build.resolve per file when sideEffects is omitted', async () => { + const [file] = await createPackage('no-side-effects-pkg', { name: 'no-side-effects-pkg' }); + + mockBuild.resolve.and.callFake(async (targetPath: string) => ({ + errors: [], + warnings: [], + path: targetPath, + external: false, + sideEffects: false, + namespace: 'file', + suffix: '', + pluginData: null, + })); + + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + const result = await resolver(file); + expect(result).toBeFalse(); + expect(mockBuild.resolve).toHaveBeenCalledTimes(1); + + // Subsequent call uses file-level cache + const cachedResult = await resolver(file); + expect(cachedResult).toBeFalse(); + expect(mockBuild.resolve).toHaveBeenCalledTimes(1); + }); + }); + + describe('when package is scoped (@scope/pkg)', () => { + it('should correctly memoize package-level sideEffects for scoped packages', async () => { + const [file1, file2] = await createPackage('@angular/core', { sideEffects: false }, [ + 'index.js', + 'signals.js', + ]); + + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + expect(await resolver(file1)).toBeFalse(); + expect(await resolver(file2)).toBeFalse(); + expect(mockBuild.resolve).not.toHaveBeenCalled(); + }); + }); + + describe('when file is not in node_modules', () => { + it('should resolve via build.resolve and memoize file-level result', async () => { + const appFile = path.join(testDir, 'src', 'main.js'); + await mkdir(path.dirname(appFile), { recursive: true }); + await writeFile(appFile, '// app', 'utf-8'); + + mockBuild.resolve.and.callFake(async (targetPath: string) => ({ + errors: [], + warnings: [], + path: targetPath, + external: false, + sideEffects: true, + namespace: 'file', + suffix: '', + pluginData: null, + })); + + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + const result1 = await resolver(appFile); + expect(result1).toBeTrue(); + expect(mockBuild.resolve).toHaveBeenCalledTimes(1); + + const result2 = await resolver(appFile); + expect(result2).toBeTrue(); + expect(mockBuild.resolve).toHaveBeenCalledTimes(1); + }); + }); + + describe('when package.json is missing or corrupted', () => { + it('should fallback to build.resolve if package.json does not exist', async () => { + const pkgDir = path.join(testDir, 'node_modules', 'missing-pkg'); + await mkdir(pkgDir, { recursive: true }); + const file = path.join(pkgDir, 'index.js'); + await writeFile(file, '// test', 'utf-8'); + + mockBuild.resolve.and.callFake(async (targetPath: string) => ({ + errors: [], + warnings: [], + path: targetPath, + external: false, + sideEffects: false, + namespace: 'file', + suffix: '', + pluginData: null, + })); + + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + expect(await resolver(file)).toBeFalse(); + expect(mockBuild.resolve).toHaveBeenCalledTimes(1); + }); + }); + + describe('concurrency', () => { + it('should handle concurrent file resolutions for the same package cleanly', async () => { + const files = await createPackage('concurrent-pkg', { sideEffects: false }, [ + 'a.js', + 'b.js', + 'c.js', + 'd.js', + ]); + + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + const results = await Promise.all(files.map((file) => resolver(file))); + expect(results).toEqual([false, false, false, false]); + expect(mockBuild.resolve).not.toHaveBeenCalled(); + }); + + it('should coalesce concurrent build.resolve calls for the exact same file', async () => { + const appFile = path.join(testDir, 'src', 'shared.js'); + await mkdir(path.dirname(appFile), { recursive: true }); + await writeFile(appFile, '// shared', 'utf-8'); + + const resolver = createSideEffectsResolver(mockBuild as unknown as PluginBuild, true); + + const [r1, r2] = await Promise.all([resolver(appFile), resolver(appFile)]); + expect(r1).toBeTrue(); + expect(r2).toBeTrue(); + expect(mockBuild.resolve).toHaveBeenCalledTimes(1); + }); + }); +});