fix(node): expose module format in registerHooks

[Tiancheng-Xu]

Summary

• Fixes Module.registerHooks never populates format, so hooks cannot tell CommonJS from ESM #36841.
• Expose the resolved module format to synchronous and ESM Module.registerHooks load hooks.
• Add a regression spec covering CommonJS and ESM context.format and nextLoad().format.

Tests

• rustup run 1.95.0 rustfmt --check cli/module_loader.rs ext/node/ops/module_hooks.rs
• git diff --check
• node --check ext/node/polyfills/01_require.js
• node --check tests/specs/node/module_register_hooks/format.mjs

Not run locally: the full ./x fmt, ./x lint, and runtime spec command. The local checkout could not be completed because GitHub's promisor fetch repeatedly failed at the network layer; the PR CI should provide the complete repository validation.

AI assistance

AI tools assisted with implementation and review. I verified the final changes and the validation results reported above.

[Copilot]

🟡 Changes recommended

There is at least one remaining incompatible Rust call site for the updated push_load signature and the CommonJS hook path hardcodes context.format in a way that can produce incorrect context.format/nextLoad().format for non-CJS targets.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR updates Deno’s Node.js module.registerHooks() implementation so load hooks can observe the resolved module format via context.format and the value returned from nextLoad().format, with a new regression spec covering both CommonJS and ESM cases.

Changes:

• Plumb a format field from the Rust loader through op_module_hooks_poll_load() into the ESM load-hook loop.
• Set context.format for the CommonJS Module.prototype.load hook execution path.
• Add a spec test validating context.format and nextLoad().format for both .cjs and .mjs.

File summaries

File	Description
tests/specs/node/module_register_hooks/format.out	Expected output for the new registerHooks format regression test.
tests/specs/node/module_register_hooks/format.mjs	New test asserting context.format and nextLoad().format for CJS + ESM loads.
tests/specs/node/module_register_hooks/format-esm.mjs	ESM fixture module for the new spec.
tests/specs/node/module_register_hooks/format-cjs.cjs	CJS fixture module for the new spec.
tests/specs/node/module_register_hooks/test.jsonc	Registers the new load_hook_format spec case.
ext/node/polyfills/01_require.js	Passes a format into CommonJS and ESM load-hook contexts.
ext/node/ops/module_hooks.rs	Extends pending load requests/op return shape to include format.
cli/module_loader.rs	Computes and forwards a load format into the hook registry for ESM bridging.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Tiancheng-Xu]

Addressed the review feedback in follow-up commit 7e8727ec:

• Updated the standalone denort loader call site to pass the resolved format.
• Moved the RequestedModuleType to format mapping into shared deno_lib::loader, reused by both loaders.
• Made the CommonJS hook path derive format from the filename and loadMaybeCjs behavior, including JSON, ESM extensions, CJS extensions, WASM, and package-type-sensitive JS/TS files.
• Extended the regression spec to cover JSON and the ESM path.

Fresh local checks passed: targeted rustfmt, git diff --check, and Node syntax checks for the modified JavaScript test/runtime files. Full ./x validation remains delegated to PR CI because the local checkout cannot complete its promisor fetch.

[Tiancheng-Xu]

Follow-up commit 4c4ce985 addresses the CI findings: RequestedModuleType::Other now returns an owned String, and the new format regression fixtures are dprint-formatted. Local targeted rustfmt, git diff --check, and Node syntax checks pass. The local checkout still lacks the complete Cargo workspace and Deno formatter binary, so full cargo check/deno fmt remain delegated to CI.

[isaacs]

@Tiancheng-Xu Thanks for taking this on!

Does this also cover format = 'json' for .json files? I ask because this also came up as a shortcoming recently that we had to work around in our module instrumentation hooks. (I think the answer is "yes", looking at the diff, but I'm unfamiliar with the codebase and just wanted to make sure.)

[Tiancheng-Xu]

Yes. For synchronous require() hooks, getLoadHookFormat() sets context.format to "json" for .json files. The regression spec requires format.json and checks both context.format and result.format. The async loader also maps RequestedModuleType::Json to "json". The current head has green checks.

[CLAassistant]

All committers have signed the CLA.