dprint-plugin-typescript icon indicating copy to clipboard operation
dprint-plugin-typescript copied to clipboard

Remove unused imports

Open declanvong opened this issue 3 years ago • 3 comments

As per title!

dprint currently will organize imports, but not remove unused ones. This is in comparison to the TS service "Organize imports", which does both. The TS service has a very different goal to dprint however.

Would removing unused imports be out of scope for a formatting tool? Is removing whole identifiers too much of a transformation?

It would be possible for us to use an external tool to do this instead, e.g. eslint, but consolidating it into a single tool (with configuration of course) would be nice.

Related, but outside of this particular issue, it would be good to get guidelines written up around what can and can't / shouldn't be done within dprint and instead should be handled by other tools. E.g. if we allow removing unused imports, do we then allow removing unused variables and other identifiers? Imports just happen to be a simple case since it's possible to work out in an isolated file context.

declanvong avatar Aug 18 '21 02:08 declanvong

I personally don't think that formatter should be removing unused imports. This requires understanding why import is there and how it is used. Also not all unused imports can be removed, sometimes they inject global legacy JS code and while it is not used in TypeScript code directly, lack of import might break an app. This will lead to a situation when specific rules should be ignoring specific files (you still want to format such rare files, but you only want to skip one specific rule for them).

Auto organising imports already causes issues with zone.js in Angular tests and it is impossible to turn this specific feature off for a specific file while keeping other rules working.

Auxx avatar Sep 14 '21 16:09 Auxx

you still want to format such rare files, but you only want to skip one specific rule for them it is impossible to turn this specific feature off for a specific file while keeping other rules working.

This sounds like it's a desirable feature, but due to edge cases it would lead to problems.

Would that mean that you'd be satisfied by a solution that adds this as an option to the configuration, if there was a way to disable specific rules for specific lines/files? Something like an extension of the current // dprint-ignore comment, e.g. // dprint-ignore importDeclaration.removeUnusedImports or // dprint-ignore-file importDeclaration.removeUnusedImports

It's a good point though, I'm now curious as to whether TS's Organize Imports will correctly detect imports that have side effects but are otherwise unused.

declanvong avatar Sep 15 '21 01:09 declanvong

I would personally not like this option because while developing I often save the file and with format on save it would be annoying if it keeps removing code while I'm developing. I initially thought it might as well add this and it could be a configuration option that's added by default, but this kind of functionality is already available in tools like deno lint so I think this makes sense as more of a linting rule rather than a formatting one.

Auto organising imports already causes issues with zone.js in Angular tests and it is impossible to turn this specific feature off for a specific file while keeping other rules working.

On a per file basis, the auto-organizing imports feature can be controlled by adding blank lines or comments between imports to control the order.

dsherret avatar Oct 03 '21 17:10 dsherret