Skip to content
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

File altered by before transform does not remove new unused imports #17552

Open
filipesilva opened this issue Aug 1, 2017 · 2 comments
Open
Labels
API Relates to the public API for TypeScript Bug A bug in TypeScript Domain: Transforms Relates to the public transform API
Milestone

Comments

@filipesilva
Copy link
Contributor

Description of the bug:

When using a before transform that makes an import unused, the import will not be removed by TypeScript.

So if I have:

import { unused } from 'unused-module';
import { something } from 'some-module';
const value = something;

And replace const value = something with

import { somethingElse } from 'some-other-module';
const value = somethingElse;

The import { unused } from 'unused-module'; statement will be removed, but import { something } from 'some-module'; will remain:

import { something } from 'some-module';
import { somethingElse } from 'some-other-module';
const value = somethingElse;

TypeScript Version: 2.4.2

Code

import { unused } from 'unused-module';
import { something } from 'some-module';
const value = something;

Reproduction of this bug requires using the transforms API.

I have prepared a repository with a simple reproduction that runs this file: https://github.com/filipesilva/ts-transform-import-bug/blob/master/test.ts

git clone https://github.com/filipesilva/ts-transform-import-bug
cd ts-transform-decorator-bug
npm install
npm test

Expected behavior:
All unused imports are removed.

import { somethingElse } from 'some-other-module';
const value = somethingElse;

Actual behavior:
Only imports there were unused before the transformed are removed.

import { something } from 'some-module';
import { somethingElse } from 'some-other-module';
const value = somethingElse;

/cc @rbuckton

@DanielRosenwasser DanielRosenwasser added API Relates to the public API for TypeScript Bug A bug in TypeScript Domain: Transforms Relates to the public transform API labels Aug 1, 2017
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 2.5 milestone Aug 1, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.5.1, TypeScript 2.5 Aug 16, 2017
@rbuckton
Copy link
Member

Currently, whether or not an import is referenced is determined by the checker at check time, prior to emit. We do not re-bind or re-check the AST between transformations, so all check-time state will always be based on the originally parsed AST.

The only way to elide an import in this case would be to have the "before" transformer explicitly elide the import. This is currently the intended behavior. We are considering ways of opening up the checker (and our internal EmitResolver) to custom transformations to allow more control, but that likely won't be ready in the 2.5 release timeframe.

@arciisine
Copy link

I'm actually facing the inverse of this problem. Something is being marked as unused (and so the import is elided) but I would like that import to stay. Is there anyway to manage this without checking the entire file and adding the import back in (if it truly is unused)

@mhegazy mhegazy modified the milestones: TypeScript 2.7, TypeScript 2.8 Jan 9, 2018
@mhegazy mhegazy modified the milestones: TypeScript 2.8, TypeScript 2.9 Mar 9, 2018
@mhegazy mhegazy modified the milestones: TypeScript 3.0, Future Jul 2, 2018
kyliau pushed a commit to angular/angular-cli that referenced this issue Feb 19, 2020
Running the `remove-ivy-jit-support-calls` and `remove_decorators` transformers causes the following TS bug microsoft/TypeScript#17552 which is why the `elide-imports` transformer exists in the first place.

However, when having a syntax like the below;
```ts
import { AccountComponentChild } from '../@types';
export class SignUpComponent implements AccountComponentChild{}
```

The `implements` parts of the class is called a `HeritageClause` with child statements of `ExpressionWithTypeArguments` also the same is for `abstract`. With this change we check the token of the `HeritageClause` and if it's an `ImplementsKeyword` we elide the import.

Closes #16907
kyliau pushed a commit to angular/angular-cli that referenced this issue Feb 19, 2020
Running the `remove-ivy-jit-support-calls` and `remove_decorators` transformers causes the following TS bug microsoft/TypeScript#17552 which is why the `elide-imports` transformer exists in the first place.

However, when having a syntax like the below;
```ts
import { AccountComponentChild } from '../@types';
export class SignUpComponent implements AccountComponentChild{}
```

The `implements` parts of the class is called a `HeritageClause` with child statements of `ExpressionWithTypeArguments` also the same is for `abstract`. With this change we check the token of the `HeritageClause` and if it's an `ImplementsKeyword` we elide the import.

Closes #16907
ikjelle pushed a commit to ikjelle/angular-cli that referenced this issue Mar 26, 2024
Running the `remove-ivy-jit-support-calls` and `remove_decorators` transformers causes the following TS bug microsoft/TypeScript#17552 which is why the `elide-imports` transformer exists in the first place.

However, when having a syntax like the below;
```ts
import { AccountComponentChild } from '../@types';
export class SignUpComponent implements AccountComponentChild{}
```

The `implements` parts of the class is called a `HeritageClause` with child statements of `ExpressionWithTypeArguments` also the same is for `abstract`. With this change we check the token of the `HeritageClause` and if it's an `ImplementsKeyword` we elide the import.

Closes angular#16907
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Relates to the public API for TypeScript Bug A bug in TypeScript Domain: Transforms Relates to the public transform API
Projects
None yet
Development

No branches or pull requests

5 participants