refactor(compiler-cli): detect unused standalone imports in local arrays by SkyZeroZx · Pull Request #66620 · angular/angular

SkyZeroZx · 2026-01-17T03:15:13Z

Extends the unused standalone imports rule to correctly identify and report unused imports that are part of local arrays (with or without spread syntax) or assigned to a local array identifier.

Fixes #59754

What is the new behavior?

const sharedImports = [UnusedComponent, UsedComponent];

@Component({
  selector: 'app-using-array-imports',
  imports: sharedImports, //  no warning (at least one import is used)
  template: '<used-comp />',
})
export class UsingArrayImports {}


const unusedImports = [UnusedComponent , Other];

@Component({
  selector: 'app-array-all-unused',
  imports: unusedImports, //  NG8113: all imports in this array are unused
  template: ' ',
})
export class ArrayAllUnused {}

JeanMeche · 2026-01-19T12:36:54Z

As mentionned in #59754, this doesn't address this mentionned issue. It should flag arrays where all of its elements are unused.

SkyZeroZx · 2026-01-19T14:57:12Z

As mentionned in #59754, this doesn't address this mentionned issue. It should flag arrays where all of its elements are unused.

I didn’t mention it explicitly in the example, nor did I include a test for it (which I should add).

However, the implementation also detects the case where none of the elements in the array are used and emits the corresponding warning

const localmports= [UnusedComponent, Another];

@Component({
  selector: 'app',
  imports: localmports, // NG8113: All imports are unused
  template: ' ',
})
export class App {}

This still intentionally omits the case of export const exportedArray = [ /* ... */ ], which is currently assumed to exist for sharing purposes.

We also include this case and emit a warning only when none of its elements are used? That also seems useful to me.

JeanMeche · 2026-01-19T16:50:02Z

Having a single unused directive in the array could be an acceptable trade of instead of having all the individual imports.
That makes the diagnostic less usable for those would want to only flag if all the array elements are unused.

Extends the unused standalone imports rule to correctly identify and report unused imports that are part of local arrays (with or without spread syntax) or assigned to a local array identifier, only in cases where all imports in the array are unused. Fixes angular#59754

SkyZeroZx · 2026-01-19T19:12:44Z

Having a single unused directive in the array could be an acceptable trade of instead of having all the individual imports. That makes the diagnostic less usable for those would want to only flag if all the array elements are unused.

I’ve just pushed an update. In addition, there were some existing tests that validated the behavior of reporting when a single array element was unused; those have been updated as well to align with this new behavior.

JeanMeche · 2026-01-20T02:36:11Z

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

+        expect(diags[0].messageText).toBe(
+          'DirectUnused is not used within the template of TestComponent',
+        );
+      });


I have never tried implementing such test but can you look if our test infra could simulate importing an array exposed by a library.

I think this would be equivalent, since it’s conceptually similar to importing something from another file:

export const exportedArray = [ /* ... */ ]

At the moment this case is not detected because exported arrays are not considered by the diagnosis and are therefore kept as-is.

I can take a look at whether our test infrastructure can simulate importing an array exposed by a library.

On the other hand, I noticed that using:

const arrayImport = [ /* ... */ ] as const; // after using don't emit anything

also bypasses the diagnostic, we could consider this acceptable and leave it as-is, or should we still emit a diagnostic? This could also be seen as an intentional workaround for users who want to explicitly opt out.

I tried simulating this in the test infra by writing a fake library .d.ts under node_modules that exports a directives

I ran the test and it works. Conceptually, this should behave the same as how libraries are consumed.

env.write( 'node_modules/fake-external-lib/index.d.ts', ` import * as i0 from '@angular/core'; export declare class FakeButton { static ɵdir: i0.ɵɵDirectiveDeclaration<ButtonDirective, "[libButton]", never, {}, {}, never, never, true, never>; static ɵfac: i0.ɵɵFactoryDeclaration<ButtonDirective, never>; } export declare class FakeTooltip { static ɵdir: i0.ɵɵDirectiveDeclaration<TooltipDirective, "[libTooltip]", never, {}, {}, never, never, true, never>; static ɵfac: i0.ɵɵFactoryDeclaration<TooltipDirective, never>; } export declare const UI_DIRECTIVES: [typeof FakeButton , typeof FakeTooltip ]; `, ); env.write( 'test.ts', ` import {Component} from '@angular/core'; import {UI_DIRECTIVES} from 'fake-external-lib'; @Component({ selector: 'app-test', imports: [UI_DIRECTIVES], template: \` \`, }) export class Test {} `, ); const diags = env.driveDiagnostics(); expect(diags.length).toBe(0);

JeanMeche · 2026-02-10T00:34:34Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the unused standalone imports rule to correctly handle imports from local arrays. The changes introduce new logic to detect when all imports in a local array are unused and avoid reporting individual unused imports from shared arrays to reduce noise. The implementation looks solid and is well-tested. I have a couple of suggestions to improve correctness and performance in the new helper methods.

gemini-code-assist · 2026-02-10T00:36:57Z

packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts

+  private findVariableDeclaration(
+    identifier: ts.Identifier,
+    sourceFile: ts.SourceFile,
+  ): ts.VariableDeclaration | null {
+    const targetText = identifier.text;
+    let foundDeclaration: ts.VariableDeclaration | null = null;
+
+    const visit = (node: ts.Node): void => {
+      if (foundDeclaration) {
+        return;
+      }
+
+      if (
+        ts.isVariableDeclaration(node) &&
+        ts.isIdentifier(node.name) &&
+        node.name.text === targetText
+      ) {
+        if (node.name.pos < identifier.pos) {
+          foundDeclaration = node;
+          return;
+        }
+      }
+
+      ts.forEachChild(node, visit);
+    };
+
+    visit(sourceFile);
+    return foundDeclaration;
+  }


The current implementation for finding a variable declaration by traversing the AST and picking the first match can be brittle and incorrect in the presence of variable shadowing. A more robust approach is to use the TypeScript TypeChecker to resolve the symbol at the identifier's location. This correctly handles scoping and aliasing. Note that the sourceFile parameter becomes unused with this approach and could be removed in a follow-up.

private findVariableDeclaration( identifier: ts.Identifier, sourceFile: ts.SourceFile, ): ts.VariableDeclaration | null { const typeChecker = this.templateTypeChecker.typeChecker; const symbol = typeChecker.getSymbolAtLocation(identifier); if (!symbol) { return null; } const resolvedSymbol = symbol.flags & ts.SymbolFlags.Alias ? typeChecker.getAliasedSymbol(symbol) : symbol; if (resolvedSymbol.valueDeclaration && ts.isVariableDeclaration(resolvedSymbol.valueDeclaration)) { return resolvedSymbol.valueDeclaration; } return null; }

The current implementation for finding a variable declaration by traversing the AST and picking the first match can be brittle and incorrect in the presence of variable shadowing.

I don't think this would be necessary considering I don't think it would happen; we would have to write something like this:

function some() { const SHARED_IMPORTS = [ComponentA]; // Different scope } const SHARED_IMPORTS = [ComponentB]; // File level @Component({ imports: SHARED_IMPORTS // Uses file-level one }) export class Comp {}

It would also be necessary to make modifications to access type Checker, which would involve more changes that I don't think are necessary in this case.

gemini-code-assist · 2026-02-10T00:36:58Z

packages/compiler-cli/src/ngtsc/validation/src/rules/unused_standalone_imports_rule.ts

+  private areAllArrayElementsUnused(
+    arrayLiteral: ts.ArrayLiteralExpression,
+    usedDirectives: Set<ts.ClassDeclaration>,
+    usedPipes: Set<string>,
+  ): boolean {
+    if (arrayLiteral.elements.length === 0) {
+      return false;
+    }
+
+    for (const element of arrayLiteral.elements) {
+      if (!ts.isIdentifier(element)) {
+        return false;
+      }
+
+      const elementName = element.text;
+
+      for (const usedDir of usedDirectives) {
+        if (usedDir.name?.text === elementName) {
+          return false;
+        }
+      }
+
+      if (usedPipes.has(elementName)) {
+        return false;
+      }
    }

-    // Otherwise the reference likely comes from an imported
-    // symbol like an array of shared common components.
    return true;
  }


The current implementation uses a nested loop to check if an element is a used directive, which has a time complexity of O(elements * usedDirectives). This can be optimized by first creating a Set of used directive names and then checking for existence in the set, which would reduce the complexity to O(elements + usedDirectives).

private areAllArrayElementsUnused( arrayLiteral: ts.ArrayLiteralExpression, usedDirectives: Set<ts.ClassDeclaration>, usedPipes: Set<string>, ): boolean { if (arrayLiteral.elements.length === 0) { return false; } const usedDirectiveNames = new Set<string>(); for (const dir of usedDirectives) { if (dir.name) { usedDirectiveNames.add(dir.name.text); } } for (const element of arrayLiteral.elements) { if (!ts.isIdentifier(element)) { return false; } const elementName = element.text; if (usedDirectiveNames.has(elementName)) { return false; } if (usedPipes.has(elementName)) { return false; } } return true; }

…cal arrays

pullapprove bot requested a review from JoostK January 17, 2026 03:15

angular-robot bot added the area: compiler Issues related to `ngc`, Angular's template compiler label Jan 17, 2026

ngbot bot added this to the Backlog milestone Jan 17, 2026

JeanMeche added the action: discuss label Jan 18, 2026

JoostK mentioned this pull request Jan 19, 2026

Unused imports diagnostics for compound arrays without NgModules #59754

Open

SkyZeroZx force-pushed the compiler-cli/improve-unused-standalone branch from 38eb438 to 74ca851 Compare January 19, 2026 19:10

JeanMeche reviewed Jan 20, 2026

View reviewed changes

gemini-code-assist bot reviewed Feb 10, 2026

View reviewed changes

fixup! refactor(compiler-cli): detect unused standalone imports in lo…

13c7e6a

…cal arrays

Conversation

SkyZeroZx commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the new behavior?

Uh oh!

JeanMeche commented Jan 19, 2026

Uh oh!

SkyZeroZx commented Jan 19, 2026

Uh oh!

JeanMeche commented Jan 19, 2026

Uh oh!

SkyZeroZx commented Jan 19, 2026

Uh oh!

JeanMeche Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

SkyZeroZx Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

SkyZeroZx Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JeanMeche commented Feb 10, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

SkyZeroZx Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

SkyZeroZx Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SkyZeroZx commented Jan 17, 2026 •

edited

Loading

SkyZeroZx Jan 20, 2026 •

edited

Loading