module: refator ESM loader for adding future synchronous hooks

[joyeecheung]

This lays the foundation for supporting synchronous hooks proposed in nodejs/loaders#198 for ESM.

• Corrects and adds several JSDoc comments for internal functions of the ESM loader, as well as explaining how require() for import CJS work in the special resolve/load paths. This doesn't consolidate it with import in require(esm) yet due to caching differences, which is left as a TODO.
• The moduleProvider passed into ModuleJob is replaced as moduleOrModulePromise, we call the translators directly in the ESM loader and verify it right after loading for clarity.
• Reuse a few refactored out helpers for require(esm) in getModuleJobForRequire().

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[joyeecheung]

The assertions for translators could be removed if we just switch to compiled = new WebAssembly.Module(source) in the wasm translator, though I am not sure if anyone still wants to keep the imported wasm asynchronously compiled (import 'wasm' is not really all that useful after https://github.com/WebAssembly/esm-integration/blob/main/proposals/esm-integration/README.md is shipped by V8 anyway)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/61931/

[codecov]

Codecov Report

Attention: Patch coverage is 98.86364% with 3 lines in your changes missing coverage. Please review.

Project coverage is 88.07%. Comparing base (c1afd2c) to head (5ecaf9c).
Report is 516 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/modules/esm/loader.js	98.57%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54769      +/-   ##
==========================================
+ Coverage   88.06%   88.07%   +0.01%     
==========================================
  Files         652      652              
  Lines      183545   183653     +108     
  Branches    35866    35860       -6     
==========================================
+ Hits       161639   161758     +119     
+ Misses      15150    15140      -10     
+ Partials     6756     6755       -1     

Files with missing lines	Coverage Δ
lib/internal/modules/esm/module_job.js	100.00% <100.00%> (ø)
lib/internal/modules/esm/translators.js	92.45% <100.00%> (+0.75%)	⬆️
lib/internal/modules/esm/loader.js	98.34% <98.57%> (+0.57%)	⬆️

... and 33 files with indirect coverage changes

[RedYetiDev]

nit: refator->refactor in commit

[mcollina]

lgtm

[jasnell]

Typo in the commit message but otherwise LGTM

[joyeecheung]

Fixed the typo commit message - also I noticed there are a few typos in the return type of the JSDocs of getModuleJobForImport and getModuleJobForRequireInImportedCJS (deleted the wrong Promise< and > after copy-pasting somehow) so I fixed that as well.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62010/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62471/

[joyeecheung]

Rebased to resolve the merge conflicts.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62481/

[joyeecheung]

@mcollina @Qard @jasnell this needs a re-approval after the rebase. Can you take a look again? Thanks

[nodejs-github-bot]

Landed in 3ac5b49

[laverdet]

@joyeecheung was it intentional to remove responseURL?

$ cat loader.mjs 
export async function load(urlString, context, nextLoad) {
	const result = await nextLoad(urlString, context);
	return {
		...result,
		responseURL: 'test://',
	};
}

$ cat register.mjs 
import { register } from 'node:module';

register('./loader.mjs', {
	parentURL: import.meta.url,
});

$ cat main.mjs 
console.log(import.meta.url);

$ node -v
v22.4.1

$ node --import ./register.mjs main.mjs                                                         
test://

$ [...]
$ node -v
v22.10.0

$ node --import ./register.mjs main.mjs  
file:///Users/marcel/tmp/main.mjs

This feature was useful because it allows you to control import.meta.url. I used it pretty significantly to hide implementation details in my hot reloader dynohot. The resolve hook would resolve something like "hot:module?url=file:///file.mjs" and the load hook knows how to unwrap that parameter and load the underlying resource. We don't want to affect the result of import.meta at runtime though.

[joyeecheung]

was it intentional to remove responseURL?

It was not intended to break any public API, though it seems responseURL was never properly documented nor tested. Do you want to submit a PR to add it back with proper tests and documentation?