module: handle null source from async loader hooks in sync hooks by joyeecheung · Pull Request #59929 · nodejs/node
added
lib / src
labels
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.
aduh95 pushed a commit that referenced this pull request
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
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
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
* 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
* 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
* 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
* 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
mentioned this pull request
joyeecheung added a commit to joyeecheung/node that referenced this pull request
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
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
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
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
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
…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