module: fix sync resolve hooks for require with node: prefixes

[joyeecheung]

Previously, when require()-ing builtins with the node: prefix, the sync resolve hooks were not properly invoked, and load hooks could not override the builtin's format. This fixes the handling and enables redirecting prefixed built-ins to on-disk files and overriding them with other module types via hooks.

Fixes: #60005

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.52%. Comparing base (4f24aff) to head (bac0a34).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61088      +/-   ##
==========================================
- Coverage   88.53%   88.52%   -0.01%     
==========================================
  Files         703      703              
  Lines      208546   208589      +43     
  Branches    40217    40236      +19     
==========================================
+ Hits       184634   184658      +24     
- Misses      15926    15948      +22     
+ Partials     7986     7983       -3     

Files with missing lines	Coverage Δ
lib/internal/modules/cjs/loader.js	98.11% <100.00%> (+0.13%)	⬆️

... and 44 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aduh95]

Suggested change

	return { resultFromHook }; // Format has been overridden, return result for the caller to continue loading.
	return { __proto__: null, resultFromHook }; // Format has been overridden, return result for the caller to continue loading.

or

Suggested change

	return { resultFromHook }; // Format has been overridden, return result for the caller to continue loading.
	return { resultFromHook, builtinExports: undefined }; // Format has been overridden, return result for the caller to continue loading.

otherwise we'll be hitting potential user-defined getters

[aduh95]

ditto

[ljharb]

i'm confused; i thought the prefix was meant to prevent any kind of user intervention in accessing the core module, with require.extensions OR loaders - does this PR change that?

[joyeecheung]

i thought the prefix was meant to prevent any kind of user intervention in accessing the core module, with require.extensions OR loaders - does this PR change that?

module.register already allows customizations of modules prefixed with node: (as far back as 20)

echo "require('node:tls')" > test.cjs
node --import ./test/fixtures/module-hooks/register-logger-async-hooks.mjs test.cjs

resolve file:///Users/joyee/projects/node/test.cjs from undefined
load file:///Users/joyee/projects/node/test.cjs
resolve node:tls from file:///Users/joyee/projects/node/test.cjs
load node:tls

[ljharb]

wow, ok, that's unfortunate - that was one of the primary justifications for adding the prefix in the first place, the performance and reliability benefit of not being able to intercept it.