eslint-plugin-import icon indicating copy to clipboard operation
eslint-plugin-import copied to clipboard

[New] `no-extraneous-dependencies`: Add `considerInParents` option

Open luxaritas opened this issue 2 years ago • 10 comments

Resolves https://github.com/import-js/eslint-plugin-import/issues/1913

I am not particularly happy with the implementation (in particular, the error passing and "requireExact" handling to match the current behavior feel misplaced and/or mishandled), but it's at the point that I feel it would be useful to get feedback (both on how the options are handled and the implementation).

luxaritas avatar Jun 22 '22 02:06 luxaritas

Codecov Report

Base: 95.27% // Head: 95.23% // Decreases project coverage by -0.04% :warning:

Coverage data is based on head (a686917) compared to base (c3d14cb). Patch coverage: 98.07% of modified lines in pull request are covered.

:exclamation: Current head a686917 differs from pull request most recent head 61eddb2. Consider uploading reports for the commit 61eddb2 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2481      +/-   ##
==========================================
- Coverage   95.27%   95.23%   -0.05%     
==========================================
  Files          68       66       -2     
  Lines        2944     2793     -151     
  Branches      998      940      -58     
==========================================
- Hits         2805     2660     -145     
+ Misses        139      133       -6     
Impacted Files Coverage Δ
utils/readPkgUp.js 96.42% <95.23%> (+14.61%) :arrow_up:
src/rules/no-extraneous-dependencies.js 100.00% <100.00%> (+1.78%) :arrow_up:
utils/parse.js 98.14% <0.00%> (-0.13%) :arrow_down:
src/rules/dynamic-import-chunkname.js 96.77% <0.00%> (-0.11%) :arrow_down:
src/rules/order.js 99.10% <0.00%> (-0.09%) :arrow_down:
src/rules/no-cycle.js 97.91% <0.00%> (-0.05%) :arrow_down:
src/rules/export.js 100.00% <0.00%> (ø)
src/rules/no-restricted-paths.js 100.00% <0.00%> (ø)
src/rules/no-anonymous-default-export.js 100.00% <0.00%> (ø)
src/rules/consistent-type-specifier-style.js
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Jun 22 '22 04:06 codecov[bot]

FYI, I have seen these comments (thanks! good points). Hopefully I'll have time within the next week or two to address them.

luxaritas avatar Sep 07 '22 01:09 luxaritas

Sorry for the delay! Rewrote this with your feedback and it is significantly simpler. :) As a bonus, this should also improve caching, since it's no longer necessary to touch the file system to look for the closest package.json. Let me know if there's anything further I can improve!

luxaritas avatar Oct 20 '22 21:10 luxaritas

Resolved all the feedback - let me know if you want me to squash.

luxaritas avatar Oct 21 '22 00:10 luxaritas

What is the status of this PR?

WangHoHan avatar Nov 22 '22 09:11 WangHoHan

Waiting for rereview as far as I'm concerned

luxaritas avatar Nov 22 '22 13:11 luxaritas

Interesting idea to use set. I'd be interested to see that benchmarked given the small sizes and additional spread when creating the cache key, though I wouldn't immediately know how to do that myself. :) I can certainly add a few more tests.

luxaritas avatar Jan 11 '23 21:01 luxaritas

yeah lol for such a small array it probably doesn't matter, but might as well use the proper data structure :-p

ljharb avatar Jan 11 '23 23:01 ljharb

@ljharb Can you help me understand why two of the tests I added are failing? It looks like this rule is trying to resolve any found import, even if it isn't present, and returns successfully if it isn't found? That.. can't be right, can it?

luxaritas avatar Jan 12 '23 03:01 luxaritas

Hmm, no, I don't know why it's failing :-/ i just rebased it so results are fresh.

ljharb avatar Apr 13 '23 22:04 ljharb