From 657a07bd6ba138a209c2a1540ea4d200c60e0f66 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 22 Nov 2023 16:56:51 +0000 Subject: [PATCH] fix(@angular-devkit/build-angular): treeshake unused class that use custom decorators This changes enables wrapping classes in side-effect free modules that make use of custom decorators when using the esbuild based builders so that when such classes are unused they can be treeshaken. (cherry picked from commit 7b9b7fe1db1727c0bbe1d4052d564a27f7c09de1) --- .../tools/esbuild/angular/compiler-plugin.ts | 20 ++++++++ .../esbuild/javascript-transformer-worker.ts | 11 ++--- .../tools/esbuild/javascript-transformer.ts | 17 ++++++- .../lib-unused-decorated-class-treeshake.ts | 49 +++++++++++++++++++ 4 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 tests/legacy-cli/e2e/tests/build/library/lib-unused-decorated-class-treeshake.ts diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts index 9e6546c38275..4d8dcbb46db6 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts @@ -358,10 +358,12 @@ export function createCompilerPlugin( }; } else if (typeof contents === 'string') { // A string indicates untransformed output from the TS/NG compiler + const sideEffects = await hasSideEffects(request); contents = await javascriptTransformer.transformData( request, contents, true /* skipLinker */, + sideEffects, ); // Store as the returned Uint8Array to allow caching the fully transformed code @@ -380,9 +382,11 @@ export function createCompilerPlugin( return profileAsync( 'NG_EMIT_JS*', async () => { + const sideEffects = await hasSideEffects(args.path); const contents = await javascriptTransformer.transformFile( args.path, pluginOptions.jit, + sideEffects, ); return { @@ -430,6 +434,22 @@ export function createCompilerPlugin( void stylesheetBundler.dispose(); void compilation.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_devkit/build_angular/src/tools/esbuild/javascript-transformer-worker.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/javascript-transformer-worker.ts index 4cfeed7b200a..27e724477486 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/javascript-transformer-worker.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/javascript-transformer-worker.ts @@ -17,7 +17,8 @@ interface JavaScriptTransformRequest { sourcemap: boolean; thirdPartySourcemaps: boolean; advancedOptimizations: boolean; - skipLinker: boolean; + skipLinker?: boolean; + sideEffects?: boolean; jit: boolean; } @@ -50,11 +51,8 @@ async function transformWithBabel({ return useInputSourcemap ? data : data.replace(/^\/\/# sourceMappingURL=[^\r\n]*/gm, ''); } - // `@angular/platform-server/init` and `@angular/common/locales/global` entry-points are side effectful. - const safeAngularPackage = - /[\\/]node_modules[\\/]@angular[\\/]/.test(filename) && - !/@angular[\\/]platform-server[\\/]f?esm2022[\\/]init/.test(filename) && - !/@angular[\\/]common[\\/]locales[\\/]global/.test(filename); + const sideEffectFree = options.sideEffects === false; + const safeAngularPackage = sideEffectFree && /[\\/]node_modules[\\/]@angular[\\/]/.test(filename); // Lazy load the linker plugin only when linking is required if (shouldLink) { @@ -85,6 +83,7 @@ async function transformWithBabel({ }, optimize: options.advancedOptimizations && { pureTopLevel: safeAngularPackage, + wrapDecorators: sideEffectFree, }, }, ], diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/javascript-transformer.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/javascript-transformer.ts index 5bd65c4af286..ca2bec585552 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/javascript-transformer.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/javascript-transformer.ts @@ -72,9 +72,14 @@ export class JavaScriptTransformer { * If no transformations are required, the data for the original file will be returned. * @param filename The full path to the file. * @param skipLinker If true, bypass all Angular linker processing; if false, attempt linking. + * @param sideEffects If false, and `advancedOptimizations` is enabled tslib decorators are wrapped. * @returns A promise that resolves to a UTF-8 encoded Uint8Array containing the result. */ - transformFile(filename: string, skipLinker?: boolean): Promise { + transformFile( + filename: string, + skipLinker?: boolean, + sideEffects?: boolean, + ): Promise { const pendingKey = `${!!skipLinker}--${filename}`; let pending = this.#pendingfileResults?.get(pendingKey); if (pending === undefined) { @@ -83,6 +88,7 @@ export class JavaScriptTransformer { pending = this.#ensureWorkerPool().run({ filename, skipLinker, + sideEffects, ...this.#commonOptions, }); @@ -98,9 +104,15 @@ export class JavaScriptTransformer { * @param filename The full path of the file represented by the data. * @param data The data of the file that should be transformed. * @param skipLinker If true, bypass all Angular linker processing; if false, attempt linking. + * @param sideEffects If false, and `advancedOptimizations` is enabled tslib decorators are wrapped. * @returns A promise that resolves to a UTF-8 encoded Uint8Array containing the result. */ - async transformData(filename: string, data: string, skipLinker: boolean): Promise { + async transformData( + filename: string, + data: string, + skipLinker: boolean, + sideEffects?: boolean, + ): Promise { // Perform a quick test to determine if the data needs any transformations. // This allows directly returning the data without the worker communication overhead. if (skipLinker && !this.#commonOptions.advancedOptimizations) { @@ -118,6 +130,7 @@ export class JavaScriptTransformer { filename, data, skipLinker, + sideEffects, ...this.#commonOptions, }); } diff --git a/tests/legacy-cli/e2e/tests/build/library/lib-unused-decorated-class-treeshake.ts b/tests/legacy-cli/e2e/tests/build/library/lib-unused-decorated-class-treeshake.ts new file mode 100644 index 000000000000..f1071469ad75 --- /dev/null +++ b/tests/legacy-cli/e2e/tests/build/library/lib-unused-decorated-class-treeshake.ts @@ -0,0 +1,49 @@ +import assert from 'assert'; +import { appendToFile, expectFileToExist, expectFileToMatch, readFile } from '../../../utils/fs'; +import { ng } from '../../../utils/process'; +import { libraryConsumptionSetup } from './setup'; +import { updateJsonFile } from '../../../utils/project'; +import { expectToFail } from '../../../utils/utils'; + +export default async function () { + await ng('cache', 'off'); + await libraryConsumptionSetup(); + + // Add an unused class as part of the public api. + await appendToFile( + 'projects/my-lib/src/public-api.ts', + ` + function something() { + return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) { + console.log("someDecorator"); + }; + } + + export class ExampleClass { + @something() + method() {} + } + `, + ); + + // build the lib + await ng('build', 'my-lib', '--configuration=production'); + const packageJson = JSON.parse(await readFile('dist/my-lib/package.json')); + assert.equal(packageJson.sideEffects, false); + + // build the app + await ng('build', 'test-project', '--configuration=production', '--output-hashing=none'); + // Output should not contain `ExampleClass` as the library is marked as side-effect free. + await expectFileToExist('dist/test-project/browser/main.js'); + await expectToFail(() => expectFileToMatch('dist/test-project/browser/main.js', 'someDecorator')); + + // Mark library as side-effectful. + await updateJsonFile('dist/my-lib/package.json', (packageJson) => { + packageJson.sideEffects = true; + }); + + // build the app + await ng('build', 'test-project', '--configuration=production', '--output-hashing=none'); + // Output should contain `ExampleClass` as the library is marked as side-effectful. + await expectFileToMatch('dist/test-project/browser/main.js', 'someDecorator'); +}