Skip to content

Commit

Permalink
fix(@ngtools/webpack): elide type only named imports when using `emit…
Browse files Browse the repository at this point in the history
…DecoratorMetadata`

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

(cherry picked from commit cf9afee)
  • Loading branch information
alan-agius4 authored and dgp1130 committed Aug 5, 2022
1 parent f41d0a6 commit dd47a5e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 38 deletions.
45 changes: 7 additions & 38 deletions packages/ngtools/webpack/src/transformers/elide_imports.ts
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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 =
Expand All @@ -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);
}
}
Expand Down
38 changes: 38 additions & 0 deletions packages/ngtools/webpack/src/transformers/elide_imports_spec.ts
Expand Up @@ -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';
Expand Down

0 comments on commit dd47a5e

Please sign in to comment.