-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[Fix] no-unused-modules
: make type imports mark a module as used
#1974
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you rebase, many of the styling changes will already be taken care of. I suggest running npx eslint . --fix
after the rebase as well.
@@ -49,3 +49,6 @@ lib/ | |||
yarn.lock | |||
package-lock.json | |||
npm-shrinkwrap.json | |||
|
|||
# macOS | |||
.DS_Store |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be in your global gitignore, rather than adding it to every project you touch (but it’s fine to add)
Assuming automatic releases is not a safe assumption; I find that incredibly dangerous and don't do it in any package I maintain. There is no concrete timeline for releases; I tend to find time every 3-6 months in this project. |
Can I request that you release soon? It's misleading to mark #1924 (and any other issues with unreleased fixes) as resolved considering it'll continue to be broken for dependents for up to 6 months. |
@saiichihashimoto it is a decade-old standard practice that issues are closed when fixes are merged, entirely unrelated to the time of release. If that doesn't match your expectations, I'm afraid the easiest path is to adjust your expectations. I am indeed planning a release soon, but it's blocked right now due to npm 7's release breaking tests on master. |
That makes sense, no sass intended. |
What's the progress on this? It's been a while... |
It's still blocked on #1986. |
Now that #1986 is resolved, is there a timeline of when we can expect a release? |
There is no such timeline. As soon as I move through all the PRs that have been waiting, I plan to cut a release. |
Is there a way you could cut a release and then go through those PRs? My understanding is that the changes that are waiting have already been tested so the wait for them to go out is arbitrary. |
Every release that goes out increases maintenance burden for the project, and anything that's not in master yet, but would be in the next release if I wait, might add features or fix bugs, thus reducing that maintenance burden. |
@saiichihashimoto If you really want to use eslint-plugin-import with this bug fix without waiting for a release, you could use GitHub dependencies in {
"devDependencies": {
"eslint-plugin-import": "benmosher/eslint-plugin-import#462b016"
}
} 462b016 is the commit that merged this PR, but you could also use a newer commit if you want. |
@saiichihashimoto v2.23.0 has been released, which includes this bug fix. |
Thanks for bearing with my persistence! |
Fixes #1924
ExportMap
now keeps track of each imported declaration (imports.get(path).declarations
) and whether the declarations are only importing types (isOnlyImportingTypes
). This is used byno-cycle
toFor example (from this existing test):
depth-zero.js
flow-types-depth-one.js
flow-types-depth-two.js
depth-one.js
The only significant change in
no-unused-modules
is here where each declaration'simportedSpecifiers
is added to a file's import list. However, because theimports
in a file'sExportMap
now include type imports, type imports now mark an export as being used.