Skip to content

fix(@ngtools/webpack): also elide type references on transform #16085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/ngtools/webpack/src/transformers/elide_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ export function elideImports(
const imports: ts.ImportDeclaration[] = [];

ts.forEachChild(sourceFile, function visit(node) {
// Skip removed nodes
if (removedNodes.includes(node)) {
// Skip type references and removed nodes. We consider both unused.
if (node.kind == ts.SyntaxKind.TypeReference || removedNodes.includes(node)) {
return;
}

Expand Down
19 changes: 16 additions & 3 deletions packages/ngtools/webpack/src/transformers/elide_imports_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,19 @@ describe('@ngtools/webpack transformers', () => {
`,
};

it('should remove unused imports', () => {
const input = tags.stripIndent`
import { promise } from './const';
import { take } from './const';
const unused = promise;
`;

const { program, compilerHost } = createTypescriptContext(input, additionalFiles);
const result = transformTypescript(undefined, [transformer(program)], program, compilerHost);

expect(tags.oneLine`${result}`).toEqual('');
});

it('should remove unused aliased imports', () => {
const input = tags.stripIndent`
import { promise as fromPromise } from './const';
Expand Down Expand Up @@ -188,7 +201,7 @@ describe('@ngtools/webpack transformers', () => {
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should only drop unused default imports when named and default', () => {
it('should only drop unused default imports when named and default (1)', () => {
const input = tags.stripIndent`
import promise, { promise as fromPromise } from './const';
const used = fromPromise;
Expand All @@ -206,7 +219,7 @@ describe('@ngtools/webpack transformers', () => {
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should only drop unused named imports when named and default', () => {
it('should only drop unused named imports when named and default (2)', () => {
const input = tags.stripIndent`
import promise, { promise as fromPromise, take } from './const';
const used = fromPromise;
Expand All @@ -224,7 +237,7 @@ describe('@ngtools/webpack transformers', () => {
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should only drop default imports when having named and default', () => {
it('should only drop default imports when having named and default (3)', () => {
const input = tags.stripIndent`
import promise, { fromPromise } from './const';
const used = promise;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,5 +248,47 @@ describe('@ngtools/webpack transformers', () => {

expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should not prevent removal of unused imports', () => {
// Note: this is actually testing the behaviour of `./elide_imports.ts`, but the problem
// only came up when used together with `./remove_decorators.ts` so we test it here.
// Originally reported as https://github.com/angular/angular-cli/issues/14617
// The bug happened because `elideImports` would attempt to remove `BigDependency` and
// in the process modify the import statement, which then prevented TS from removing it
// the interface too.
const input = tags.stripIndent`
import {BigDependency, BigDependencyOptions} from 'src/app/lorem/big-dependency';
import {Injectable} from '@angular/core';

const bigDependencyLoader = () => import('../lorem/big-dependency');

@Injectable({providedIn: 'root'})
export class LoremService {
load(options: BigDependencyOptions): Promise<any> {
return bigDependencyLoader()
.then((m: {BigDependency}) => new m.BigDependency().setOptions(options));
}
}

`;
const output = tags.stripIndent`
const bigDependencyLoader = () => import('../lorem/big-dependency');
export class LoremService {
load(options) {
return bigDependencyLoader()
.then((m) => new m.BigDependency().setOptions(options));
}
}
`;

const { program, compilerHost } = createTypescriptContext(input);
const transformer = removeDecorators(
() => true,
() => program.getTypeChecker(),
);
const result = transformTypescript(undefined, [transformer], program, compilerHost);

expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});
});
});