fix(@ngtools/webpack): elide type only named imports when using emitDecoratorMetadata

alan-agius4 · dgp1130 · commit cf9afeec20c0 · 2022-08-05T11:02:07.000-07:00
With this change we fix an issue where type only named imports were being emitted. As a result webpack failed to resolve such symbols as they don't exist in JavaScript. Closes #23667
diff --git a/packages/ngtools/webpack/src/transformers/elide_imports.ts b/packages/ngtools/webpack/src/transformers/elide_imports.ts
@@ -54,39 +54,8 @@ export function elideImports(
       return;
     }
 
-    let symbol: ts.Symbol | undefined;
-    if (ts.isTypeReferenceNode(node)) {
-      if (!compilerOptions.emitDecoratorMetadata) {
-        // Skip and mark as unused if emitDecoratorMetadata is disabled.
-        return;
-      }
-
-      const parent = node.parent;
-      let isTypeReferenceForDecoratoredNode = false;
-
-      switch (parent.kind) {
-        case ts.SyntaxKind.GetAccessor:
-        case ts.SyntaxKind.PropertyDeclaration:
-        case ts.SyntaxKind.MethodDeclaration:
-          isTypeReferenceForDecoratoredNode = !!parent.decorators?.length;
-          break;
-        case ts.SyntaxKind.Parameter:
-          // - A constructor parameter can be decorated or the class itself is decorated.
-          // - The parent of the parameter is decorated example a method declaration or a set accessor.
-          // In all cases we need the type reference not to be elided.
-          isTypeReferenceForDecoratoredNode = !!(
-            parent.decorators?.length ||
-            (ts.isSetAccessor(parent.parent) && !!parent.parent.decorators?.length) ||
-            (ts.isConstructorDeclaration(parent.parent) &&
-              !!parent.parent.parent.decorators?.length)
-          );
-          break;
-      }
-
-      if (isTypeReferenceForDecoratoredNode) {
-        symbol = typeChecker.getSymbolAtLocation(node.typeName);
-      }
-    } else {
+    if (!ts.isTypeReferenceNode(node)) {
+      let symbol: ts.Symbol | undefined;
       switch (node.kind) {
         case ts.SyntaxKind.Identifier:
           const parent = node.parent;
@@ -106,10 +75,10 @@ export function elideImports(
           symbol = typeChecker.getShorthandAssignmentValueSymbol(node);
           break;
       }
-    }
 
-    if (symbol) {
-      usedSymbols.add(symbol);
+      if (symbol) {
+        usedSymbols.add(symbol);
+      }
     }
 
     ts.forEachChild(node, visit);
@@ -153,7 +122,7 @@ export function elideImports(
         clausesCount += namedBindings.elements.length;
 
         for (const specifier of namedBindings.elements) {
-          if (isUnused(specifier.name)) {
+          if (specifier.isTypeOnly || isUnused(specifier.name)) {
             removedClausesCount++;
             // in case we don't have any more namedImports we should remove the parent ie the {}
             const nodeToRemove =
@@ -168,7 +137,7 @@ export function elideImports(
       if (node.importClause.name) {
         clausesCount++;
 
-        if (isUnused(node.importClause.name)) {
+        if (node.importClause.isTypeOnly || isUnused(node.importClause.name)) {
           specifierNodeRemovals.push(node.importClause.name);
         }
       }
diff --git a/packages/ngtools/webpack/src/transformers/elide_imports_spec.ts b/packages/ngtools/webpack/src/transformers/elide_imports_spec.ts
@@ -440,6 +440,44 @@ describe('@ngtools/webpack transformers', () => {
         experimentalDecorators: true,
       };
 
+      it('should elide type only named imports', () => {
+        const input = tags.stripIndent`
+          import { Decorator } from './decorator';
+          import { type OnChanges, type SimpleChanges } from './type';
+
+          @Decorator()
+          export class Foo implements OnChanges {
+            ngOnChanges(changes: SimpleChanges) { }
+          }
+
+          ${dummyNode}
+        `;
+
+        const output = tags.stripIndent`
+          import { __decorate } from "tslib";
+          import { Decorator } from './decorator';
+
+          let Foo = class Foo { ngOnChanges(changes) { } };
+          Foo = __decorate([ Decorator() ], Foo);
+          export { Foo };
+        `;
+
+        const { program, compilerHost } = createTypescriptContext(
+          input,
+          additionalFiles,
+          true,
+          extraCompilerOptions,
+        );
+        const result = transformTypescript(
+          undefined,
+          [transformer(program)],
+          program,
+          compilerHost,
+        );
+
+        expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
+      });
+
       it('should not remove ctor parameter type reference', () => {
         const input = tags.stripIndent`
           import { Decorator } from './decorator';
