Repository navigation
Detect require vs. import based on the entry point, not package.json #39353
Description
Activity
Related discussion: https://github.com/nodejs/node/discussions/37857, nodejs/modules#300 (comment), nodejs/modules#296.
- node scans that files for the import keyword
I don't thing that's a reasonable solution: it would introduce a performance penalty on startup time if node has to parse the file for
import(dynamicimport()is a completely valid thing in CJS, are you suggesting you should remove that? And we would need to know if theimportkeyword appear inside a string or a comment, so it would be pretty expensive to determine that), and also not all ES module containsimportso that would introduce some weird edge cases (AFAIK, there is no known algorithm to tell for sure if a file is written as a module or a script).There are no alternatives:
You could use loaders to customize which format / parse goal for a given file: https://nodejs.org/docs/latest/api/esm.html#esm_loaders. It's not stable yet though.
Node should never need to consult it just to run normal, valid, modern or legacy, JS
Legacy code is exactly why changing the default behaviour of Node.js is a very difficult thing to do, it would be a problem if older code base stopped working. Note that Node.js looks for
package.jsononly for.jsfile, if you use.mjsor.cjsfiles Node.js won't be looking for anypackage.json.dynamic import() is a completely valid thing in CJS, are you suggesting you should remove that
I'm suggesting changing the default behaviour, so if Node should allow mixed import/require when there's a package.json, then perfect: that is not at issue, I'm all for that. But Node should also do the right thing without a package.json.
it would introduce a performance penalty on startup time if node has to parse the file
Correct. And that would be very little time because it only has to scan one file, plus (and this is the big one): if you use a package.json, Node doesn't need to scan anything, it already knows which mode to run in, so it would do exactly what it's already doing right now, and no one following current necessary practices will be impacted. Only folks who invoke Node without a package.json would run into this performance hit, although I struggle to call it that: it'd be insignificant compared to the rest of Node's initialisation.
You could use loaders to customize which format / parse goal for a given file
While I see a lot of potential for loaders, they are antithetical to why I filed this issue: Node should do the "obvious" right thing out of the box (i.e. if asked to run a file, it should run that file, either in cjs mode, or esm mode, automatically. Not first be configured to do the right thing through runtime flags, or config files, or config code).
Note that Node.js looks for package.json only for .js file, if you use .mjs or .cjs files Node.js won't be looking for any package.json
Sure, but again: that's not doing the right thing, that's forcing people to use nonstandard extensions as workaround. Node should (and this is a long term "should", not "it must, now!") do the right thing when given a
.jsfile, based on the content of that.jsfile (and also, only that.jsfile. There is no need to resolve the full tree, Node can do the "obvious" thing based on the fact that people who use mixed import/require will almost certainly be using a package.json already to make sure things work properly).Legacy code is exactly why changing the default behaviour of Node.js is a very difficult thing to do
No it isn't? That's what major versions are for. I have no expectation of this getting changed in the current version, but I do kind of expect this to be seriously considered for Node 17 or 18. The way Node works as of v16, especially as LTS, should stay exactly what it is now, let's definitely not change that. People rely on it to work a specific way. But 17 or 18 are fair game.
Hum so you'd like to see this behavior only for the entry point 🤔 In this case, I have more questions, how would you define parse goal for dependency of such modules:
// entry-point.js import './submodule.js'; // is this ESM no matter its content? import './submodule.cjs'; // is this CJS? import 'some_package'; // can this be CJS?
// other-entry-point.js require('./entry-point.js'); // is this CJS?
// another-entry-point.js // no import, no export, no require, is this ESM or CJS? console.log(this); // is this undefined? var obj2 = { get x() { return 17; } }; obj2.x = 5; // should this throw?
I'm suggesting changing the default behaviour, so if Node should allow mixed import/require when there's a package.json, then perfect: that is not at issue, I'm all for that. But Node should also do the right thing without a package.json.
Why would the "right" thing to remove dynamic imports for CJS? Are they causing an issue, or is this just for convenience to tell if a file is CJS or MJS?
While I see a lot of potential for loaders, they are antithetical to why I filed this issue: Node should do the "obvious" right thing out of the box (i.e. if asked to run a file, it should run that file, either in cjs mode, or esm mode, automatically. Not first be configured to do the right thing through runtime flags, or config files, or config code).
Note that what is "obvious" to you may not be for everyone. While I understand loaders are not the long term solution you'd like, it can still be useful for you (or someone else) to build a POC.
this is not a novel topic of discussion and I think the general sentiment is that this would be cool if someone builds something, but no one has built something because it's a very complex problem, and you have to make a lot of subjective calls which people disagree on.
Reacted by Luigi PincaGood questions. In the absence of a package.json, keep it simple, and keep it naive:
// entry-point.js import './submodule.js'; // is this ESM no matter its content? import './submodule.cjs'; // is this CJS? import 'some_package'; // can this be CJS?would result in going "that's a normal import on line 1, stop checking and use ESM parsing" and then when it runs the file it tries to import submodule.cjs using ESM rules and it'll throw an error, and that's fine. Use a package.json if you need to mix modalities.
// other-entry-point.js require('./entry-point.js'); // is this CJS?Yes, it is, because the first import mechanism encountered uses
require, notimport.// another-entry-point.js // no import, no export, no require, is this ESM or CJS? console.log(this); // is this undefined?This is Node's default mode. If that's still CJS by v17 or v18, then it's CJS, unless Node's finally ready to switch to ESM parsing by default, then it's ESM.
Why would the "right" thing to remove dynamic imports for CJS? Are they causing an issue, or is this just for convenience to tell if a file is CJS or MJS?
Who said anything about removing dynamic imports? If the file starts as a normal, modern, plain, JS file (that is it starts with a bunch of
imports of other files that end in .js) then it picks ESM parsing, and rolls with that, and if there are dynamic imports, no problem. Those work fine. If it starts with a bunch ofrequires then pick CJS parsing, and roll with that instead, and then if there are dynamic imports that are valid under the CJS model, then also no problem.This feature request is entirely about what Node does when asked to run "plain files", in the absence of a
package.json. That is, we're not dealing with a project, we're not dealing with external dependencies, someone just wrote a bunch of plain JS files using Node's API and they want to run them, they just happen to be more than a single file because the person doing the writing was taught proper code house keeping and sticks to that. Node should be able to run their code without anything other than "being run": in this scenario, Node can pick the correct parsing mode 100% of the time in these cases, because the examples you're showing just don't factor into that use-case.This really is a matter of "if the entry file starts as a normal JS file with
import, use ESM, if it starts withrequire, use CJS, and we're done". Any "oh hey you actually did something unusual" code like in those examples should, quite reasonably, halt with an error telling folks that they're mix-and-matching, and to use a package.json (or a runtime flag that explicitly tells Node which parser to use, which would most certainly be worth adding to 16, as that would be purely new functionality and not break any preexisting code).Note that what is "obvious" to you may not be for everyone.
Hence the quotes, I'm talking about the kind of obvious that exists when looking in from the outside: "I have some JS files, I've written them following current standards, running
node firstfile.jsshould works". The kind of expectation that someone who is new to Node would have.Thanks for the clarification, I think this makes a lot of sense; first we'd need to find or write an algorithm that can tell if a JS file is ESM or CJS, is this something you'd be interested in contributing?
Related (vaguely): TC39 proposal
"use module";.in the absence of a
package.json[...], we're not dealing with a project, we're not dealing with external dependenciesIf you allow me to be pedantic, you can have external deps without a
package.json– Node.js doesn't usepackage.jsonto resolve local deps and there is an open PR to add support for HTTPS imports :)If you allow me to be pedantic, you can have external deps without a package.json
Ah, yes that's true, good point.
Something that goes in this direction is being done in #39175
Nice!
we'd need to find or write an algorithm that can tell if a JS file is ESM or CJS, is this something you'd be interested in contributing?
While I wouldn't mind giving it a shot, given the number of projects I'm already involved with any promise to try to get to that soon would pretty much be a lie, I'm basically booked up for projects for at least a year, filing issues and discussing whether there's merit to it is the most I can do for larger projects for the foreseeable future =(
Reacted by Antoine du Hamel- added a commit that references this issue
on Jul 24, 2021 These arguments seem to apply on the idea that "this makes it easier for beginners". That's an argument I don't buy into. Changing the behaviour as suggested in this issue (making Node use ESM parsing mode if the entry file and only the entry file is ESM) makes Node better as a tool. Tools should have sensible default behaviours, and a way to override any and all of that behaviour by specifying an explicit configuration (either throught runtime flags, or a config file, which for Node is the
package.jsonfile).So if we're talking about sensible default behaviour: if someone writes plain, modern JS, which because of the JS spec can only be ES module based code (because that's what TC39 decided is the only import mechanism that spec-compliant JS can use) then Node should be able to execute that file, because Node's job is to run JS, which means at the very least it should run spec-compliant JS, even if it can also run its own flavour of JS. And if it's asked to run legacy CJS code, it should run that without any complains too, because it's been doing that for about a decade now; that should keep working. And if someone writes code and specifies a the configuration file that Node looks at, then obviously now Node should do whatever it does based on what's in that configuration file. This is basic tool design: if there's a config, that config kicks in.
And yes: that means that ESM code that works "on its own" suddenly won't run anymore when you use
npm initto create apackage.jsonfile, because npm right now does not include thetypefield. That's npm's failing (and if there isn't an issue for this on their side already, I will be quite surprised), not Node's. In fact, that's exactly what you want: there is no problem here, Node will tell you that it can't run ESM unless you add"type": "module"to your package file, you follow Node's instruction and updatepackage.json, and immediately your code works again."What if your ESM code has imports that somewhere down the line switchi to CJS style requires?"
Literally everyone doing that today already has apackage.jsonfile, because their code won't run if they don't. This is a non-issue, their code keeps working exactly as it already works right now."What if someone mixes ESM and CJS in their own (flat file) code?"
Then someone should tell them to fix that, because that's just nonsense. And that someone is the Node cli when it's asked to execute a script. It'll throw you an error right now if you try this, and it should keep throwing you that error."What if their ESM imports a Node API, which uses CJS?"
This would be the only valid argument against, if it weren't for the fact that Node can already expose its own API in ESM context just fine, making this a non-issue, too.CC: @nodejs/modules
I think we should close this as a duplicate of many older issues and discussions unless something novel comes up.
Per:
Something that goes in this direction is being done in #39175
This is only a heuristic to help debugging, it isn't 100% reliable or performant nor is it expected to become so to my knowledge.
Reacted by Jordan HarbandThe hope was that this explanation was the "novel" thing people keep mentioning. Every other issue seems to want way more than what Node, the command line util, should be doing, making this proposal far more reasonable in comparison.
26 remaining items
@GeoffreyBooth that sounds amazing, and it would be super useful if its docs specified which values it can take, rather than leaving users guessing (it currently says
--input-type=... set module type for string input).The PR was never merged in. I’m just referencing it since it seems to do what you’re asking for. It would need to be updated to reflect changes to the ESM implementation since it was written, most prominently that a new flag would need to be chosen. Also presumably the version of Acorn in Node core is newer now, so the problem with
import()would be avoided.With ESM gaining traction in and beyond node.js worlds, I am now also in the category of users who find both the available options to enable ESM less-than-ideal:
.mjsextension orpackage.json. They just come across as superfluous and unnatural requirements.Two questions:
- Is breaking change an option for the next (or future) major versions of node?
- For sake of argument, if breaking change was an option, what would be the ideal no-compromise performance-wise breaking change here, that will lead user to the pit of success -- without requiring .mjs extension or existence of package.json?
-
In the absence of .mjs and package.json explicitly specifying how to behave ... Treat `require()` as CJS and `import` as ESM. If the user is assuming something different, let things continue (and fail) implicitly. Improve debugging/analysis tools to spot potential code issues during the development time. - something else?
-
Is breaking change an option for the next (or future) major versions of node?
It is unlikely that a change based upon syntax guessing will make it in; but breaking changes do happen for more minor things that are more forwards compatibility safe after the breakage, yes.
For sake of argument, if breaking change was an option, what would be the ideal no-compromise performance-wise breaking change here, that will lead user to the pit of success -- without requiring .mjs extension or existence of package.json?
Node could always just ship a 2nd binary that swaps the default. A
node_esmexecutable or some similar name for example. Such an alternative binary wouldn't break code when calling out to child_process/the other concerns presumably since code would be written against it and the concerns about altering behavior accidentally wouldn't come into play since the behavior is reliably the same even if CJS, ESM, or a new parse goal collide further than they do today.I don't understand why
.mjsfeels "unnatural" - does using.jsfeel unnatural? You already can't just use any arbitrary file extension you want without extra configuration - does anything on the server (not just in the JS world) work that way for code?It is unlikely that a change based upon syntax guessing will make it in;
Yes, my thinking is also aligned that such validation can/should be done by the code analysis tool rather than the node.js runtime. In this situation, I'd rather have the runtime continue and fail in implicit ways (whatever may happen, happens), rather than guessing anything just to be able to produce a high-level speculative-at-best kind of error message.
I don't understand why
.mjsfeels "unnatural"If this ES feature "just works" with .js extension like many other ES features do, then there is no confusion and we don't have to explain anything. When it doesn't work OOTB, that kicks things into the situation where we are right now.
does anything on the server (not just in the JS world) work that way for code?
No, but you don't ask users to change file extensions to use another feature of same language, just because it conflicts with an existing feature; a command-line option (which we are missing here) and configuration option (which we have in package.json) comes to mind; sometimes accompanied by additional environment variable support. This approach of "for this particular feature only, change extension of your files" is a bad design and should not be promoted for anything, imo.
@kasperk81 that's not really possible though, because the JS spec created two ambiguous parse goals. Anything that appears to "just work" will also silently fail for nonzero programs.
JS kind of contains two languages now - ie, two parse goals. Modules and Scripts aren't the same and it makes perfect sense to me that they require different extensions (or explicit configuration).
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.
For more information on how the project manages feature requests, please consult the feature request management document.
FWIW Node.js 21.1.0 ships a
--experimental-detect-moduleCLI flag that will load an ambigeous.jsfile as ESM if it can't be parsed as CJS.that's great to hear, thank you letting us know!
Is your feature request related to a problem? Please describe.
Node does not require a
package.jsonfile to run, unless you're writing modernimport/exportcode, in which case you suddenly need to define a package.json even if you have no intention of creating a project out of the code you just wrote. You can't even usenode --input-type=modulebecause it will --for no reason that makes sense for users-- complain that youCannot use import statement outside a module, the thing we're literally saying that's what we're doing by using that flag.Describe the solution you'd like
Node should not need folks to tell it which parsing mode to use: it should scan the entry point it's being asked to run, and simply check whether or not the reserved JS keyword
importis found in that file. If it is, done: run in ES module mode without needing folks to create a file that Node should not rely on to do its job.require(...)during the run is now a perfectly normal "this function is not defined in this scope" error.importduring the run is now a perfectly normal "import is a reserved keyword" error.import(...)during the run is now a perfectly normal "this function is not defined in this scope" error.And now Node does the correct thing, given perfectly normal code as run target. Will that very first step add to the startup time before code actually runs? Sure, but scanning for the
importkeyword takes on the order of nanoseconds, not milliseconds. We're not scanning the entire dependency tree to see if somewhere down the line we suddenly switch from import to require or vice versa: by default, without a package.json, Node doesn't need to mix and match: as default behaviour Node should run both legacy CJS and modern JS (either/or, not a mix, obviously) without runtime flags or needing files that have nothing to do with Node itself created.Describe alternatives you've considered
There are no alternatives: Node should not rely on package.json just to run normal modern code, it should do the right thing automatically, with runtime flags and package.json only for folks who need it to do something else (and it should probably never need package.json, that file is so that NPM can do proper package management. Node should never need to consult it just to run normal, valid, modern or legacy, JS)