module: bootstrap module loaders in shadow realm by legendecas · Pull Request #48655 · nodejs/node

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

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/

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:moduleregister, 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').

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 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>

Avoid the realm being strongly referenced by the process binding data. PR-URL: #48655 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 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 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 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 deps: * add simdjson (Yagiz Nizipli) #50322 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) #49884 module: * merge config with `package_json_reader` (Yagiz Nizipli) #50322 * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) #48655 * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) #48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) #50661 * move package resolver to c++ (Yagiz Nizipli) #50322 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) #48655 * (SEMVER-MINOR) create fs_dir per isolate properties (Chengzhong Wu) #48655 * (SEMVER-MINOR) create worker per isolate properties (Chengzhong Wu) #48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) #48655 PR-URL: #50954

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) #50805 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) #49884 module: * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) #48655 * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (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) create fs_dir per isolate properties (Chengzhong Wu) #48655 * (SEMVER-MINOR) create worker per isolate properties (Chengzhong Wu) #48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) #48655 PR-URL: #50954

Avoid the realm being strongly referenced by the process binding data. PR-URL: #48655 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 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>

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

Comment threadlib/internal/bootstrap/realm.js Outdated
Comment threadlib/internal/vm/module.js Outdated

legendecas force-pushed the shadow-realm/module branch from e1e63e3 to 23a9cb4Compare 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-actionsBot 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

Comment threadlib/internal/bootstrap/shadow_realm.js Outdated

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 f1f3986Compare 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-actionsBot 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

Comment threadtest/parallel/test-shadow-realm-import-value-resolve.js Outdated

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-actionsBot mentioned this pull request Aug 22, 2023
CI Reliability 2023-08-22 nodejs/reliability#642
Open
21 tasks

legendecas closed this Nov 13, 2023

legendecas deleted the shadow-realm/module branch November 13, 2023 14:11

This was referenced Nov 14, 2023
test-shadow-realm-gc-module can crash on macOS (OOM) #50726
Closed
src: iterate on import attributes array correctly #50703
Merged

joyeecheung mentioned this pull request Nov 16, 2023
parallel/test-worker-nearheaplimit-deadlock is flaky #50761
Closed

RafaelGSS mentioned this pull request Nov 28, 2023
v21.3.0 proposal #50954
Merged

UlisesGascon mentioned this pull request Dec 12, 2023
v20.11.0 proposal #51124
Merged

legendecas mentioned this pull request Dec 20, 2023
[v20.x] module: bootstrap module loaders in shadow realm #51239
Closed

joyeecheung mentioned this pull request Jan 12, 2024
lib: remove unnecessary refreshHrtimeBuffer() #51446
Merged

Navigation Menu

Uh oh!

Uh oh!

module: bootstrap module loaders in shadow realm - #48655

module: bootstrap module loaders in shadow realm#48655
legendecas wants to merge 6 commits into
nodejs:mainfrom
legendecas:shadow-realm/module

legendecas commented Jul 5, 2023•
edited
Loading

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

legendecas commented Aug 15, 2023•
edited
Loading

Uh oh!

nodejs-github-bot commented Aug 15, 2023

Uh oh!

Uh oh!

mcollina left a comment

Uh oh!

nodejs-github-bot commented Aug 17, 2023

Uh oh!

GeoffreyBooth commented Aug 17, 2023

Uh oh!

legendecas commented Aug 17, 2023

Uh oh!

GeoffreyBooth commented Aug 17, 2023

Uh oh!

legendecas commented Aug 17, 2023

Uh oh!

GeoffreyBooth commented Aug 17, 2023•
edited
Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Uh oh!

Conversation

legendecas commented Jul 5, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 5, 2023• edited by legendecas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

legendecas commented Aug 15, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 15, 2023

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Aug 17, 2023

Uh oh!

GeoffreyBooth commented Aug 17, 2023

Uh oh!

legendecas commented Aug 17, 2023

Uh oh!

GeoffreyBooth commented Aug 17, 2023

Uh oh!

legendecas commented Aug 17, 2023

Uh oh!

GeoffreyBooth commented Aug 17, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

legendecas commented Jul 5, 2023•
edited
Loading

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

legendecas commented Aug 15, 2023•
edited
Loading

GeoffreyBooth commented Aug 17, 2023•
edited
Loading