fix(compiler-cli): ensure component import diagnostics are reported within the imports expression

JoostK · crisbeto · commit 4d9c4567edfb · 2026-01-02T08:21:12.000+01:00
PR #60455 improved error reporting for `@Component.imports` by scoping the diagnostic to an individual element within the `imports` array, but this may introduce hard to track diagnostics when it ends up being reported (far) away from the component itself. This can be even more problematic when the diagnostic would end up being reported in a declaration file, as happened in issue #65686; the declaration files of an imported library contained syntax that the static interpreter did not support, hence the `@Component.imports` was rejected with a diagnostic reported in the library's declaration file. This diagnostic isn't guaranteed to be reported (e.g. the CLI only gathers Angular-specific diagnostics for Angular-compiled files, which excludes declaration files). This commit changes the diagnostic location to ensure it is being reported within the `@Component.imports` expression, in most cases retaining the desirable effect of #60455 while avoiding out-of-band diagnostics. (cherry picked from commit 106ba63)
diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/util.ts
@@ -99,10 +99,18 @@ export function validateAndFlattenComponentImports(
 
   for (let i = 0; i < imports.length; i++) {
     const ref = imports[i];
+    let refExpr = expr;
+    if (
+      ts.isArrayLiteralExpression(expr) &&
+      expr.elements.length === imports.length &&
+      !expr.elements.some(ts.isSpreadAssignment)
+    ) {
+      refExpr = expr.elements[i];
+    }
 
     if (Array.isArray(ref)) {
       const {imports: childImports, diagnostics: childDiagnostics} =
-        validateAndFlattenComponentImports(ref, expr, isDeferred);
+        validateAndFlattenComponentImports(ref, refExpr, isDeferred);
       flattened.push(...childImports);
       diagnostics.push(...childDiagnostics);
     } else if (ref instanceof Reference) {
@@ -138,20 +146,16 @@ export function validateAndFlattenComponentImports(
       let diagnosticNode: ts.Node;
       let diagnosticValue: ResolvedValue;
 
-      if (ref instanceof DynamicValue) {
+      // Reporting a diagnostic on the entire array can be noisy, especially if the user has a
+      // large array. Attempt to determine the most accurate position within the `imports` expression to report the
+      // diagnostic on.
+      if (ref instanceof DynamicValue && isWithinExpression(ref.node, expr)) {
+        // Use the dynamic value position itself if it occurs within the `imports` expression.
         diagnosticNode = ref.node;
         diagnosticValue = ref;
-      } else if (
-        ts.isArrayLiteralExpression(expr) &&
-        expr.elements.length === imports.length &&
-        !expr.elements.some(ts.isSpreadAssignment) &&
-        !imports.some(Array.isArray)
-      ) {
-        // Reporting a diagnostic on the entire array can be noisy, especially if the user has a
-        // large array. The most common case is that the array will be static so we can reliably
-        // trace back a `ResolvedValue` back to its node using its index. This allows us to report
-        // the exact node that caused the issue.
-        diagnosticNode = expr.elements[i];
+      } else if (refExpr !== expr) {
+        // The reference comes from a specific element in `expr`, so use that element to report the diagnostic on.
+        diagnosticNode = refExpr;
         diagnosticValue = ref;
       } else {
         diagnosticNode = expr;
@@ -167,6 +171,17 @@ export function validateAndFlattenComponentImports(
   return {imports: flattened, diagnostics};
 }
 
+function isWithinExpression(node: ts.Node, expr: ts.Expression): boolean {
+  let current: ts.Node | undefined = node;
+  while (current !== undefined) {
+    if (current === expr) {
+      return true;
+    }
+    current = current.parent;
+  }
+  return false;
+}
+
 /**
  * Inspects `value` to determine if it resembles a `ModuleWithProviders` value. This is an
  * approximation only suitable for error reporting as any resolved object with an `ngModule`
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -2589,12 +2589,70 @@ runInEachFileSystem((os: string) => {
           export class HelloDir {}
 
           const someVar = {} as any;
+          const tuple = [() => {}] as const;
 
           @Component({
             template: '<div hello></div>',
             imports: [
               someVar,
               HelloDir,
+              'invalid',
+              tuple,
+            ]
+          })
+          export class TestCmp {}
+        `,
+      );
+
+      const diags = env.driveDiagnostics();
+      expect(diags.length).toBe(3);
+      {
+        const message = ts.flattenDiagnosticMessageText(diags[0].messageText, '\n');
+        expect(getDiagnosticSourceCode(diags[0])).toBe('someVar');
+        expect(message).toContain(
+          `'imports' must be an array of components, directives, pipes, or NgModules.`,
+        );
+        expect(message).toContain(`Value is of type '{}'`);
+      }
+      {
+        const message = ts.flattenDiagnosticMessageText(diags[1].messageText, '\n');
+        expect(getDiagnosticSourceCode(diags[1])).toBe(`'invalid'`);
+        expect(message).toContain(
+          `'imports' must be an array of components, directives, pipes, or NgModules.`,
+        );
+        expect(message).toContain(`Value is of type 'string'`);
+      }
+      {
+        const message = ts.flattenDiagnosticMessageText(diags[2].messageText, '\n');
+        expect(getDiagnosticSourceCode(diags[2])).toBe('tuple');
+        expect(message).toContain(
+          `'imports' must be an array of components, directives, pipes, or NgModules.`,
+        );
+        expect(message).toContain(`Value is of type '[(not statically analyzable)]'.`);
+      }
+    });
+
+    it('should report imports diagnostic for declaration file in original expression', () => {
+      env.write(
+        'node_modules/external/index.d.ts',
+        `
+        export declare const UNRESOLVED_ITEM: readonly [unresolved];
+      `,
+      );
+      env.write(
+        'test.ts',
+        `
+          import {Component, Directive} from '@angular/core';
+          import {UNRESOLVED_ITEM as Unresolved} from 'external';
+
+          @Directive({selector: '[hello]'})
+          export class HelloDir {}
+
+          @Component({
+            template: '<div hello></div>',
+            imports: [
+              [Unresolved],
+              HelloDir,
             ]
           })
           export class TestCmp {}
@@ -2606,11 +2664,12 @@ runInEachFileSystem((os: string) => {
         ? ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')
         : '';
       expect(diags.length).toBe(1);
-      expect(getDiagnosticSourceCode(diags[0])).toBe('someVar');
+      expect(diags[0].file!.fileName).toContain('test.ts');
+      expect(getDiagnosticSourceCode(diags[0])).toBe('Unresolved');
       expect(message).toContain(
         `'imports' must be an array of components, directives, pipes, or NgModules.`,
       );
-      expect(message).toContain(`Value is of type '{}'`);
+      expect(message).toContain(`Value is of type '[(not statically analyzable)]'.`);
     });
 
     describe('empty and missing selectors', () => {
