modules
modules copied to clipboard
nodejs
Transition Path Problems For Tooling
Just tracking tooling encountering issues with what we have been rolling out, please add more as they are seen for gathering overall impact:
| issue | resolved | summary |
|---|---|---|
| https://github.com/eslint/eslint/issues/12319 | ❌ | Always used require() and .js now errors for type: "module" |
.js-based script config files is a really important class of bug - it could be worth having a dedicated issue for this one.
we could also just get rid of type: module, assuming we want people to depend less on package.json magic, not more
BTW, this works flawlessly in 12.0. The newer (ie better) package boundary detection is throwing the error.
Semantics feel off. The consumer (ie CJS) is consuming a module (ie ESM) that exists outside of it's package boundary. Shouldn't the package type constraint be based on the consumer not the consumed?
Anyway, w/ a 1-line patch to eslint giving the config a .cjs extension works flawlessly.
we could also just get rid of type: module, assuming we want people to depend less on package.json magic, not more
This breaks browser interop for pure ESM packages.
Using .mjs for ESM breaks the most tools build to write ES6+. Outside the Node ecosystem .js and ESM are one-and-the-same.
That’s not entirely accurate; for example, in the babel ecosystem, everything is either a script or a module based on config, not file extension. Since nothing but node yet parses JS based on file extension, what .js means everywhere else is “i have no idea which parse goal to use yet”.
I definitely didn't intend to start any arguments about extensions, I was just hoping to get in some ideas about various directions to go with ecosystem feedback. best to think of breaking changes now before we unflag.
This isn’t really a breaking change for the tool, because the user has opted into the new mode by setting type: module. The tool just doesn’t support that mode yet. Until the tool is updated, the user can simply take out type: module (or stop using that tool).
And while the user waits for the tool to be updated, they can keep using CommonJS or ESM as they do now, via esm or Babel or whatever. I’m not convinced that https://github.com/nodejs/node/pull/29732 is really necessary.
There are going to be lots of tools that won’t fully support ESM mode in Node for quite awhile, beyond just require of .js inside type: module. Until we get loaders completed, code instrumentation or mocks won’t work, for example. There’s going to be a lengthy period where users will have to choose between ESM (whether via type: module or ESM more broadly) and various tools that they may want to use.
I’m not convinced that nodejs/node#29732 is really necessary.
I agree - the resolver under a type: module modifier is explicitly experimental right now, so this becoming an error isn't a "new break", it's just how it should be in the experimental feature - and any pain points using type: module with tools brings up are likely other shortcomings in our current interop story.
Is nodejs/node#29732 getting fast tracked because of a resolution at the meeting that I missed? Or do some of the core devs just feel that strongly about it?
@weswigham all discussions and decisions are as made publicly there, there have been no side agreements. If you disagree with fast tracking or merging it behind the flag we can certainly reconsider, my primary goal was just to avoid shipping frictions to the ecosystem before we are 100% certain about it, to avoid unnecessary concern / issues for users. I see it as more of a courtesy than anything else.
(this issue was not discussed at the last meeting, because we were not yet aware of the compatibility case)
I just wanted to know if it'd been discussed in the WG at all (and wasn't sure since I'd missed Wednesday's meeting), since it's a slight walk-back of what'd already been merged (and i thought part of the reason for merging it was to see what kinds of issues cropped up).
I definitely asked if it should have been behind a flag when it was first added, and I still do agree enough with it having been unflagged, provided one believes that using type: module in your package.json right now implicitly opts you in to experimental behavior (regardless of node flags). If that's not perceived to be the case, I imagine we're going to have some degree of trouble shipping type: module at all, I'd wager.
@weswigham as I say it is as a courtesy - we want to encourage experimentation of type module, and we don't want to cause unnecessary frictions in doing that, unless we are 100% it is those frictions we want to ship. So it's not so much about whether it is breaking as whether this is the story we want users to be dealing with. Personally I think it is still debatable whether this should be an error or a warning or similar.
I was under the impression that this was expected - when you have type: module set, every .js file in scope is a module, and that includes config files. I, at least, was under no other illusions. Tools don't currently load module-based config files, so, y'know, those tools aren't going to work yet. There are no APIs for them to work yet, really. There are certainly some things we could do to make the story better (like require of esm just working, rather than erroring), or failing that TLA and dynamic import alongside package.json probing should allow those tools to support module-based configuration (with async APIs T.T).
The major constraint for tools is that they often have synchronous config loading internals (I know at least Babel does /cc @loganfsmyth), so that using import() and falling back to require is not such an easy option or non-breaking change for the tool itself.
But yes if we can accept these breaks for tools and we are all 100% sure this is what we want to dictate that tools should have to deal with then that is fine too.
As I say though I don't see why we couldn't still support these cases under a deprecation path as the major part of the hazard that concerned me previously was that the CJS loader was injecting these into the ESM loader such that depending on if you loaded CJS or ESM first you would get CJS or ESM interpretations respectively through the ESM loader itself. If we can at least avoid that hazard, what hazard is left?
If we can at least avoid that hazard, what hazard is left?
The hazard is that require("path/to/esm-module") shouldn't ever be parsed as cjs. And require("module-with-type-module/eslint.config.js") is, by all rights, a module.
The usability issue is the lack of an API (eg, require itself, for simplicity), to synchronously load a module.
I really hope I can hammer that home to enough people that that's, like, one of the biggest non-bikeshed concerns of node's (current) esm implementation.
And
require("module-with-type-module/eslint.config.js")is, by all rights, a module.
what I'm asking is where is the actual hazard in this case? How does it bite?
Literally, this - the same file could be interpreted twice, in two different ways (one of which may or may not error due to syntax), which is exactly what we're trying to avoid, as if the file does anything stateful (eg, set a global, muck with process), that state change will occur twice.
So to try flesh out the example - say you have a "type": "module" package on npm (P) and install it such that it has two dependents - one CommonJS and one ES module package that both import from it. The CommonJS package loads a file with no module syntax from P that does a global mutation, and the ES module package loads that same file from P resulting in the global mutation happening twice?
Note this "hazard" is naturally guarded by the following:
- Any use of require or ES module exports will interrupt this hazard from occurring as one of the dependents will get an error
- The mutations can only be global mutations since side effects that rely on imports would also throw
- The bugs are only caused if there are side-effect calls eg to
processorconsoleor mutations of globals. But without these there would be no conflict too.
My feeling is the best solution is still just to allow synchronous require of ESM that strictly does not include a top-level await. The TLA proposal specifically chose to make sure that module graphs that only contain synchronous modules evaluate synchronously.
This means it would be possible to have something like sourceTextModule.evaluateSync() that only works if all modules to be evaluated don't contain [[Async]]: true and hence a sync-require could work fine for modules that don't depend on TLA (or WASM modules) avoiding significant breakage.
I would not like to bifurcate TLA like that, if we support it we support it.
I would not like to bifurcate TLA like that, if we support it we support it.
The web is already intending to reject TLA in service workers, given that require of ESM would primarily be provided as an escape hatch for existing code depending on require I don't think it's unreasonable to say we provide require of certain ES graphs (e.g. those without TLA) so that existing tools can be easily updated.
I disagree with the web disabling TLA in service workers. if TLA randomly doesn't work in certain contexts why would anyone feel motivated to write code with TLA in it.
I disagree with the web disabling TLA in service workers. if TLA randomly doesn't work in certain contexts why would anyone feel motivated to write code with TLA in it.
This is super snarky, but bear with me:
I disagree with node disabling esm in require. if eem randomly doesn't work in certain contexts why would anyone feel motivated to write code with esm in it.
Because the convenience of use outweighs the inconvenience of incompatibility, ofc. That's the hope, at least. We've been making similar choices the whole time. I'm not saying I wouldn't prefer uniform TLA support (and Jordan's right, having both TLA and non-TLA modules is pretty much a tooling nightmare - different supported syntax subsets in the same runtime is not going to be fun for explaining to people in an error), but I do get the trade-off. Sort-of. It sounds like the argument is just "TLA in service workers is a footgun since they're often performance sensitive and it's easy to deadlock or wait on unneeded async resources using TLA" - which is all true in other contexts, too. It's again just a matter of weighing that possibly against the convenience of its use, and deciding differently in the SW context. If anything, that w3c feels free to make ecosystem bifrucating choices like this, should be freeing to us, as it certainly means runtimes should be considered as free to determine their own destiny when it comes to module support and use, since there are even compat concerns within the browser.
I disagree with node disabling esm in require.
require could never load esm. nothing was disabled. nothing was lost. esm is still working. I think i've said a lot in the past i am a fan of require(esm), and i still am, but randomly disabling language features is in the "unacceptable" category for me. Why not just disable the instantiation checks that verify that imports are wired correctly and map cjs modules at runtime? Why not just make our own module system that mirrors babel esm instead of native esm? If we randomly change the language for our own purposes we end up bifurcating the ecosystem. I am hopeful TC39 will choose to disallow this when its brought up in a few days.
require could never load esm. nothing was disabled. nothing was lost
And service workers have never been able to be modules yet, so nothing was lost with no TLA in them.
randomly disabling language features is in the "unacceptable" category for me
I think the line between a language feature and a runtime feature is slim - especially for a module system feature like TLA. In any case, the distinction is lost on most less in-the-weeds users and I do think you're right; inconsistent is inconsistent, but I do think I can also call a spade a spade and say we've made similar choices thus far, to weight technical concerns and imagined future problems over real ecosystem and compatibility issues, and I understand theirs in that context, thus I don't feel quite right criticizing it.
I am hopeful TC39 will choose to disallow this when its brought up in a few days.
I don't think they can. It's not possible. An implementor could just say "we support es2020 modules with TLA and other features in these contexts, and es2016 modules that don't have TLA in these others". If you simply rephrase the bifrucating as essentially two runtimes based on differing spec versions (and SW really are essentially their own runtime), the language spec doesn't get a word in edgewise about it. I don't think tc39 will be able to do a lick about it, other than some committee members saying "I don't like it". The committee could always say "but if you don't support it in all contexts then you're not a real es2020 runtime" (philosophically), but safari is the only es2016, runtime by that metric, as they're the only runtime that implemented tail calls, but you don't typically see people up in arms about that, do you?
And service workers have never been able to be modules yet, so nothing was lost with no TLA in them.
replacing words in a sentence doesn't inherently preserve the logical meaning of the sentence. your argument leans on the fallacy that esm doesn't work without require being able to load it, which is quite obviously false.
An implementor could just say "we support es2020 modules with TLA and other features in these contexts, and es2016 modules that don't have TLA in these others".
implementations do this with TCO, and therefore the ecosystem as a whole doesn't use TCO. It would really be a shame for the same to happen to TLA. Alternatively, people just bundle all their TLA, bypassing the restriction but keeping the behaviour, and there was no point to disallowing it in the first place.
If node follows web workers, we just make the problem worse because of how large a platform node is. Given that the ecosystem already gets along without TLA, why would anyone choose to use it if part of the web and node don't support it? Or, if they do use it, they can just bundle it and bypass the restriction, at the cost of a worse API surface. There is no winning if we disable it.
This also has the classic issue we have worked hard to avoid where the importer shouldn't impose constraints on the imported.