eslint-plugin-import: Rule `import/no-unresolved` raise false positive when there is no `main` field in the `package.json`
What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.
import GBAEmulator, { GbaContext } from 'react-gbajs'
./node_modules/.bin/eslint .
What did you expect to happen?
Should raise no error. This package is present in the node_modules
.
An important note:
Since it’s only for the browser, it has the field browser
instead of main
in its package.json
(link). I made it following the NPM documentation.
Despite that, I noticed that by adding the field main
in its package.json
, the error suppress - but it doesn’t look correct.
What actually happened? Please copy-paste the actual, raw output from ESLint.
/Users/macabeus/ApenasMeu/klo-gba.js/brush/src/components/Emulator/index.js
2:41 error Unable to resolve path to module 'react-gbajs' import/no-unresolved
/Users/macabeus/ApenasMeu/klo-gba.js/brush/src/components/KloGbaSidebar/Hacks/index.js
7:28 error Unable to resolve path to module 'react-gbajs' import/no-unresolved
/Users/macabeus/ApenasMeu/klo-gba.js/brush/src/hooks/useGbaSaveRestoreState.js
2:28 error Unable to resolve path to module 'react-gbajs' import/no-unresolved
/Users/macabeus/ApenasMeu/klo-gba.js/brush/src/index.js
8:29 error Unable to resolve path to module 'react-gbajs' import/no-unresolved
/Users/macabeus/ApenasMeu/klo-gba.js/brush/src/providers/VisionProvider.js
4:28 error Unable to resolve path to module 'react-gbajs' import/no-unresolved
✖ 5 problems (5 errors, 0 warnings)
✨ Done in 4.73s.
Steps to reproduce this issue:
- Clone this repository: https://github.com/macabeus/klo-gba.js
- Run
yarn
on its root cd brush
yarn run lint
About this issue
- Original URL
- State: open
- Created 3 years ago
- Reactions: 17
- Comments: 40 (22 by maintainers)
Commits related to this issue
- fix: add main field for eslint https://github.com/benmosher/eslint-plugin-import/issues/2132 — committed to christophehurpeau/check-package-dependencies by christophehurpeau 3 years ago
- [Test] add tests for `"main": false` See https://github.com/import-js/eslint-plugin-import/issues/2132#issuecomment-1035886707 — committed to browserify/resolve by ljharb 2 years ago
- [Test] add tests for `"main": false` See https://github.com/import-js/eslint-plugin-import/issues/2132#issuecomment-1035886707 — committed to ljharb/resolve by ljharb 2 years ago
The pressure you’re discussing is not actually helping the ecosystem; i invite you to check the per-version download counts for every package that’s gone ESM-only. It’s a clear signal that developers do not want that.
The reason is because sindre is actually authoring their packages incorrectly - by omitting
main
entirely, there’s a default “main” ofindex.js
, which execa and sort-keys have. The proper way to have an ESM-only package would be"main": false
, which would then correctly create a warning for all of them.Regardless of this plugin’s inevitable eventual support for ESM resolution - which should of course work - I would strongly encourage you to migrate to package authors that maintain compatibility over ones that attempt to subjugate their users by forcing them to use a specific subset of node’s available module systems.
@ljharb You claim
false
to be valid formain
. Can you please cite your source?The official docs for the field don’t mention
false
:v13 seems to lack that page ¯\_(ツ)_/¯On the other hand, the
nodejs.org
pages list type as<string>
. Plus, https://nodejs.org/docs/latest-v16.x/api/modules.html shows this pseudo code:1.b
indicates thatfalse
would not that the package cannot berequire
d but thatindex.js
would be tried.All package authored by @sindresorhus will be migrated to pure esm module see article It’s already the case for
chalk
and many other packages.I have just upgraded a side-project to ESM – it consists of data-processing scripts and a Next.js app for data visualisations. Everything seems to be working with
"type": "module"
inpackage.json
, thanks to experimental ESM support ints-node
and to a small temp hack innext.config.mjs
.For me, this unlocked
execa@v6
,chalk@v5
andsort-keys@v5
. All of these project dependencies are authored by sindresorhus and are ESM-only.Although I have no runtime issues, importing
chalk
unexpectedly triggers ESLint:The other two very similar packages don’t produce any ESLint errors. Here are
package.json
of the versions I am using:Does anyone see anything that would break
import/no-unresolved
only forchalk
? 🤔In the meantime, I’ll stick with
"import/no-unresolved": "off"
and will expectyarn lint:tsc
to monitor imports.P.S.: Pure ESM packages are somewhat painful, but I appreciate the migration pressure created by maintainers. The faster we move the ecosystem to ESM-only, the better for everyone. This year I had to explain differences between ESM and CommonJS to new devs and also assist with quite intricate repo configs. I hope all this knowledge becomes unnecessary for most folks by the end of 2022!
@ghoullier yes, i’m aware - and that’s very user-hostile and will break a ton of things, including this plugin.
I suggest switching to packages that preserve compatibility.
The same error occurs for the p-queue package which
package.json
only shows anexports
field and nomain
field.I agree that this is rather odd considering what is currently advised on the NodeJS site.
Yes, I do that as well. I just add a dummy
main
pointing to a ESM file to make this plugin work. CJS import will be broken anyways for ESM packages, so it does not matter whethermain
points to a valid CJS file.Actually I point
main
to an ESM file in my projects. It still works well and will throw when the package is imported byrequire
. I do think package authors need to add this field back, but eslint-plugin-import should reconsider add support forexports
.Anyway, thank you for your hard work!
Not reliably - because if the main is in a subfolder, that might have a package.json itself, it gets tricky. That’s a big part of why it’s a best practice for packages using
exports
to always explicitly expose package.json."./package.json": "./package.json"
.and no, with a string
exports
nothing is exposed except the package name itself, that’s the whole point.I’m sorry. A personal attack was not intended.
@ljharb Sorry about the personal attack from them. I’ll ignore it for now and monitor the bug that you have open on it. Thanks for your hard work on this package!
@macabeus In the issue I have open with
chalk
this was the feedback from @sindresorhus:He seems to have a valid point regarding throwing this linting error on a package that is not backwards compatible with CommonJS. Thoughts? I’m just stuck in the middle.
@kachkaev I have this same issue. I opened an issue with chalk. Looks like it’s because
"main": "./source/index.js",
is missing frompackage.json
. Chalk with ESLint & Airbnb Base #536@rasenplanscher
mkdir -p node_modules/main-false && echo '{"main": false}' > node_modules/main-false/package.json && touch node_modules/main-false/index.js && node -pe "require.resolve('main-false')"
does show that you’re right. I’ll add a test case toresolve
for this.That still makes “omit main entirely” an incorrect approach, unless there’s no
index.js
present - which the packages mentioned above have.In that case, my testing shows that setting main to
{}
will, at least, error in every node version down to 0.10 (0.6 and 0.8 still default toindex.js
in that case), and “exports” takes precedence over this incorrect “main” in node versions that support it. Probably a better alternative, then, is to have an explicit main that points to a file that exists, but throws a runtime error immediately upon being required.I mean, that package is super broken because it has a “main” that doesn’t exist - it should specify
"main": false
if it’s not meant to be importable. You may want to file a bug on it.That said, we should be using
package.json
, not the main, to verifyimport type
packages, so i think that’s the fix here.There are the same lint errors even when using
false
.