◐ Shell
clean mode source ↗

module: handle null source from async loader hooks in sync hooks by joyeecheung · Pull Request #59929 · nodejs/node

@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

Sep 18, 2025

GeoffreyBooth

JakobJingleheimer

This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

@joyeecheung

JakobJingleheimer

aduh95 pushed a commit that referenced this pull request

Oct 23, 2025
This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

PR-URL: #59929
Fixes: #59384
Fixes: #57327
Refs: #59666
Refs: https://github.com/dygabo/load_module_test
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

aduh95 pushed a commit that referenced this pull request

Oct 31, 2025
This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

PR-URL: #59929
Fixes: #59384
Fixes: #57327
Refs: #59666
Refs: https://github.com/dygabo/load_module_test
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request

Nov 3, 2025

aduh95 pushed a commit that referenced this pull request

Nov 7, 2025
This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

PR-URL: #59929
Fixes: #59384
Fixes: #57327
Refs: #59666
Refs: https://github.com/dygabo/load_module_test
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

jkleinsc added a commit to electron/electron that referenced this pull request

Nov 14, 2025

jkleinsc added a commit to electron/electron that referenced this pull request

Nov 18, 2025
* chore: bump node in DEPS to v24.11.1

* src: add a default branch for module phase

nodejs/node#60261

* src: conditionally disable source phase imports by default

nodejs/node#60364

* chore: update patches

* src: update locks to use DictionaryTemplate and other minor cleanups

nodejs/node#60061

* deps: update simdjson to 4.0.7

nodejs/node#59883

* test: move sea tests into test/sea

nodejs/node#60250

* fixup deps: update simdjson to 4.0.7a

* src: conditionally disable source phase imports by default

nodejs/node#60364

* module: handle null source from async loader hooks in sync hooks

nodejs/node#59929

* Revert "src: conditionally disable source phase imports by default"

This reverts commit 5f85b84.

* src: allow disabling JS source phase imports

nodejs/node#60364

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

jkleinsc added a commit to electron/electron that referenced this pull request

Nov 18, 2025
* chore: bump node in DEPS to v24.11.1

* src: add a default branch for module phase

nodejs/node#60261

* src: conditionally disable source phase imports by default

nodejs/node#60364

* chore: update patches

* src: update locks to use DictionaryTemplate and other minor cleanups

nodejs/node#60061

* deps: update simdjson to 4.0.7

nodejs/node#59883

* test: move sea tests into test/sea

nodejs/node#60250

* fixup deps: update simdjson to 4.0.7a

* src: conditionally disable source phase imports by default

nodejs/node#60364

* module: handle null source from async loader hooks in sync hooks

nodejs/node#59929

* Revert "src: conditionally disable source phase imports by default"

This reverts commit 5f85b84.

* src: allow disabling JS source phase imports

nodejs/node#60364

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

jkleinsc added a commit to electron/electron that referenced this pull request

Nov 18, 2025
* chore: bump node in DEPS to v24.11.1

* src: add a default branch for module phase

nodejs/node#60261

* src: conditionally disable source phase imports by default

nodejs/node#60364

* chore: update patches

* src: update locks to use DictionaryTemplate and other minor cleanups

nodejs/node#60061

* deps: update simdjson to 4.0.7

nodejs/node#59883

* test: move sea tests into test/sea

nodejs/node#60250

* fixup deps: update simdjson to 4.0.7a

* src: conditionally disable source phase imports by default

nodejs/node#60364

* module: handle null source from async loader hooks in sync hooks

nodejs/node#59929

* Revert "src: conditionally disable source phase imports by default"

This reverts commit 5f85b84.

* src: allow disabling JS source phase imports

nodejs/node#60364

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

jkleinsc added a commit to electron/electron that referenced this pull request

Nov 18, 2025
* chore: bump node in DEPS to v24.11.1

* chore: bump node to v24.11.1 (main) (#48917)

* chore: bump node in DEPS to v24.11.1

* src: add a default branch for module phase

nodejs/node#60261

* src: conditionally disable source phase imports by default

nodejs/node#60364

* chore: update patches

* src: update locks to use DictionaryTemplate and other minor cleanups

nodejs/node#60061

* deps: update simdjson to 4.0.7

nodejs/node#59883

* test: move sea tests into test/sea

nodejs/node#60250

* fixup deps: update simdjson to 4.0.7a

* src: conditionally disable source phase imports by default

nodejs/node#60364

* module: handle null source from async loader hooks in sync hooks

nodejs/node#59929

* Revert "src: conditionally disable source phase imports by default"

This reverts commit 5f85b84.

* src: allow disabling JS source phase imports

nodejs/node#60364

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

@sxzz sxzz mentioned this pull request

Feb 13, 2026

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

Feb 25, 2026
This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

PR-URL: nodejs#59929
Fixes: nodejs#59384
Fixes: nodejs#57327
Refs: nodejs#59666
Refs: https://github.com/dygabo/load_module_test
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

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

Feb 25, 2026
This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

PR-URL: nodejs#59929
Fixes: nodejs#59384
Fixes: nodejs#57327
Refs: nodejs#59666
Refs: https://github.com/dygabo/load_module_test
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

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

Feb 27, 2026
This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

PR-URL: nodejs#59929
Fixes: nodejs#59384
Fixes: nodejs#57327
Refs: nodejs#59666
Refs: https://github.com/dygabo/load_module_test
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

marco-ippolito pushed a commit that referenced this pull request

Apr 27, 2026
This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

PR-URL: #59929
Backport-PR-URL: #62029
Fixes: #59384
Fixes: #57327
Refs: #59666
Refs: https://github.com/dygabo/load_module_test
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Fixes: #61801

BridgeAR added a commit to BridgeAR/import-in-the-middle that referenced this pull request

Jun 17, 2026
module.registerHooks exists since v22.15, but its synchronous load hook rejected
the nullish CommonJS source the loader returns for require()s pulled into the ESM
graph (ERR_INVALID_RETURN_PROPERTY_VALUE) until nodejs/node#59929, released in
22.22.3, 24.11.1, 25.1.0 and 26.0.0. register() now throws on versions that ship
registerHooks but predate the fix (< 22.22.3, <= 24.11.0, 25.0.0) so embedders can
fall back to the asynchronous loader instead of crashing mid-graph. supportsSyncHooks()
exposes the same check for consumers that register the hooks themselves.

Refs: nodejs/node#59929

BridgeAR added a commit to DataDog/dd-trace-js that referenced this pull request

Jun 17, 2026
…upport

module.registerHooks rejects the nullish CommonJS source the synchronous loader
returns for require()s pulled into the ESM graph (ERR_INVALID_RETURN_PROPERTY_VALUE)
until nodejs/node#59929, released in 22.22.3, 24.11.1, 25.1.0 and 26.0.0; note
24.0.0-24.11.0 and 25.0.0 ship registerHooks but predate it. Consult iitm's
supportsSyncHooks() (which owns this capability check) and bump the fork to pick
it up; unsupported versions fall back to the asynchronous loader.

Refs: nodejs/node#59929