fix(select-style): remove unused imports after fixing violations #218
Conversation
src/rules/store/select-style.ts
Outdated
| fixer.remove(parent), | ||
| ...(nextToken ? [fixer.remove(nextToken)] : []), | ||
| fixer.insertTextAfter(storeNode, `.${text}`), |
There was a problem hiding this comment.
I've improved the fixers so it's not dependant on a well-formatted code, instead of decreasing -1 or adding +1 to find the expected range (which may cause a lot of breakage - because it all depends on how one format their code), it looks for the closest node through the souceCode.getTokenBefore/After.
rafaelss95
force-pushed
the
fix/maybe-finally-remove-unused-imports
branch
5 times, most recently
from
437abd0
to
f8869a7
Compare
There was a problem hiding this comment.
I like this a lot, smart!
I think it's much better than what we currently have and what was proposed before.
The only downside is that if someone fixes the import, they will get errors because select is still being used, correct?
I'm fine with that though, just wanted to make sure.
EDIT: Or should the import fix and the usage fix be separated in order to fix this? E.g. that the import fix is only proposed when there are no declared variables 🤔
|
Hmm.. I got your concern, but IMHO, fixes, unlike suggestions are usually applied automatically, by saving a file if you've configured your IDE for that, or running via the CLI (manual or pre-commit hook or something like that)... so I assume that it's only a problem if you try to fix every occurrence by hand 🤔. I don't know... should we bother with this? 😅 |
|
We shouldn't be bothered with this, at least not for now. |
rafaelss95
force-pushed
the
fix/maybe-finally-remove-unused-imports
branch
from
f8869a7
to
778f4b8
Compare
|
🎉 This PR is included in version 1.42.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Fyi, I just tested this in a real application and it works as a charm! Thanks @rafaelss95 🏆 |
|
How dare you not believe only in tests? Hahaha, just kidding. Glad to see that it works in a real app 😄 |
@timdeschryver I took the liberty of submitting this PR as a draft in an attempt to finally clean up any traces of unused imports.
In a short, it basically lookup for the node import
import { ..., select } from '@ngrx/platform'and then it grab the references that points to this node. Note that the same approach could be used forno-effect-creator-and-decorator,no-effect-decoratorandprefer-concat-latest-from. Let me know what you think.