fix(compiler-cli): handle const enums used inside HMR data (#59815)

When we generate an HMR replacement function, we determine which locals from the file are used and we pass them by reference. This works fine in most cases, but breaks down for const enums which don't have a runtime representation.

These changes work around the issue by passing in all the values as an object literal.

Fixes #59800.

PR Close #59815

Author: crisbeto

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

@@ -1659,6 +1659,7 @@ export class ComponentDecoratorHandler
       ? extractHmrMetatadata(
           node,
           this.reflector,
+          this.evaluator,
           this.compilerHost,
           this.rootDirs,
           def,
@@ -1725,6 +1726,7 @@ export class ComponentDecoratorHandler
       ? extractHmrMetatadata(
           node,
           this.reflector,
+          this.evaluator,
           this.compilerHost,
           this.rootDirs,
           def,
@@ -1787,6 +1789,7 @@ export class ComponentDecoratorHandler
       ? extractHmrMetatadata(
           node,
           this.reflector,
+          this.evaluator,
           this.compilerHost,
           this.rootDirs,
           def,
@@ -1843,6 +1846,7 @@ export class ComponentDecoratorHandler
       ? extractHmrMetatadata(
           node,
           this.reflector,
+          this.evaluator,
           this.compilerHost,
           this.rootDirs,
           def,

packages/compiler-cli/src/ngtsc/hmr/BUILD.bazel

@@ -9,6 +9,7 @@ ts_library(
     ]),
     deps = [
         "//packages/compiler",
+        "//packages/compiler-cli/src/ngtsc/partial_evaluator",
         "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/compiler-cli/src/ngtsc/transform",
         "//packages/compiler-cli/src/ngtsc/translator",

packages/compiler-cli/src/ngtsc/hmr/src/extract_dependencies.ts

@@ -13,9 +13,10 @@ import {
   R3HmrNamespaceDependency,
   outputAst as o,
 } from '@angular/compiler';
-import {DeclarationNode} from '../../reflection';
+import {DeclarationNode, ReflectionHost} from '../../reflection';
 import {CompileResult} from '../../transform';
 import ts from 'typescript';
+import {EnumValue, PartialEvaluator} from '../../partial_evaluator';
 
 /**
  * Determines the file-level dependencies that the HMR initializer needs to capture and pass along.
@@ -33,7 +34,12 @@ export function extractHmrDependencies(
   deferBlockMetadata: R3ComponentDeferMetadata,
   classMetadata: o.Statement | null,
   debugInfo: o.Statement | null,
-): {local: string[]; external: R3HmrNamespaceDependency[]} {
+  reflection: ReflectionHost,
+  evaluator: PartialEvaluator,
+): {
+  local: {name: string; runtimeRepresentation: o.Expression}[];
+  external: R3HmrNamespaceDependency[];
+} {
   const name = ts.isClassDeclaration(node) && node.name ? node.name.text : null;
   const visitor = new PotentialTopLevelReadsVisitor();
   const sourceFile = node.getSourceFile();
@@ -57,16 +63,73 @@ export function extractHmrDependencies(
   // variables inside of functions. Note that we filter out the class name since it is always
   // defined and it saves us having to repeat this logic wherever the locals are consumed.
   const availableTopLevel = getTopLevelDeclarationNames(sourceFile);
+  const local: {name: string; runtimeRepresentation: o.Expression}[] = [];
+  const seenLocals = new Set<string>();
+
+  for (const readNode of visitor.allReads) {
+    const readName = readNode instanceof o.ReadVarExpr ? readNode.name : readNode.text;
+
+    if (readName !== name && !seenLocals.has(readName) && availableTopLevel.has(readName)) {
+      local.push({
+        name: readName,
+        runtimeRepresentation: getRuntimeRepresentation(readNode, reflection, evaluator),
+      });
+      seenLocals.add(readName);
+    }
+  }
 
   return {
-    local: Array.from(visitor.allReads).filter((r) => r !== name && availableTopLevel.has(r)),
+    local,
     external: Array.from(visitor.namespaceReads, (name, index) => ({
       moduleName: name,
       assignedName: `ɵhmr${index}`,
     })),
   };
 }
 
+/**
+ * Gets a node that can be used to represent an identifier in the HMR replacement code at runtime.
+ */
+function getRuntimeRepresentation(
+  node: o.ReadVarExpr | ts.Identifier,
+  reflection: ReflectionHost,
+  evaluator: PartialEvaluator,
+): o.Expression {
+  if (node instanceof o.ReadVarExpr) {
+    return o.variable(node.name);
+  }
+
+  // Const enums can't be passed by reference, because their values are inlined.
+  // Pass in an object literal with all of the values instead.
+  if (isConstEnumReference(node, reflection)) {
+    const evaluated = evaluator.evaluate(node);
+
+    if (evaluated instanceof Map) {
+      const members: {key: string; quoted: boolean; value: o.Expression}[] = [];
+
+      for (const [name, value] of evaluated.entries()) {
+        if (
+          value instanceof EnumValue &&
+          (value.resolved == null ||
+            typeof value.resolved === 'string' ||
+            typeof value.resolved === 'boolean' ||
+            typeof value.resolved === 'number')
+        ) {
+          members.push({
+            key: name,
+            quoted: false,
+            value: o.literal(value.resolved),
+          });
+        }
+      }
+
+      return o.literalMap(members);
+    }
+  }
+
+  return o.variable(node.text);
+}
+
 /**
  * Gets the names of all top-level declarations within the file (imports, declared classes etc).
  * @param sourceFile File in which to search for locals.
@@ -81,8 +144,7 @@ function getTopLevelDeclarationNames(sourceFile: ts.SourceFile): Set<string> {
     if (
       ts.isClassDeclaration(node) ||
       ts.isFunctionDeclaration(node) ||
-      (ts.isEnumDeclaration(node) &&
-        !node.modifiers?.some((m) => m.kind === ts.SyntaxKind.ConstKeyword))
+      ts.isEnumDeclaration(node)
     ) {
       if (node.name) {
         results.add(node.name.text);
@@ -157,7 +219,7 @@ function trackBindingName(node: ts.BindingName, results: Set<string>): void {
  * inside functions.
  */
 class PotentialTopLevelReadsVisitor extends o.RecursiveAstVisitor {
-  readonly allReads = new Set<string>();
+  readonly allReads = new Set<o.ReadVarExpr | ts.Identifier>();
   readonly namespaceReads = new Set<string>();
 
   override visitExternalExpr(ast: o.ExternalExpr, context: any) {
@@ -168,7 +230,7 @@ class PotentialTopLevelReadsVisitor extends o.RecursiveAstVisitor {
   }
 
   override visitReadVarExpr(ast: o.ReadVarExpr, context: any) {
-    this.allReads.add(ast.name);
+    this.allReads.add(ast);
     super.visitReadVarExpr(ast, context);
   }
 
@@ -186,7 +248,7 @@ class PotentialTopLevelReadsVisitor extends o.RecursiveAstVisitor {
    */
   private addAllTopLevelIdentifiers = (node: ts.Node) => {
     if (ts.isIdentifier(node) && this.isTopLevelIdentifierReference(node)) {
-      this.allReads.add(node.text);
+      this.allReads.add(node);
     } else {
       ts.forEachChild(node, this.addAllTopLevelIdentifiers);
     }
@@ -326,3 +388,25 @@ class PotentialTopLevelReadsVisitor extends o.RecursiveAstVisitor {
     return !!value && typeof value.kind === 'number';
   }
 }
+
+/** Checks whether a node is a reference to a const enum. */
+function isConstEnumReference(node: ts.Identifier, reflection: ReflectionHost): boolean {
+  const parent = node.parent;
+
+  // Only check identifiers that are in the form of `Foo.bar` where `Foo` is the node being checked.
+  if (
+    !parent ||
+    !ts.isPropertyAccessExpression(parent) ||
+    parent.expression !== node ||
+    !ts.isIdentifier(parent.name)
+  ) {
+    return false;
+  }
+
+  const declaration = reflection.getDeclarationOfIdentifier(node);
+  return (
+    declaration !== null &&
+    ts.isEnumDeclaration(declaration.node) &&
+    !!declaration.node.modifiers?.some((m) => m.kind === ts.SyntaxKind.ConstKeyword)
+  );
+}

packages/compiler-cli/src/ngtsc/hmr/src/metadata.ts

@@ -17,6 +17,7 @@ import {getProjectRelativePath} from '../../util/src/path';
 import {CompileResult} from '../../transform';
 import {extractHmrDependencies} from './extract_dependencies';
 import ts from 'typescript';
+import {PartialEvaluator} from '../../partial_evaluator';
 
 /**
  * Extracts the HMR metadata for a class declaration.
@@ -33,6 +34,7 @@ import ts from 'typescript';
 export function extractHmrMetatadata(
   clazz: DeclarationNode,
   reflection: ReflectionHost,
+  evaluator: PartialEvaluator,
   compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
   rootDirs: readonly string[],
   definition: R3CompiledExpression,
@@ -57,6 +59,8 @@ export function extractHmrMetatadata(
     deferBlockMetadata,
     classMetadata,
     debugInfo,
+    reflection,
+    evaluator,
   );
   const meta: R3HmrMetadata = {
     type: new o.WrappedNodeExpr(clazz.name),

packages/compiler-cli/test/ngtsc/hmr_spec.ts

@@ -800,5 +800,92 @@ runInEachFileSystem(() => {
       expect(jsContents).toContain('ɵɵreplaceMetadata(Cmp, m.default, [i0, i1], []));');
       expect(hmrContents).toContain('function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces) {');
     });
+
+    it('should pass const enums defined in the same file as an object literal', () => {
+      enableHmr();
+      env.write(
+        'test.ts',
+        `
+          import {Component, InjectionToken} from '@angular/core';
+
+          const token = new InjectionToken<number>('TEST');
+
+          const numberThree = 3;
+
+          export const enum Foo {
+            one,
+            two = '2',
+            three = numberThree
+          }
+
+          @Component({
+            template: '',
+            providers: [{
+              provide: token,
+              useValue: Foo.three
+            }]
+          })
+          export class Cmp {}
+        `,
+      );
+
+      env.driveMain();
+
+      const jsContents = env.getContents('test.js');
+      const hmrContents = env.driveHmr('test.ts', 'Cmp');
+      expect(jsContents).toContain(
+        'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, { one: 0, two: "2", three: 3 }, Component]));',
+      );
+      expect(hmrContents).toContain(
+        'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, token, Foo, Component) {',
+      );
+    });
+
+    it('should pass const enum defined in other file as an object literal', () => {
+      enableHmr();
+
+      env.write(
+        'deps.ts',
+        `
+        const numberThree = 3;
+
+        export const enum Foo {
+          one,
+          two = '2',
+          three = numberThree
+        }
+      `,
+      );
+
+      env.write(
+        'test.ts',
+        `
+          import {Component, InjectionToken} from '@angular/core';
+          import {Foo} from './deps';
+
+          const token = new InjectionToken<number>('TEST');
+
+          @Component({
+            template: '',
+            providers: [{
+              provide: token,
+              useValue: Foo.three
+            }]
+          })
+          export class Cmp {}
+        `,
+      );
+
+      env.driveMain();
+
+      const jsContents = env.getContents('test.js');
+      const hmrContents = env.driveHmr('test.ts', 'Cmp');
+      expect(jsContents).toContain(
+        'ɵɵreplaceMetadata(Cmp, m.default, [i0], [token, { one: 0, two: "2", three: 3 }, Component]));',
+      );
+      expect(hmrContents).toContain(
+        'export default function Cmp_UpdateMetadata(Cmp, ɵɵnamespaces, token, Foo, Component) {',
+      );
+    });
   });
 });

packages/compiler/src/render3/r3_hmr_compiler.ts

@@ -31,9 +31,9 @@ export interface R3HmrMetadata {
   /**
    * HMR update functions cannot contain imports so any locals the generated code depends on
    * (e.g. references to imports within the same file or imported symbols) have to be passed in
-   * as function parameters. This array contains the names of those local symbols.
+   * as function parameters. This array contains the names and runtime representation of the locals.
    */
-  localDependencies: string[];
+  localDependencies: {name: string; runtimeRepresentation: o.Expression}[];
 }
 
 /** HMR dependency on a namespace import. */
@@ -59,7 +59,6 @@ export function compileHmrInitializer(meta: R3HmrMetadata): o.Expression {
   const dataName = 'd';
   const timestampName = 't';
   const importCallbackName = `${meta.className}_HmrLoad`;
-  const locals = meta.localDependencies.map((localName) => o.variable(localName));
   const namespaces = meta.namespaceDependencies.map((dep) => {
     return new o.ExternalExpr({moduleName: dep.moduleName, name: null});
   });
@@ -70,7 +69,12 @@ export function compileHmrInitializer(meta: R3HmrMetadata): o.Expression {
   // ɵɵreplaceMetadata(Comp, m.default, [...namespaces], [...locals]);
   const replaceCall = o
     .importExpr(R3.replaceMetadata)
-    .callFn([meta.type, defaultRead, o.literalArr(namespaces), o.literalArr(locals)]);
+    .callFn([
+      meta.type,
+      defaultRead,
+      o.literalArr(namespaces),
+      o.literalArr(meta.localDependencies.map((l) => l.runtimeRepresentation)),
+    ]);
 
   // (m) => m.default && ɵɵreplaceMetadata(...)
   const replaceCallback = o.arrowFn([new o.FnParam(moduleName)], defaultRead.and(replaceCall));
@@ -159,11 +163,13 @@ export function compileHmrUpdateCallback(
   meta: R3HmrMetadata,
 ): o.DeclareFunctionStmt {
   const namespaces = 'ɵɵnamespaces';
-  const params = [meta.className, namespaces, ...meta.localDependencies].map(
-    (name) => new o.FnParam(name, o.DYNAMIC_TYPE),
-  );
+  const params = [meta.className, namespaces].map((name) => new o.FnParam(name, o.DYNAMIC_TYPE));
   const body: o.Statement[] = [];
 
+  for (const local of meta.localDependencies) {
+    params.push(new o.FnParam(local.name));
+  }
+
   // Declare variables that read out the individual namespaces.
   for (let i = 0; i < meta.namespaceDependencies.length; i++) {
     body.push(