fix(@angular-devkit/build-angular): treeshake unused class that use custom decorators

alan-agius4 · alan-agius4 · commit 7b9b7fe1db17 · 2023-11-22T18:12:31.000+01:00
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.
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
@@ -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<boolean | undefined> {
+        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
@@ -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
@@ -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<Uint8Array> {
+  transformFile(
+    filename: string,
+    skipLinker?: boolean,
+    sideEffects?: boolean,
+  ): Promise<Uint8Array> {
     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<Uint8Array> {
+  async transformData(
+    filename: string,
+    data: string,
+    skipLinker: boolean,
+    sideEffects?: boolean,
+  ): Promise<Uint8Array> {
     // 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
@@ -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');
+}
