Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[v12 backport] ESM (named exports for CJS, package.json docs, subpath export patterns, warning removal) #35757
Conversation
|
Review requested: |
aduh95
force-pushed the
aduh95:backport-esm-history-to-v12
branch
from
3c94340
to
16fcd04
aduh95
changed the title
aduh95
force-pushed the
aduh95:backport-esm-history-to-v12
branch
2 times, most recently
from
6d32a9a
to
db5cf81
|
@guybedford This PR includes a backport for #34718 and I've also cherry-picked your commit in #35749. Can you have a look to check if I haven't done something odd? |
|
Thanks for picking this one up! I had a brief skim through and couldn't see anything out of place. I've added this to the list at #35749 as well. |
|
@guybedford I just realised in #35749 you said that the export pattern support should be backported, but I actually did that in this PR. That's because I cherry-picked all the commits in the @nodejs/backporters What should be the process here? Should I split this PR into three (documentation-only commits that landed before the export pattern one, export pattern support, documentation-only after the export pattern commit), or should I keep it all in this PR? I think it makes sense to land all of it in one bulk, but that mixes up semver-minor commits, I wouldn't want to make things difficult for you if that's a problem. |
|
@aduh95 a single backport PR to bring the modules implementation in 12.x up to date is preferable imho. Does this include everything that has landed up until now? We might want to bring in more commits, not less :P |
PR-URL: #35249 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
PR-URL: #35426 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: #35501 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
aduh95
force-pushed the
aduh95:backport-esm-history-to-v12
branch
from
db5cf81
to
85a2187
Using a "Modules:" prefix groups all the related pages together when using alphabetical order. Refs: nodejs/modules#539 PR-URL: #34663 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
This part of the docs aims to contain documentation regarding package configuration that covers both ESM and CJS realms. * Move Enabling section * Update Enabling section * Remove -u flag * Package scopes do not carry through `node_modules` folders Refs: nodejs/modules#539 Co-authored-by: Geoffrey Booth <webmaster@geoffreybooth.com>> Co-authored-by: Guy Bedford <guybedford@gmail.com> PR-URL: #34748 Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Ruy Adorno <ruyadorno@github.com>
PR-URL: #34718 Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* Use parallel construction in the two sentences
* Backticks around _package.json_ to match rest of file
* Add comma for readability
* Own the recommendation ("we recommend")
PR-URL: #35254
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
PR-URL: #35311 Reviewed-By: Jan Krems <jan.krems@gmail.com>
PR-URL: #35370 Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Some minor adjustments, mostly moving things to present tense and avoiding the ambiguity of _may_. Refs: #35370 (comment) PR-URL: #35427 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #35428 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Documents which versions of Node.js support which ESM-feature. PR-URL: #35395 Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #31974 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #35750 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>
The dynamicInstantiate loader hook requires that the hooks run in the same global scope as the code being loaded. We don't want to commit to this being true in the future. It stops us from sharing hooks between multiple worker threads or isolating loader hook from the application code. Using `getSource` and `getGlobalPreloadCode` the same use cases should be covered. PR-URL: #33501 Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
PR-URL: #35387 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
aduh95
force-pushed the
aduh95:backport-esm-history-to-v12
branch
from
85a2187
to
0fffe00
aduh95
changed the title
|
@MylesBorins Gotcha! I've added a few more commits, I think I got them all. |
|
Remarking my approval to be clear this seems comprehensive and correct with the new list. |
|
I would suggest though that we do not include the "remove dynamicInstantiate hook" commit, as in theory that is a breaking change and while even on the experimental implementation, it seems unnecessary friction to me. |
No strong opinion from my side, I included it because it was labeled |
|
If the rebasing isn't too bad I think we should leave it out actually. No need to break users unnecessarily. I remember for example early Wasm loaders were using this. |
This PR backports a bunch of ESM related PRs:
Named exports for CJS:
packages.mdandpackage.jsonfields documentationsubpath export patterns:
Various documentation improvments:
ESM experimental warning removal:
Other ESM features not yet backported:
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes