module: move helpers out of CommonJS loader by GeoffreyBooth · Pull Request #49912 · nodejs/node · GitHub

Following up #49869, supporting #49432, this PR moves some methods that are used by both the CommonJS and ESM startup flows out of cjs/loader.js and into shared common files. This is a precursor for the next PR that refactors how the initial entry point is handled in ESM-first mode.

All this PR does is move the code; nothing is changed other than references to variables. @nodejs/loaders

Review requested:

@nodejs/loaders

CI: https://ci.nodejs.org/job/node-test-pull-request/54319/

CI: https://ci.nodejs.org/job/node-test-pull-request/54329/

CI: https://ci.nodejs.org/job/node-test-pull-request/54338/

CI: https://ci.nodejs.org/job/node-test-pull-request/54378/

CI: https://ci.nodejs.org/job/node-test-pull-request/54382/

Commit Queue failed

- Loading data for nodejs/node/pull/49912
✔  Done loading data for nodejs/node/pull/49912
----------------------------------- PR info ------------------------------------
Title      module: move helpers out of CommonJS loader (#49912)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     GeoffreyBooth:move-cjs-helpers -> nodejs:main
Labels     module, esm, author ready, needs-ci, commit-queue-squash
Commits    2
 - module: move helpers out of cjs loader
 - readPackage and readPackageScope
Committers 1
 - Geoffrey Booth 
PR-URL: https://github.com/nodejs/node/pull/49912
Reviewed-By: Antoine du Hamel 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49912
Reviewed-By: Antoine du Hamel 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - module: move helpers out of cjs loader
   ⚠  - readPackage and readPackageScope
   ℹ  This PR was created on Wed, 27 Sep 2023 22:57:40 GMT
   ✔  Approvals: 2
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/49912#pullrequestreview-1648294902
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/49912#pullrequestreview-1649641847
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-09-29T17:25:32Z: https://ci.nodejs.org/job/node-test-pull-request/54382/
- Querying data for job/node-test-pull-request/54382/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/6358059657

Landed in 51f4ff2

PR-URL: #49912 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

PR-URL: nodejs#49912 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

PR-URL: #49912 Backport-PR-URL: #50669 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Xref: nodejs/node#49912 no code changes; patch just needed an update due to nearby upstream changes

* chore: bump node in DEPS to v20.10.0 * chore: update feat_initialize_asar_support.patch no code changes; patch just needed an update due to nearby upstream changes Xref: nodejs/node#49986 * chore: update pass_all_globals_through_require.patch no manual changes; patch applied with fuzz Xref: nodejs/node#49657 * chore: update refactor_allow_embedder_overriding_of_internal_fs_calls Xref: nodejs/node#49912 no code changes; patch just needed an update due to nearby upstream changes * chore: update chore_allow_the_node_entrypoint_to_be_a_builtin_module.patch Xref: nodejs/node#49986 minor manual changes needed to sync with upstream change * update fix_expose_the_built-in_electron_module_via_the_esm_loader.patch Xref: nodejs/node#50096 Xref: nodejs/node#50314 in lib/internal/modules/esm/load.js, update the code that checks for `format === 'electron'`. I'd like 👀 on this Xref: nodejs/node#49657 add braces in lib/internal/modules/esm/translators.js to sync with upstream * fix: lazyload fs in esm loaders to apply asar patches * nodejs/node#50127 * nodejs/node#50096 * esm: jsdoc for modules code nodejs/node#49523 * test: set test-cli-node-options as flaky nodejs/node#50296 * deps: update c-ares to 1.20.1 nodejs/node#50082 * esm: bypass CommonJS loader under --default-type=module nodejs/node#49986 * deps: update uvwasi to 0.0.19 nodejs/node#49908 * lib,test: do not hardcode Buffer.kMaxLength nodejs/node#49876 * crypto: account for disabled SharedArrayBuffer nodejs/node#50034 * test: fix edge snapshot stack traces nodejs/node#49659 * src: generate snapshot with --predictable nodejs/node#48749 * chore: fixup patch indices * fs: throw errors from sync branches instead of separate implementations nodejs/node#49913 * crypto: ensure valid point on elliptic curve in SubtleCrypto.importKey nodejs/node#50234 * esm: detect ESM syntax in ambiguous JavaScrip nodejs/node#50096 * fixup! test: fix edge snapshot stack traces * esm: unflag extensionless ES module JavaScript and Wasm in module scope nodejs/node#49974 * [tagged-ptr] Arrowify objects https://chromium-review.googlesource.com/c/v8/v8/+/4705331 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Charles Kerr <charles@charleskerr.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

PR-URL: nodejs#49912 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

PR-URL: nodejs/node#49912 Backport-PR-URL: nodejs/node#50669 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

GeoffreyBooth added module Issues and PRs related to the module subsystem. esm Issues and PRs related to the ECMAScript Modules implementation. labels Sep 27, 2023

GeoffreyBooth requested a review from aduh95 September 27, 2023 22:57

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Sep 27, 2023

aduh95 approved these changes Sep 28, 2023

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2023

lpinca approved these changes Sep 28, 2023

View reviewed changes

aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 29, 2023

GeoffreyBooth added 2 commits September 29, 2023 08:49

module: move helpers out of cjs loader

ed3ca73

readPackage and readPackageScope

3ea0b32

GeoffreyBooth force-pushed the move-cjs-helpers branch from b87ab39 to 3ea0b32 Compare September 29, 2023 15:56

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 29, 2023

github-actions Bot mentioned this pull request Sep 30, 2023

CI Reliability 2023-09-30 nodejs/reliability#679

Open

31 tasks

anonrig approved these changes Sep 30, 2023

View reviewed changes

GeoffreyBooth added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Sep 30, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 30, 2023

nodejs-github-bot merged commit 51f4ff2 into nodejs:main Sep 30, 2023

github-actions Bot mentioned this pull request Oct 1, 2023

CI Reliability 2023-10-01 nodejs/reliability#680

Open

16 tasks

github-actions Bot mentioned this pull request Oct 2, 2023

CI Reliability 2023-10-02 nodejs/reliability#681

Open

12 tasks

RafaelGSS mentioned this pull request Oct 9, 2023

2023-10-17, Version 21.0.0 (Current) #49870

Merged

targos mentioned this pull request Nov 11, 2023

[v18.x] Backport most ESM and customization hook changes #50669

Closed

targos pushed a commit that referenced this pull request Nov 11, 2023

module: move helpers out of cjs loader

2ff4e71

PR-URL: #49912 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

targos mentioned this pull request Nov 12, 2023

v20.10.0 release proposal #50682

Merged

targos added the backported-to-v18.x label Nov 23, 2023

targos mentioned this pull request Nov 28, 2023

v18.19.0 release proposal #50953

Merged

juanarbol mentioned this pull request Dec 6, 2023

2023-12-06, N|Solid v5.0.1 Iron Release nodesource/nsolid#41

Merged

ckerr added a commit to electron/electron that referenced this pull request Dec 7, 2023

chore: update refactor_allow_embedder_overriding_of_internal_fs_calls

a5b9475

Xref: nodejs/node#49912 no code changes; patch just needed an update due to nearby upstream changes

codebytere pushed a commit to electron/electron that referenced this pull request Dec 8, 2023

chore: update refactor_allow_embedder_overriding_of_internal_fs_calls

4cbd676

Xref: nodejs/node#49912 no code changes; patch just needed an update due to nearby upstream changes

module: move helpers out of CommonJS loader by GeoffreyBooth · Pull Request #49912 · nodejs/node · GitHub

Uh oh!

Uh oh!

module: move helpers out of CommonJS loader - #49912

module: move helpers out of CommonJS loader#49912
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
GeoffreyBooth:move-cjs-helpers

GeoffreyBooth commented Sep 27, 2023

Uh oh!

nodejs-github-bot commented Sep 27, 2023

Uh oh!

nodejs-github-bot commented Sep 28, 2023

Uh oh!

nodejs-github-bot commented Sep 28, 2023

Uh oh!

nodejs-github-bot commented Sep 28, 2023

Uh oh!

nodejs-github-bot commented Sep 29, 2023

Uh oh!

nodejs-github-bot commented Sep 29, 2023

Uh oh!

nodejs-github-bot commented Sep 29, 2023

Uh oh!

nodejs-github-bot commented Sep 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

module: move helpers out of CommonJS loader by GeoffreyBooth · Pull Request #49912 · nodejs/node · GitHub

Uh oh!

Uh oh!

module: move helpers out of CommonJS loader - #49912

module: move helpers out of CommonJS loader#49912nodejs-github-bot merged 2 commits intonodejs:mainnodejs/node:mainfrom GeoffreyBooth:move-cjs-helpersGeoffreyBooth/node:move-cjs-helpersCopy head branch name to clipboard

Conversation

GeoffreyBooth commented Sep 27, 2023

Uh oh!

nodejs-github-bot commented Sep 27, 2023

Uh oh!

nodejs-github-bot commented Sep 28, 2023

Uh oh!

nodejs-github-bot commented Sep 28, 2023

Uh oh!

nodejs-github-bot commented Sep 28, 2023

Uh oh!

nodejs-github-bot commented Sep 29, 2023

Uh oh!

nodejs-github-bot commented Sep 29, 2023

Uh oh!

nodejs-github-bot commented Sep 29, 2023

Uh oh!

nodejs-github-bot commented Sep 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

module: move helpers out of CommonJS loader#49912
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
GeoffreyBooth:move-cjs-helpers