module.registerHooks() tracking issue

[joyeecheung]

• Support node:builtin?param=val & figure out how to make search params work with CJS cache
  • Some clues can be found in modules: fix sync hook short-circuit in require() in imported CJS #62920 (which smooths it for internal test mocking)
• Symbol.dispose integration (requested in implement module.registerHooks() to run synchronous module customization hooks in thread #55698 (comment))
• Initial proposal: Proposal for a simple, universal module loader hooks API to replace require() monkey-patching #52219
• Loaders design proposal: doc: add synchronous hooks proposal loaders#198
• Initial implementation: implement module.registerHooks() to run synchronous module customization hooks in thread #55698
• Make the internal resolution and loading paths synchronous, to reduce the chance of races esm: use sync loading/resolving on non-loader-hook thread #60380 so that module.register() can be an helper built on top of module.registerHooks(), and internally we only have one set of hooking points to take care of we can't really polyfill module.register with it if we want to keep the internals to ourselves, oh well
• Handle with the nullish sources of CommonJS caused by module.register() quirk during interop: module: handle null source from async loader hooks in sync hooks #59929
  • For ESM, there's currently no way for Node.js to hook into the evaluation of child modules, will need to discuss with V8 about it (issue). Allowing mutation of the exports would require a spec change so out of scope for module.registerHooks() or Node.js in general
• Reordering of the documentation when module.registerHooks() is battle tested enough and should be preferred over module.register to avoid various caveats module.registerHooks() tracking issue #56241
• Support of unknown extensions (learned about it when I was initially proposing Proposal for a simple, universal module loader hooks API to replace require() monkey-patching #52219): this is already fully possible with the resolve hook?

Moved to #62720

• Implement link hook for ESM, as proposed in doc: add synchronous hooks proposal loaders#198 to make use cases like import-in-the-middle less hacky Proposal: new vm module primitives & loader API for ESM customization #62720
• startGraph hook proposed in Proposal: Moving hooks on thread loaders#205
• Implement evaluation hook for at least CJS to help use cases like require-in-the-middle, which was supposed to be based on doc: add synchronous hooks proposal loaders#198 but we had some discussions over the collaboration summit/NodeConf EU about changing the hook to be "wrapping around evaluation", not "after evaluation": WIP

Nice to have:

• Advocating it to popular npm packages doing CJS monkey-patching to reduce the overall dependency of CJS loader internals in the ecosystem
• Create a polyfill of module.register() built on top of module.registerHooks(): WIP in https://github.com/joyeecheung/module-register-ponyfill

[joyeecheung]

cc @nodejs/loaders

[jsumners-nr]

FYI https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/adding-sub-issues

[legendecas]

Off-topic: the new sub-issue feature is not enabled for the nodejs org yet: nodejs/admin#921

[joyeecheung]

I have a WIP for the evaluate hook which already works for a mock of require-in-the-middle. Pending on resolution about whether/how ESM can play into it. Opened an issue in Chromium to discuss about ESM evaluation hook: https://issues.chromium.org/u/1/issues/384413088 (mutability of exports would probably be out of scope of V8 as that's mandated by the spec, but at least we can address the use case where the hook does not need to patch and simply wants to observe).

[timokoessler]

Thanks for your great work! Are there any plans to officially support modifying the exports of builtins (to instrument them)?
One workaround we are exploring is to return commonjs code for builtins. In this code we import, modify and re-export the original builtin module. This does not seem to be bulletproof as Node.js e.g. adds a module.exports property to the exports when importing in ESM. Also returning code with the module format does not work because Node.js tries to read the non-existing package.json of the builtin. We want to use the new hook system to add ESM support to AikidoSec/firewall-node.

[joyeecheung]

Are there any plans to officially support modifying the exports of builtins (to instrument them)?

The current priority is mostly testing out module.registerHooks() and making it an equivalent replacement for module.register(), and then re-implementing module.register() on top of it. I don't think there is active work being put into the use case of modifying builtins, though volunteers are welcomed to drive it. If it requires a new hook, it probably deserves some thoughts into the design considering how caching and ESM namespace detection complicates things. An approach based on the load hook may be easier to implement off the top of my head.