Repository navigation
doc: ES module wrapper guide #34714
Description
📗 API Reference Docs Problem
- Version: 14.7.0
- Subsystem: ESM
Location
Affected URL(s):
Description
I was wondering if it would be better to write this example as:
{
"type": "commonjs",
"main": "./index.js",
"exports": {
"import": "./wrapper.mjs",
"require": "./index.js"
}
}since its current form might be misleading for package maintainers that want to support both ESM and CommonJS contexts. Maybe what happened here could be prevented if the documentation was more clear.
- I would like to work on this issue and
submit a pull request.
Activity
aduh95 commentedon Aug 10, 2020
What do you mean by that? The current example does support both contexts. I does even support old version of Node.js.
Why do you think your example would be clearer? If anything, using explicit extension (
.cjs,.mjs) is arguably clearer than using.jsextension IMHO.fox1t commentedon Aug 10, 2020
I agree with you that
.cjsand.mjsare better. However, this issue is about the fact that setting"type": "module"makes.jsfiles become ESM modules. This could be an issue if CommonJS package maintainer just copy-pastes that property and leaves.jsas its CommonJS files extension.aduh95 commentedon Aug 11, 2020
@fox1t that's a valid point! Maybe we could change only the
"type": "module"line and keep the.cjsextension? Anyway, PR are welcome.{ - "type": "module", + "type": "commonjs", "main": "./index.cjs", "exports": { "import": "./wrapper.mjs", "require": "./index.cjs" } }MylesBorins commentedon Aug 12, 2020
FWIW the specific layout here was intentional. If you are using exports node will node utilize package.main. Older versions of node.js that don't support Exports also don't support ESM, so main can be used as a fallback for legacy versions of node. Perhaps it would be better if we names it
./index-legacy.cjs?In regards to the is-promise article, some of the documentation that currently exists was written specifically in response.
fox1t commentedon Aug 12, 2020
@MylesBorins I am not sure I fully understand what you said here. I think we can think only about the newer Node.js versions, that support
exportsprop. I think that older Node.js versions are not affected by any mean by"type": "module"setting: is this true?What I've understood about native ESM in Node.js, is that
"type": "module"makes the.jsfiles behave as ESM. If so I am sure that many of the confusion about this docs derives from the fact that the users (and worst of all, package maintainers) copy that example, replaces.cjswith.jsand leaves type asmodule. From what I understand, this is harmful.That's why I think that writing
in the doc section where we talk about porting legacy packages to be consumed by both world, is way better. I think that 99% of the packages and package maintainers are using
.jsfor their CommonJS files at this point in time and they will transform them to ESM without even noticing it.@aduh95 I agree with you that
.cjsextension would be fine, but it implies that maintainers read thru all of the documentation abut ESM: I am almost sure that the majority will not do that since they are only trying to figure out how to support ESM context. On the other hand, setting"type": "commonjs"in that snippet will at least prevent them from doing really bad things without noticing it.Of course, I might be wrong about this topic too and in that case, I am open to understanding how it works and, if needed, to update the docs accordingly!
As final words, @MylesBorins that addition about
exports:is perfect and makes clear what is the "public API" of the package.MylesBorins commentedon Aug 12, 2020
@fox1t There is no version of Node.js that will utilize main if exports exists that also supports modules, thus having a .js file there is essentially setting the module up to fail in all those environments as the .js file will be ESM.
fox1t commentedon Aug 13, 2020
@MylesBorins I know and completely agree with you about the fact that there will never be overlaps on main and exports props.
Let's see if I manage to make more clear what I am saying.
I'm going to make an example.
I am a maintainer of a package published to npm that many depend on. I don't have a well made CI/CD pipeline that runs all of my tests in every current LTS version of Node.js.
One day I just wake up and I decide to support native ESM. I go to the docs, copy-paste
to my package.json and since my current files have
.jsextension I just change"require": "./index.cjs"to"require": "./index.js".Since I copied
"type": "module"and I am not aware of what it does, I've just broken the package for all node versions that supports theexportsfield. (and here I am supposing that all of my public API is inindex.jsfile just for semplicity).I hope that this clarifies why having
"type": "commonjs",would be better in a guide that is used by users that already has a codebase with.jsfiles.P.S. I see this mistake done many times into the wild, by several developers with different skill levels.
guybedford commentedon Aug 13, 2020
We could remove the
"type"field entirely from the example certainly.MylesBorins commentedon Aug 13, 2020
@fox1t apologies for missing the point you were trying to get across. +1 on removing module from the example.
fox1t commentedon Aug 14, 2020
@MylesBorins no worries! I am happy I've managed to explain what I meant! Said that, where are we going to remove
"type": "module",? Just in that example alone or are other places where having that line might be harmful?MylesBorins commentedon Aug 14, 2020
@fox1t I think opening a PR with just removing module should be sufficient
{ "main": "./index.js", "exports": { "import": "./wrapper.mjs", "require": "./index.js" } }fox1t commentedon Aug 17, 2020
Ok, I am going to make the PR.
doc: fix ESM/CJS wrapper example
doc: fix ESM/CJS wrapper example
aduh95 commentedon Dec 23, 2020
Closed by ac3049d