module: bootstrap module loaders in shadow realm #48655

legendecas · 2023-07-05T08:06:13Z

This bootstraps ESM loaders in the ShadowRealm with ShadowRealm.prototype.importValue as its entry point and enables loading ESM and CJS modules in the ShadowRealm. The module is imported without a parent URL and resolved with the current process's working directory.

nodejs-github-bot · 2023-07-05T08:06:17Z

lib/internal/bootstrap/realm.js

lib/internal/vm/module.js

legendecas · 2023-08-15T17:52:02Z

Rebased on the tip of the main branch and removed dependencies on #48510 (with known limitation on memory leaks). Marking as ready to be reviewed. This PR would unblock the verification of built-in modules support in the shadow realm.

nodejs-github-bot · 2023-08-15T18:00:36Z

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

lib/internal/bootstrap/shadow_realm.js

mcollina

lgtm

nodejs-github-bot · 2023-08-17T15:47:23Z

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

GeoffreyBooth · 2023-08-17T16:51:27Z

We’re working on eventually removing the CommonJS loader, leaving only the ESM loader as the single module loader (as it can load both CommonJS and ESM code). Is it possible to refactor this so that ShadowRealm always loads everything only through the ESM loader? Then this part of things won’t need refactoring when the CommonJS loader goes away, and there would also be more uniformity in how modules are loaded within ShadowRealm.

legendecas · 2023-08-17T17:32:09Z

@GeoffreyBooth The ShadowRealm interface only exposes ESM ShadowRealm.prototype.importValue(speficier) and no CJS equivalent. The test case in the PR merely shows that the modules imported in the ShadowRealm can create a require function, just like what we can do in the principal realm.

Are you suggesting this PR didn't load modules in ShadowRealm.prototype.importValue(speficier) with the ESM loader? If so, could you share a hint of how I can fix that?

GeoffreyBooth · 2023-08-17T17:53:29Z

Are you suggesting this PR didn’t load modules in ShadowRealm.prototype.importValue(speficier) with the ESM loader? If so, could you share a hint of how I can fix that?

I was just responding to the PR description, “This bootstraps ESM and CJS module loaders in the ShadowRealm.” Did what you mean to say that it allows for loading ESM and CJS modules in the ShadowRealm?

I don’t really follow what a lot of the code in this PR does. If its only internal interactions with module loaders are through the ESM loader, then we’re good.

legendecas · 2023-08-17T18:16:36Z

Yes, CJS support in ShadowRealm is enabled by ESM loaders. It seems that the PR description is misleading somehow, I've fixed the wording.

GeoffreyBooth · 2023-08-17T19:20:54Z

The module is imported without a parent URL and resolved with the current process’s working directory.

We had a similar situation with node:module register, of needing to resolve relative to process.cwd as an unfortunate default that we dislike but couldn’t find a better alternative for. We added a second parameter to register allowing the user to pass parentURL to resolve against, so that most usage would be like register('ts-node', import.meta.url). You might want to consider the same here.

Also please make sure that the string is interpreted as a file URL and not as a path, because import specifiers must be file URLs and you’re calling this API importValue. Then users can do things like realm.importValue(import.meta.resolve('./file.js'), 'foo').

test/parallel/test-shadow-realm-import-value-resolve.js

This bootstraps ESM loaders in the ShadowRealm with `ShadowRealm.prototype.importValue` as its entry point and enables loading ESM and CJS modules in the ShadowRealm. The module is imported without a parent URL and resolved with the current process's working directory. PR-URL: #48655 Backport-PR-URL: #51239 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) #50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) #50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) #49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) #48655 * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) #48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) #50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) #48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) #48655 stream: * use Array for Readable buffer (Robert Nagy) #50341 * optimize creation (Robert Nagy) #50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443 PR-URL: #51124

Avoid the realm being strongly referenced by the process binding data. PR-URL: #48655 Backport-PR-URL: #51239 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

PR-URL: #48655 Backport-PR-URL: #51239 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

The flag is always true and can be determined by isLoaderWorker solely. PR-URL: #48655 Backport-PR-URL: #51239 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

This bootstraps ESM loaders in the ShadowRealm with `ShadowRealm.prototype.importValue` as its entry point and enables loading ESM and CJS modules in the ShadowRealm. The module is imported without a parent URL and resolved with the current process's working directory. PR-URL: #48655 Backport-PR-URL: #51239 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) #50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) #50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) #49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) #48655 * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) #48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) #50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) #48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) #48655 stream: * use Array for Readable buffer (Robert Nagy) #50341 * optimize creation (Robert Nagy) #50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443 PR-URL: #51124

nodejs/node#48655

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) nodejs#50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) nodejs#50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) nodejs#50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) nodejs#48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) nodejs#49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) nodejs#48655 * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) nodejs#48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) nodejs#50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) nodejs#48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) nodejs#48655 stream: * use Array for Readable buffer (Robert Nagy) nodejs#50341 * optimize creation (Robert Nagy) nodejs#50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) nodejs#50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) nodejs#48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) nodejs#50443 PR-URL: nodejs#51124

We now serialize/deseialize the hrtime buffers properly instead of throwing them away at serialization and creating new ones at pre-execution, so there is no need to reset the variables to the binding property at pre-execution time. PR-URL: #51446 Refs: #48655 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>

nodejs/node#48655

* chore: bump node in DEPS to v20.11.0 * module: bootstrap module loaders in shadow realm nodejs/node#48655 * src: add commit hash shorthand in zlib version nodejs/node#50158 * v8,tools: expose necessary V8 defines nodejs/node#50820 * esm: do not call getSource when format is commonjs nodejs/node#50465 * esm: fallback to readFileSync when source is nullish nodejs/node#50825 * vm: allow dynamic import with a referrer realm nodejs/node#50360 * test: skip test-diagnostics-channel-memory-leak.js nodejs/node#50327 * esm: do not call getSource when format is commonjs nodejs/node#50465 * lib: fix assert throwing different error messages in ESM and CJS nodejs/node#50634 * src: fix compatility with upcoming V8 12.1 APIs nodejs/node#50709 * deps: update base64 to 0.5.1 nodejs/node#50629 * src: avoid silent coercion to signed/unsigned int nodejs/node#50663 * src: fix compatility with upcoming V8 12.1 APIs nodejs/node#50709 * chore: fix patch indices * chore: update patches * test: disable TLS cipher test This can't be enabled owing to BoringSSL incompatibilities. nodejs/node#50186 * fix: check for Buffer and global definition in shadow realm nodejs/node#51239 * test: disable parallel/test-shadow-realm-custom-loader Incompatible with our asar logic, resulting in the following failure: > Failed to CompileAndCall electron script: electron/js2c/asar_bundle * chore: remove deleted parallel/test-crypto-modp1-error test * test: make test-node-output-v8-warning generic nodejs/node#50421 * chore: fixup ModuleWrap patch * test: match wpt/streams/transferable/transform-stream-members.any.js to upstream * fix: sandbox is not enabled on arm * chore: disable v8 sandbox on ia32/arm --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: Cheng Zhao <zcbenz@gmail.com>

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) nodejs#50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) nodejs#50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) nodejs#50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) nodejs#48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) nodejs#49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) nodejs#48655 * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) nodejs#48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) nodejs#50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) nodejs#48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) nodejs#48655 stream: * use Array for Readable buffer (Robert Nagy) nodejs#50341 * optimize creation (Robert Nagy) nodejs#50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) nodejs#50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) nodejs#48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) nodejs#50443 PR-URL: nodejs#51124

We now serialize/deseialize the hrtime buffers properly instead of throwing them away at serialization and creating new ones at pre-execution, so there is no need to reset the variables to the binding property at pre-execution time. PR-URL: nodejs#51446 Refs: nodejs#48655 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>

We now serialize/deseialize the hrtime buffers properly instead of throwing them away at serialization and creating new ones at pre-execution, so there is no need to reset the variables to the binding property at pre-execution time. PR-URL: #51446 Refs: #48655 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 5, 2023

legendecas added module Issues and PRs related to the module subsystem. realm Issues and PRs related to the ShadowRealm API and node::Realm labels Jul 5, 2023

ljharb reviewed Jul 5, 2023

View reviewed changes

lib/internal/bootstrap/realm.js Outdated Show resolved Hide resolved

lib/internal/vm/module.js Outdated Show resolved Hide resolved

legendecas force-pushed the shadow-realm/module branch from e1e63e3 to 23a9cb4 Compare August 15, 2023 16:38

legendecas marked this pull request as ready for review August 15, 2023 17:52

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 15, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 15, 2023

joyeecheung reviewed Aug 15, 2023

View reviewed changes

lib/internal/bootstrap/shadow_realm.js Outdated Show resolved Hide resolved

mcollina closed this Aug 16, 2023

mcollina reopened this Aug 16, 2023

mcollina approved these changes Aug 16, 2023

View reviewed changes

legendecas force-pushed the shadow-realm/module branch from 23a9cb4 to f1f3986 Compare August 17, 2023 15:44

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2023

GeoffreyBooth reviewed Aug 17, 2023

View reviewed changes

test/parallel/test-shadow-realm-import-value-resolve.js Outdated Show resolved Hide resolved

This was referenced Aug 18, 2023

CI Reliability 2023-08-18 nodejs/reliability#640

Open

CI Reliability 2023-08-21 nodejs/reliability#641

Open

legendecas mentioned this pull request Aug 21, 2023

test: verify dynamic import call with absolute path strings #49275

Merged

github-actions bot mentioned this pull request Aug 22, 2023

CI Reliability 2023-08-22 nodejs/reliability#642

Open

21 tasks

codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024

module: bootstrap module loaders in shadow realm

0f1bee6

nodejs/node#48655

zcbenz pushed a commit to electron/electron that referenced this pull request Jan 12, 2024

module: bootstrap module loaders in shadow realm

762008a

nodejs/node#48655

zcbenz pushed a commit to electron/electron that referenced this pull request Jan 12, 2024

module: bootstrap module loaders in shadow realm

03f587a

nodejs/node#48655

joyeecheung mentioned this pull request Jan 12, 2024

lib: remove unnecessary refreshHrtimeBuffer() #51446

Merged

codebytere added a commit to electron/electron that referenced this pull request Jan 15, 2024

module: bootstrap module loaders in shadow realm

5f75352

nodejs/node#48655

codebytere added a commit to electron/electron that referenced this pull request Jan 15, 2024

module: bootstrap module loaders in shadow realm

a8bf498

nodejs/node#48655

codebytere added a commit to electron/electron that referenced this pull request Jan 16, 2024

module: bootstrap module loaders in shadow realm

6259355

nodejs/node#48655

codebytere added a commit to electron/electron that referenced this pull request Jan 18, 2024

module: bootstrap module loaders in shadow realm

9ee98f6

nodejs/node#48655

jkleinsc pushed a commit to electron/electron that referenced this pull request Jan 18, 2024

module: bootstrap module loaders in shadow realm

53628a7

nodejs/node#48655

codebytere added a commit to electron/electron that referenced this pull request Jan 18, 2024

module: bootstrap module loaders in shadow realm

b0c244a

nodejs/node#48655

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

module: bootstrap module loaders in shadow realm #48655

module: bootstrap module loaders in shadow realm #48655

legendecas commented Jul 5, 2023 •

edited

nodejs-github-bot commented Jul 5, 2023 •

edited by legendecas

legendecas commented Aug 15, 2023 •

edited

nodejs-github-bot commented Aug 15, 2023

mcollina left a comment

nodejs-github-bot commented Aug 17, 2023

GeoffreyBooth commented Aug 17, 2023

legendecas commented Aug 17, 2023

GeoffreyBooth commented Aug 17, 2023

legendecas commented Aug 17, 2023

GeoffreyBooth commented Aug 17, 2023 •

edited

module: bootstrap module loaders in shadow realm #48655

module: bootstrap module loaders in shadow realm #48655

Conversation

legendecas commented Jul 5, 2023 • edited

nodejs-github-bot commented Jul 5, 2023 • edited by legendecas

legendecas commented Aug 15, 2023 • edited

nodejs-github-bot commented Aug 15, 2023

mcollina left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Aug 17, 2023

GeoffreyBooth commented Aug 17, 2023

legendecas commented Aug 17, 2023

GeoffreyBooth commented Aug 17, 2023

legendecas commented Aug 17, 2023

GeoffreyBooth commented Aug 17, 2023 • edited

legendecas commented Jul 5, 2023 •

edited

nodejs-github-bot commented Jul 5, 2023 •

edited by legendecas

legendecas commented Aug 15, 2023 •

edited

GeoffreyBooth commented Aug 17, 2023 •

edited