Skip to content
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

Bug: no-extraneous-import doesn't support import maps #147

Closed
1 task
privatenumber opened this issue Dec 11, 2023 · 4 comments · Fixed by #139
Closed
1 task

Bug: no-extraneous-import doesn't support import maps #147

privatenumber opened this issue Dec 11, 2023 · 4 comments · Fixed by #139

Comments

@privatenumber
Copy link

Environment

Node version: 21
npm version:
ESLint version:
eslint-plugin-n version: v16.4.0
Operating System: macOS

What rule do you want to report?

no-extraneous-import

Link to Minimal Reproducible Example

https://eslint-online-playground.netlify.app/#eNp1kDFvwyAQhf+KRVfH3j1371J1KRkwnCJaw9EDqlaR/3sOiCPZUjbu3uPTu3cVkfQ4D19RTEL0dVJtsi4gpU6Kl1kWaYC4WJ9Is4yeDVfpO5bhL4E3UYqp++SpmiYCjc7xHowUPbvCki/WT37cKee+MYKiCPQWkkVfSZVd6dqpD6DIQtlLsagEMTF0c0TMpOH9P0AzODR54XfR1zufeLPn/mRkzhaaCKnlNJjn8vtcv0u/8uVB6W91gcPdrZ89tVRVMgzjVuQ+h4HfVwjleK/tIVFr7smRTTy1Fk+HKh5R1xvHDJTm

What did you expect to happen?

For it to resolve the package.json import map

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

It only seems to check the dependency objects:

Object.keys(packageInfo.dependencies || {}),
Object.keys(packageInfo.devDependencies || {}),
Object.keys(packageInfo.peerDependencies || {}),
Object.keys(packageInfo.optionalDependencies || {})
)

scagood added a commit to scagood/eslint-plugin-n that referenced this issue Dec 11, 2023
@scagood
Copy link

scagood commented Dec 11, 2023

Thank you for this report! I think #139 also fixes this (completely by accident) 👀

I have added a test and it seems to be covered!

@privatenumber
Copy link
Author

Oh nice, thank you!

Out of curiosity: if pulling in enhanced-resolve to eslint-plugin-n, wouldn't there be a significant amount of overlap with https://github.com/import-js/eslint-plugin-import + https://github.com/markjm/eslint-import-resolver-enhanced-resolve ?

In what cases would someone prefer to use the import resolvers in this plugin over eslint-plugin-import?

@scagood
Copy link

scagood commented Dec 11, 2023

mmm, that is a sound point!

I wonder if we should support the import resolver API. 🤔
I do like the extensibility of that too

@aladdin-add
Copy link

so can the plugins use the shared settings? sounds interesting! 👍

scagood added a commit to scagood/eslint-plugin-n that referenced this issue Jan 2, 2024
scagood added a commit to scagood/eslint-plugin-n that referenced this issue Jan 9, 2024
aladdin-add added a commit that referenced this issue Jan 9, 2024
* feat: Use enhanced-resolve for imports

* chore: Improve the metadata from "ImportTarget"

* chore: remove "enhanced-resolve" from "no-hide-core-modules"

* test: Add a test for #66

* feat!: Allow ts paths aliases (#84)

* feat: Allow for "allowImportingTsExtensions" (#134)

* feat: Add test for import maps (#147)

* Update lib/util/import-target.js

Co-authored-by: Sebastian Good <2230835+scagood@users.noreply.github.com>

* test: Add test for n/no-missing-require eslint/use-at-your-own-risk

* chore: Remove esbuild

* feat: Allow for settings.cwd to be used before process.cwd

* chore: replace reference to eslint/use-at-your-own-risk

* chore: Remove more unused packages

* chore: update rule test options to flat config

* fix: incorrect env in tests

---------

Co-authored-by: 唯然 <weiran.zsd@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants