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: Load CommonJS .eslintrc.js files within a "type": "module" package scope (fixes #12319) #12333
Conversation
8b5b86c
to
29e263c
Compare
Thanks for the PR. As mentioned here, we ask that changes to core go through the RFC process outlined here](https://github.com/eslint/rfcs). |
I agree that this is not an enhancement. But we need tests to confirm what PR fixed and because the |
|
I'm seeing that the error has been moved behind an experimental flag in nodejs/node#29732. After re-look, I think that we can replace |
Yes, for now. I’m on the Node team working on this, and we haven’t decided what to do about this when Ultimately, the error was added for a reason: if a user adds
Sure! I don’t have the time to do that kind of testing; thanks for investigating. |
@GeoffreyBooth Thank you for working on that! I found some questions while I play with ES modules, may I ask you? My goal is to find the way that imports modules with As the document,
From the above, I guess
But I couldn't find a way to use query strings with a package name. await import("x-esm?q=2") //→ Cannot find package 'x-esm?q=2'
await import("x-esm/?q=3") //→ Cannot find module C:\Users\t-nagashima.AD\dev\eslint\node_modules\x-esm\
await import(require.resolve("x-esm") + "?q=4") //→ Cannot find module /\Users\t-nagashima.AD\dev\eslint\node_modules\x-esm\index.js How should I use query strings with a package name? (or is there another way to import packages without cache?) I found an odd behavior about // test.js (cjs)
;(async () => {
// console.log("ESM")
await import("x-esm") //→ Found (this package just contains `console.log("***")`)
await import("./node_modules/x-esm/index.js?q=0") //→ Found and `x-esm` re-ran as expected
await import("./node_modules/x-esm/index.js?q=1") //→ Found and `x-esm` re-ran as expected
console.log("CJS")
await import("x-cjs") //→ Found (this package just contains `console.log("***")`)
await import("./node_modules/x-cjs/index.js?q=0") //→ Found but `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=1") //→ Found but `x-cjs` didn't re-run
console.log("remove 'require.cache'.")
delete require.cache[require.resolve("x-cjs")]
await import("x-cjs") //→ Found but `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=0") //→ Found but `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=1") //→ Found but `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=2") //→ Found and `x-cjs` re-ran as expected
await import("./node_modules/x-cjs/index.js?q=3") //→ Found but `x-cjs` didn't re-run
console.log("remove 'require.cache'.")
delete require.cache[require.resolve("x-cjs")]
await import("x-cjs") //→ Found but `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=0") //→ Found but `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=1") //→ Found but `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=2") //→ Found and `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=3") //→ Found but `x-cjs` didn't re-run
await import("./node_modules/x-cjs/index.js?q=4") //→ Found and `x-cjs` re-ran as expected
})().catch(error => {
console.error(error)
process.exitCode = 1
}) If the import source is ESM, it works fine. But if the import source is CJS, it's odd.
Actually, shouldn't This is off-topic, but I have a concern. await import("x-esm/") //→ Cannot find module C:\Users\t-nagashima.AD\dev\eslint\node_modules\x-esm\ Since built-in punycode module has been deprecated, we use third-party punycode package. But Cannot we use this workflow in ES modules? Is there the formal way to import third-party packages which have the same name as a built-in module? |
Hi @mysticatea, I’ll try to answer your questions, but I’m certainly not an expert. I’ve asked other members of the modules team to chime in if they can.
I think you’d have to read the package’s
Yes. For CommonJS, the specifier (the string passed to
Probably? Though maybe not because
This is an edge case that I don’t think anyone’s considered, and your use of a trailing slash there seems like an unsafe workaround. It might be a good idea to publish a wrapper package like |
Thank you for your answer!
I got it. It's great if ESM version of
I have opened an issue: nodejs/node#29812
Indeed, it's an edge case. But also shims for browsers often have the same name as built-in modules, so I guess it's not a rare case. I hope the standard way to load such a package without the knowledge of the internal file structure of the packages. |
https://nodejs.org/dist/latest-v13.x/docs/api/esm.html Maybe using Another example: babel/babel#10599 |
How do we want to proceed with this? |
I mildly oppose this change. Because this will make hard to support ESM config files in the future. If we leave the |
I agree with @mysticatea. I don't think this is something we should do right now, since we're actively working towards supporting ESM in core. |
I'm going to go ahead and close this PR since the team has decided not to go forward with this proposal. We appreciate your contribution! |
Fixes #12319, where CommonJS
.eslintrc.js
files inside a project where the user has added"type": "module"
to theirpackage.json
were throwing an error.Sorry for the lack of tests, but please feel free to complete this PR if you like this approach to solving the problem. It may become moot if the error is removed in Node (see nodejs/modules#389 and nodejs/node#29732).