search

import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.57k stars 1.57k forks source link

[New] `no-unused-modules`: Add `ignoreUnusedTypeExports` option #3011

Closed silverwind closed 4 months ago

silverwind commented 6 months ago

Fixes: https://github.com/import-js/eslint-plugin-import/issues/2694

silverwind commented 6 months ago

Will tweak tests a bit.

silverwind commented 6 months ago

Ok, tests are fine now.

ljharb commented 6 months ago

This seems great, except that it's best when boolean arguments default to false. Can you come up with an alternative name for it so it can default to false?

silverwind commented 6 months ago

This seems great, except that it's best when boolean arguments default to false. Can you come up with an alternative name for it so it can default to false?

Hmm I could invert the name and function but thought that the current name matches nicely into the existing options missingExports and unusedExports.

silverwind commented 6 months ago

Inverted it, so now it's ignoreUnusedTypeExports defaulting to false.

ljharb commented 6 months ago

Your original option name definitely did match better, but i prioritize booleans defaulting to false over option names :-) thanks!

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.02%. Comparing base (fc361a9) to head (e96db1a).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3011 +/- ## ======================================= Coverage 96.02% 96.02% ======================================= Files 78 78 Lines 3296 3299 +3 Branches 1158 1160 +2 ======================================= + Hits 3165 3168 +3 Misses 131 131 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

silverwind commented 5 months ago

This CI failures seem intermittent and the coverage annotation "Added line #L518 was not covered by tests" is incorrect, I verified the line is being hit during tests.

ljharb commented 4 months ago

node 22.5 itself is broken rn, so i'll rerun those failing jobs once a fixed 22.5.1 is released.