fix(compiler-cli): disable tree shaking during HMR (#59595)

crisbeto · pkozlowski-opensource · commit 95a05bb2021a · 2025-01-20T08:59:30.000+01:00
When HMR is enabled, we need to capture the dependencies used in a template and forward them to the HMR replacement function. One half of this process is static, meaning that we can't change it after the initial compilation. Tree shaking becomes a problem in such a case, because the user can change the template in a way that changes the set of dependencies which will start matching with the static part of the HMR code. These changes disable the tree shaking when HMR is enabled to ensure that the dependencies stay stable. Fixes #59581. PR Close #59595
diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts
@@ -1222,14 +1222,25 @@ export class ComponentDecoratorHandler
       // Register all Directives and Pipes used at the top level (outside
       // of any defer blocks), which would be eagerly referenced.
       const eagerlyUsed = new Set<ClassDeclaration>();
-      for (const dir of bound.getEagerlyUsedDirectives()) {
-        eagerlyUsed.add(dir.ref.node);
-      }
-      for (const name of bound.getEagerlyUsedPipes()) {
-        if (!pipes.has(name)) {
-          continue;
+
+      if (this.enableHmr) {
+        // In HMR we need to preserve all the dependencies, because they have to remain consistent
+        // with the initially-generated code no matter what the template looks like.
+        for (const dep of dependencies) {
+          if (dep.ref.node !== node) {
+            eagerlyUsed.add(dep.ref.node);
+          }
+        }
+      } else {
+        for (const dir of bound.getEagerlyUsedDirectives()) {
+          eagerlyUsed.add(dir.ref.node);
+        }
+        for (const name of bound.getEagerlyUsedPipes()) {
+          if (!pipes.has(name)) {
+            continue;
+          }
+          eagerlyUsed.add(pipes.get(name)!.ref.node);
         }
-        eagerlyUsed.add(pipes.get(name)!.ref.node);
       }
 
       // Set of Directives and Pipes used across the entire template,
diff --git a/packages/compiler-cli/test/ngtsc/hmr_spec.ts b/packages/compiler-cli/test/ngtsc/hmr_spec.ts
@@ -291,7 +291,7 @@ runInEachFileSystem(() => {
       expect(jsContents).toContain('i0.ɵɵdefer(1, 0, Cmp_Defer_1_DepsFn);');
 
       expect(hmrContents).toContain(
-        'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, Component, Dep) {',
+        'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, Dep, Component) {',
       );
       expect(hmrContents).toContain('const Cmp_Defer_1_DepsFn = () => [Dep];');
       expect(hmrContents).toContain('function Cmp_Defer_0_Template(rf, ctx) {');
@@ -502,5 +502,153 @@ runInEachFileSystem(() => {
         'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, token, value, Component) {',
       );
     });
+
+    it('should preserve eager standalone imports in HMR even if they are not used in the template', () => {
+      enableHmr({
+        // Disable class metadata since it can add noise to the test.
+        supportTestBed: false,
+        extendedDiagnostics: {
+          checks: {
+            // Disable the diagnostic that flags standalone imports since
+            // we need one to simulate the case we're looking for.
+            unusedStandaloneImports: 'suppress',
+          },
+        },
+      });
+
+      env.write(
+        'dep.ts',
+        `
+          import {Directive} from '@angular/core';
+
+          @Directive({selector: '[dep]'})
+          export class Dep {}
+        `,
+      );
+
+      env.write(
+        'test.ts',
+        `
+          import {Component} from '@angular/core';
+          import {Dep} from './dep';
+
+          @Component({
+            selector: 'cmp',
+            template: '',
+            imports: [Dep],
+          })
+          export class Cmp {}
+        `,
+      );
+
+      env.driveMain();
+
+      const jsContents = env.getContents('test.js');
+      const hmrContents = env.driveHmr('test.ts', 'Cmp');
+
+      expect(jsContents).toContain('dependencies: [Dep]');
+      expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0], [Dep]));');
+      expect(hmrContents).toContain('function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, Dep) {');
+    });
+
+    it('should preserve eager module imports inside standalone component in HMR even if they are not used in the template', () => {
+      enableHmr({
+        // Disable class metadata since it can add noise to the test.
+        supportTestBed: false,
+      });
+
+      env.write(
+        'dep.ts',
+        `
+          import {NgModule, Directive} from '@angular/core';
+
+          @Directive({selector: '[dep]', standalone: false})
+          export class Dep {}
+
+          @NgModule({declarations: [Dep], exports: [Dep]})
+          export class DepModule {}
+        `,
+      );
+
+      env.write(
+        'test.ts',
+        `
+          import {Component} from '@angular/core';
+          import {DepModule} from './dep';
+
+          @Component({
+            selector: 'cmp',
+            template: '',
+            imports: [DepModule],
+          })
+          export class Cmp {}
+        `,
+      );
+
+      env.driveMain();
+
+      const jsContents = env.getContents('test.js');
+      const hmrContents = env.driveHmr('test.ts', 'Cmp');
+
+      expect(jsContents).toContain('dependencies: [DepModule, i1.Dep]');
+      expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0, i1], [DepModule]));');
+      expect(hmrContents).toContain('function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, DepModule) {');
+    });
+
+    it('should preserve eager module imports inside non-standalone component in HMR even if they are not used in the template', () => {
+      enableHmr({
+        // Disable class metadata since it can add noise to the test.
+        supportTestBed: false,
+      });
+
+      env.write(
+        'dep.ts',
+        `
+          import {NgModule, Directive} from '@angular/core';
+
+          @Directive({selector: '[dep]', standalone: false})
+          export class Dep {}
+
+          @NgModule({declarations: [Dep], exports: [Dep]})
+          export class DepModule {}
+        `,
+      );
+
+      env.write(
+        'test-module.ts',
+        `
+        import {NgModule} from '@angular/core';
+        import {Cmp} from './test';
+        import {DepModule} from './dep';
+
+        @NgModule({imports: [DepModule], declarations: [Cmp], exports: [Cmp]})
+        export class CmpModule {}
+      `,
+      );
+
+      env.write(
+        'test.ts',
+        `
+          import {Component} from '@angular/core';
+          import {DepModule} from './dep';
+
+          @Component({
+            selector: 'cmp',
+            template: '',
+            standalone: false,
+          })
+          export class Cmp {}
+        `,
+      );
+
+      env.driveMain();
+
+      const jsContents = env.getContents('test.js');
+      const hmrContents = env.driveHmr('test.ts', 'Cmp');
+
+      expect(jsContents).toContain('dependencies: [i1.Dep]');
+      expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0, i1], []));');
+      expect(hmrContents).toContain('function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces) {');
+    });
   });
 });
