Skip to content

Commit 56a70bc

Browse files
filipesilvamgechev
authored andcommitted
fix(@ngtools/webpack): also elide type references on transform (angular#16085)
Fix angular#14617
1 parent 722c3cb commit 56a70bc

File tree

3 files changed

+60
-5
lines changed

3 files changed

+60
-5
lines changed

packages/ngtools/webpack/src/transformers/elide_imports.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ export function elideImports(
3333
const imports: ts.ImportDeclaration[] = [];
3434

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

packages/ngtools/webpack/src/transformers/elide_imports_spec.ts

+16-3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,19 @@ describe('@ngtools/webpack transformers', () => {
3333
`,
3434
};
3535

36+
it('should remove unused imports', () => {
37+
const input = tags.stripIndent`
38+
import { promise } from './const';
39+
import { take } from './const';
40+
const unused = promise;
41+
`;
42+
43+
const { program, compilerHost } = createTypescriptContext(input, additionalFiles);
44+
const result = transformTypescript(undefined, [transformer(program)], program, compilerHost);
45+
46+
expect(tags.oneLine`${result}`).toEqual('');
47+
});
48+
3649
it('should remove unused aliased imports', () => {
3750
const input = tags.stripIndent`
3851
import { promise as fromPromise } from './const';
@@ -188,7 +201,7 @@ describe('@ngtools/webpack transformers', () => {
188201
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
189202
});
190203

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

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

227-
it('should only drop default imports when having named and default', () => {
240+
it('should only drop default imports when having named and default (3)', () => {
228241
const input = tags.stripIndent`
229242
import promise, { fromPromise } from './const';
230243
const used = promise;

packages/ngtools/webpack/src/transformers/remove_decorators_spec.ts

+42
Original file line numberDiff line numberDiff line change
@@ -248,5 +248,47 @@ describe('@ngtools/webpack transformers', () => {
248248

249249
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
250250
});
251+
252+
it('should not prevent removal of unused imports', () => {
253+
// Note: this is actually testing the behaviour of `./elide_imports.ts`, but the problem
254+
// only came up when used together with `./remove_decorators.ts` so we test it here.
255+
// Originally reported as https://github.com/angular/angular-cli/issues/14617
256+
// The bug happened because `elideImports` would attempt to remove `BigDependency` and
257+
// in the process modify the import statement, which then prevented TS from removing it
258+
// the interface too.
259+
const input = tags.stripIndent`
260+
import {BigDependency, BigDependencyOptions} from 'src/app/lorem/big-dependency';
261+
import {Injectable} from '@angular/core';
262+
263+
const bigDependencyLoader = () => import('../lorem/big-dependency');
264+
265+
@Injectable({providedIn: 'root'})
266+
export class LoremService {
267+
load(options: BigDependencyOptions): Promise<any> {
268+
return bigDependencyLoader()
269+
.then((m: {BigDependency}) => new m.BigDependency().setOptions(options));
270+
}
271+
}
272+
273+
`;
274+
const output = tags.stripIndent`
275+
const bigDependencyLoader = () => import('../lorem/big-dependency');
276+
export class LoremService {
277+
load(options) {
278+
return bigDependencyLoader()
279+
.then((m) => new m.BigDependency().setOptions(options));
280+
}
281+
}
282+
`;
283+
284+
const { program, compilerHost } = createTypescriptContext(input);
285+
const transformer = removeDecorators(
286+
() => true,
287+
() => program.getTypeChecker(),
288+
);
289+
const result = transformTypescript(undefined, [transformer], program, compilerHost);
290+
291+
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
292+
});
251293
});
252294
});

0 commit comments

Comments
 (0)