Home Original page

sea: allow requiring core modules with the "node:" prefix by RaisinTen · Pull Request #47779 · nodejs/node

@RaisinTen 

Previously, the `require()` function exposed to the embedded SEA code
was calling the internal `require()` function if the module name
belonged to the list of public core modules but the internal `require()`
function does not support loading modules with the "node:" prefix, so
this change forwards the calls to another `require()` function that
supports this.

Fixes: nodejs/single-executable#69
Signed-off-by: Darshan Sen <raisinten@gmail.com>

@RaisinTen added the single-executable

Issues and PRs related to single-executable applications

label

Apr 29, 2023

@RaisinTen RaisinTen added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Apr 29, 2023

joyeecheung

mhdawson

joyeecheung

This was referenced

May 2, 2023

@RaisinTen 

…e with the SEA require

Previously, this code:
```sh
node --snapshot-blob snapshot.blob --build-snapshot <(echo "require('node:test')")`
```
was failing with a "Error: Cannot find module 'node:test'." error, so
this change fixes that and uses the same code path for normalizing ids
for both the snapshot require function as well as the SEA require
function.

However, building a snapshot when `node:test` is required now fails with:
```sh
./node --snapshot-blob snapshot.blob --build-snapshot <(echo "require('node:test')")
(node:5362) Warning: built-in module node:test is not yet supported in user snapshots
(Use `node --trace-warnings ...` to show where the warning was created)
Unknown external reference 0x10201dd70.
<unresolved>
[1]    5362 trace trap  ./node --snapshot-blob snapshot.blob --build-snapshot
```
which could be solved in a separate PR.

Refs: nodejs#47779 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>

joyeecheung

@RaisinTen RaisinTen added commit-queue

Add this label to land a pull request using GitHub Actions.

commit-queue-rebase

Add this label to allow the Commit Queue to land a PR in several commits.

commit-queue-squash

Add this label to instruct the Commit Queue to squash all the PR commits into the first one.

and removed commit-queue-rebase

Add this label to allow the Commit Queue to land a PR in several commits.

labels

May 4, 2023

@RaisinTen RaisinTen deleted the sea-allow-requiring-core-modules-with-node-prefix branch

May 4, 2023 14:33

RaisinTen added a commit to RaisinTen/node that referenced this pull request

May 6, 2023 
`BuiltinModule.normalizeRequirableId()` was introduced in
nodejs#47779 to fix a bug in the require
function of SEAs and Snapshots, so that built-in modules with the
`node:` scheme could be required correctly. This change makes more use
of this API instead of `BuiltinModule.canBeRequiredByUsers()` and
`BuiltinModule.canBeRequiredWithoutScheme()` to reduce chances of
such bugs.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

nodejs-github-bot pushed a commit that referenced this pull request

May 8, 2023 
`BuiltinModule.normalizeRequirableId()` was introduced in
#47779 to fix a bug in the require
function of SEAs and Snapshots, so that built-in modules with the
`node:` scheme could be required correctly. This change makes more use
of this API instead of `BuiltinModule.canBeRequiredByUsers()` and
`BuiltinModule.canBeRequiredWithoutScheme()` to reduce chances of
such bugs.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: #47896
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

targos pushed a commit that referenced this pull request

May 12, 2023 
Previously, the `require()` function exposed to the embedded SEA code
was calling the internal `require()` function if the module name
belonged to the list of public core modules but the internal `require()`
function does not support loading modules with the "node:" prefix, so
this change forwards the calls to another `require()` function that
supports this.

Fixes: nodejs/single-executable#69
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: #47779
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

targos pushed a commit that referenced this pull request

May 12, 2023 
`BuiltinModule.normalizeRequirableId()` was introduced in
#47779 to fix a bug in the require
function of SEAs and Snapshots, so that built-in modules with the
`node:` scheme could be required correctly. This change makes more use
of this API instead of `BuiltinModule.canBeRequiredByUsers()` and
`BuiltinModule.canBeRequiredWithoutScheme()` to reduce chances of
such bugs.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: #47896
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

targos pushed a commit that referenced this pull request

Nov 10, 2023 
Previously, the `require()` function exposed to the embedded SEA code
was calling the internal `require()` function if the module name
belonged to the list of public core modules but the internal `require()`
function does not support loading modules with the "node:" prefix, so
this change forwards the calls to another `require()` function that
supports this.

Fixes: nodejs/single-executable#69
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: #47779
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>

targos pushed a commit that referenced this pull request

Nov 10, 2023 
`BuiltinModule.normalizeRequirableId()` was introduced in
#47779 to fix a bug in the require
function of SEAs and Snapshots, so that built-in modules with the
`node:` scheme could be required correctly. This change makes more use
of this API instead of `BuiltinModule.canBeRequiredByUsers()` and
`BuiltinModule.canBeRequiredWithoutScheme()` to reduce chances of
such bugs.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: #47896
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

sercher added a commit to sercher/graaljs that referenced this pull request

Apr 25, 2024 
Previously, the `require()` function exposed to the embedded SEA code
was calling the internal `require()` function if the module name
belonged to the list of public core modules but the internal `require()`
function does not support loading modules with the "node:" prefix, so
this change forwards the calls to another `require()` function that
supports this.

Fixes: nodejs/single-executable#69
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs/node#47779
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>

sercher added a commit to sercher/graaljs that referenced this pull request

Apr 25, 2024 
`BuiltinModule.normalizeRequirableId()` was introduced in
nodejs/node#47779 to fix a bug in the require
function of SEAs and Snapshots, so that built-in modules with the
`node:` scheme could be required correctly. This change makes more use
of this API instead of `BuiltinModule.canBeRequiredByUsers()` and
`BuiltinModule.canBeRequiredWithoutScheme()` to reduce chances of
such bugs.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs/node#47896
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

sercher added a commit to sercher/graaljs that referenced this pull request

Apr 25, 2024 
Previously, the `require()` function exposed to the embedded SEA code
was calling the internal `require()` function if the module name
belonged to the list of public core modules but the internal `require()`
function does not support loading modules with the "node:" prefix, so
this change forwards the calls to another `require()` function that
supports this.

Fixes: nodejs/single-executable#69
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs/node#47779
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>

sercher added a commit to sercher/graaljs that referenced this pull request

Apr 25, 2024 
`BuiltinModule.normalizeRequirableId()` was introduced in
nodejs/node#47779 to fix a bug in the require
function of SEAs and Snapshots, so that built-in modules with the
`node:` scheme could be required correctly. This change makes more use
of this API instead of `BuiltinModule.canBeRequiredByUsers()` and
`BuiltinModule.canBeRequiredWithoutScheme()` to reduce chances of
such bugs.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs/node#47896
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request

May 5, 2024 
Previously, the `require()` function exposed to the embedded SEA code
was calling the internal `require()` function if the module name
belonged to the list of public core modules but the internal `require()`
function does not support loading modules with the "node:" prefix, so
this change forwards the calls to another `require()` function that
supports this.

Fixes: nodejs/single-executable#69
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs/node#47779
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>