| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@nodejs/modules |
Sorry, something went wrong.
|
Should this perhaps be Semver-Major? |
Sorry, something went wrong.
|
If we did want to ship this it might make sense to try to ship it soon so we don't break unmaintained ESM packages. What's the usual policy on a log-only deprecation? |
Sorry, something went wrong.
|
Runtime deprecations are semver-major according to https://github.com/nodejs/node/blob/master/doc/guides/collaborator-guide.md#deprecations |
Sorry, something went wrong.
|
Thanks @aduh95 I've put this deprecation beind --pending-deprecation. |
Sorry, something went wrong.
|
What's the benefit of doing this? What about "type": "commonjs" packages? |
Sorry, something went wrong.
|
@ljharb it means that a package like: {
"main": "dir"
}where the main is eg dir/index.js, would only work when not using "type": "module" - so we restrict the main index lookup to CommonJS modules. |
Sorry, something went wrong.
|
You should probably document the deprecation in packages.md as well. |
Sorry, something went wrong.
|
@guybedford right, i understand the consequences. why, though? type: module has no necessary correlation to "uses ESM", nor is that specific type something we're necessarily recommending (it's just an available option). |
Sorry, something went wrong.
Because "exports" is better than "main"? Or put another way, "exports" is aware of the multiple module systems and can support distinguishing between them, whereas "main" can’t; and while that’s not strictly necessary for an ESM-only package, many packages will be dual-system for the next few years and therefore we want to push them to use "exports". It would be too disruptive to deprecate "main" entirely, so we’re taking the approach of deprecating it only for "type": "module" packages. Assuming that’s the reasoning (?) I guess I can get behind that, but it’s a big change that should probably have more than a few eyes on it. It might be something we should write a blog post about, encouraging the adoption of "exports". |
Sorry, something went wrong.
|
@GeoffreyBooth ok, but why not do so for any package that declares an explicit type, including "type": "commonjs"? Why is type module special? |
Sorry, something went wrong.
Sure, we could do that for any "type". It might be confusing though that this deprecation only appears when "type": "commonjs" is explicit rather than implied (by virtue of being the default). Also arguably we might not want to deprecate "main" for CommonJS packages, as "exports" isn’t exactly fully backward-compatible since you can’t opt out of the encapsulation (I think?). |
Sorry, something went wrong.
|
I'm -0 on this since it makes "type" have a linting like effect rather than just being configuration of the file types. There are minutiae we could argue about this, but if we ever do overload "type" to have objects on the RHS for things like custom format mappings this gets complicated. Also, while I do agree in theory "exports" is better than "main" I don't really see the need to simplify here. We would be breaking users so I would at least like to get some active returns over linting. If there was a semantic reason to cause an error that seems like a good reason to do this, but I don't see any clear why except that we can here. Things like static resolution are still possible even with the current behavior (albeit complex). I would be happy if we at least took a strong stance that we are trying to avoid the searching behavior there in this PR. I would also prefer we take a stance that if we do allow configuration of "type" to expand such as "wasm", file extension mappings, etc. on what we want to do with this behavior. If we have the presence of "type" always ban this type of index resolution (including in CJS packages) at least we can make a firm stance across the board here. |
Sorry, something went wrong.
|
IMO, type set to anything other than "commonjs" should deprecate the CommonJS‑style resolution of the main field, e.g. the following is valid: {
"type": "module",
"main": "./lib/index.js"
}But the following for the same directory structure is deprecated: {
"type": "module",
"main": "./lib/index"
}{
"type": "module",
"main": "./lib"
}In other words, I prefer alternative option 2. |
Sorry, something went wrong.
|
@GeoffreyBooth a package is not a "commonJS package" or an "ESM package" except by virtue of what files are contained within it. A package with type "module" is no more an ESM package than one with no type at all, assuming both provide (or do not provide) ESM modules, and there should be no such implication by any use of the "type" field. |
Sorry, something went wrong.
I don’t see us expanding "type", at least not within the next semver major release of Node (or the next two releases even). When "type": "wasm" was discussed, it was for the purposes of defining how Node should treat extensionless files; but since then we’ve pretty much settled on always treating extensionless files as CommonJS, even inside a "type": "module" scope. (Hence the complaints in #32316.) As long as that doesn’t change, and I vaguely remember that there are strong reasons why it can’t (hence why we had to revert "type": "wasm") then extensions will always be required in ESM and therefore we don’t need configuration to tell Node how to handle a Wasm file (or any other non-JavaScript file). As for an extensions map, the functionality that that would unlock is possible with loaders today and will become easier as that API continues to evolve (#36396). Loaders wouldn’t help published packages, but I’m not sure that mapping extensions within dependencies is a use case we need to support. |
Sorry, something went wrong.
|
This PR as written effectively deprecates the "main" field for Node.js. There are variations that could restrict what valid "main" fields are permitted, eg not defaulting to index.js and enforcing that main points to a valid path without extension searching. "type": "module" works as the deprecation path here because .js files in main would always be CommonJS otherwise (and extension searching does not include .mjs). So while we don't capture the "main": "./x.mjs" case as being a deprecation in this PR, by effectively deprecating for "type": "module" it's the first runtime deprecation step to a full deprecation for "main" when using ES modules in Node.js. @bmeck in terms of more blanket proposals, we could eg outright deprecate the main in packages and for example warn when executing a local package without an exports field. The difficulty with a blanket warning for non-exports is that's even more agressive than what this PR is doing. The less agressive options are likely to refine the semantics of main. Alternatively we can just do nothing here and let index.js remain as an ease of use vestige going forward. Note that index.mjs is not supported though so that there is a natural "type": "module" asymmetry already. |
Sorry, something went wrong.
|
I guess the big question here is do we want to "force" encapsulation of packages, or allow using the "main" as a way to continue to optionally choose to use encapsulation or not. I'm tending towards thinking making encapsulation optional might be better after all? |
Sorry, something went wrong.
|
That matches my understanding of the original intention - opt-in encapsulation, not forced. |
Sorry, something went wrong.
|
To try and flesh out some of the options we have here then:
Any opinions? At least picking a coherent story going forward seems like it might make sense at this point. |
Sorry, something went wrong.
I would prefer this at the least since if we add extension support over time the resolved file may change. Deprecation would allow some future proofing and get back some space avoiding compatibility concerns. The implicit index.* seems also like it would need to be deprecated if this occurs though. |
Sorry, something went wrong.
|
Here's a alternative which is options 1 and 3 combined:
|
Sorry, something went wrong.
Would this mean self imports of the current package wouldn't use it / would error? |
Sorry, something went wrong.
@bmeck I think that's already the case (self-reference only sees "exports"). By runtime deprecate only for local development, I mean using require on directories that do not have package.json#exports would emit a warning (unless this is done by a module inside a node_modules folder) encouraging to use self-reference instead. |
Sorry, something went wrong.
ES module main entry resolution now requires an explicit "exports" or "main" field with the exact file extension. The legacy index.js and extension-searching lookups that previously succeeded with a DEP0151 warning now throw ERR_INVALID_PACKAGE_CONFIG. This is a semver-major change. It is triggered by `import 'pkg'` when the resolved package entry is an ES module and either has no "main"/"exports" field or has a "main" value that omits the file extension. CommonJS packages are unaffected. Refs: nodejs#37206 Refs: nodejs#36918 Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
| Back | FazBrowse Home | New Git URL |
This adds a new deprecation warning when importing a main that resolves to an ES module that relies on the "index" or extension searching "main" resolution semantics.
In the next major this can become a runtime error, while for now it is behind --pending-deprecation only.
This is an update to the previous iteration which allows "main" and "exports" to continue to coexist without encapsulation being enforced.
The two warnings shown are for extensions not present:
and for no main field present:
The warning is shown for third party packages and local packages equally.
This approach was taken after discussion that pushing encapsulation or deprecating the main would be too strong of a move to make.