Skip to content

vm: allow dynamic import with a referrer realm - #50360

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
legendecas:vm/realm-import
Nov 1, 2023
Merged

vm: allow dynamic import with a referrer realm#50360
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
legendecas:vm/realm-import

Conversation

@legendecas

Copy link
Copy Markdown
Member

A referrer can be a Script Record, a Cyclic Module Record, or a Realm
Record as defined in https://tc39.es/ecma262/#sec-HostLoadImportedModule.

Add support for dynamic import calls with a realm as the referrer and
allow specifying an importModuleDynamically callback in
vm.createContext.

Dynamic import with a realm as the referrer in a principal realm is not
supported in this PR.

Refs: #49726
Refs: #48655 (comment)

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/vm

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. 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 Oct 24, 2023
@legendecas
legendecasforce-pushed the vm/realm-import branch 3 times, most recently from eb2985c to ee58562CompareOctober 24, 2023 09:18
Comment threadtest/parallel/test-vm-module-referrer-realm.js Outdated
Comment threadtest/parallel/test-vm-module-referrer-realm.js Outdated
Comment threadtest/parallel/test-vm-module-referrer-realm.js Outdated
Comment threadtest/parallel/test-vm-module-referrer-realm.js Outdated
Comment threadsrc/node_contextify.cc Outdated
Comment threadsrc/module_wrap.cc Outdated
Comment threadsrc/node_contextify.cc Outdated
A referrer can be a Script Record, a Cyclic Module Record, or a Realm
Record as defined in https://tc39.es/ecma262/#sec-HostLoadImportedModule.
Add support for dynamic import calls with a realm as the referrer and
allow specifying an `importModuleDynamically` callback in
`vm.createContext`.

@joyeecheungjoyeecheung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@legendecaslegendecas added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 31, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@aduh95aduh95 added experimental Issues and PRs related to experimental features. commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 31, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@legendecaslegendecas added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Nov 1, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 1, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 86afefa into nodejs:mainNov 1, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 86afefa

@legendecas
legendecas deleted the vm/realm-import branch November 1, 2023 06:20
targos pushed a commit that referenced this pull request Nov 11, 2023
A referrer can be a Script Record, a Cyclic Module Record, or a Realm
Record as defined in https://tc39.es/ecma262/#sec-HostLoadImportedModule.
Add support for dynamic import calls with a realm as the referrer and
allow specifying an `importModuleDynamically` callback in
`vm.createContext`.
PR-URL: #50360
Refs: #49726
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@targostargos mentioned this pull request Nov 12, 2023
targos pushed a commit that referenced this pull request Nov 14, 2023
A referrer can be a Script Record, a Cyclic Module Record, or a Realm
Record as defined in https://tc39.es/ecma262/#sec-HostLoadImportedModule.
Add support for dynamic import calls with a realm as the referrer and
allow specifying an `importModuleDynamically` callback in
`vm.createContext`.
PR-URL: #50360
Refs: #49726
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
A referrer can be a Script Record, a Cyclic Module Record, or a Realm
Record as defined in https://tc39.es/ecma262/#sec-HostLoadImportedModule.
Add support for dynamic import calls with a realm as the referrer and
allow specifying an `importModuleDynamically` callback in
`vm.createContext`.
PR-URL: #50360
Refs: #49726
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@UlisesGasconUlisesGascon mentioned this pull request Dec 12, 2023
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
zcbenz pushed a commit to electron/electron that referenced this pull request Jan 12, 2024
zcbenz pushed a commit to electron/electron that referenced this pull request Jan 12, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 15, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 15, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 16, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 18, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Jan 18, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 18, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Jan 18, 2024
* 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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.experimentalIssues and PRs related to experimental features.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@legendecas@nodejs-github-bot@targos@joyeecheung@aduh95