module: refactor to use normalizeRequirableId in the CJS module loader

[RaisinTen]

BuiltinModule.normalizeRequirableId() was introduced in #47779 to fix a bug in the require function of SEAs and Snapshots, so that built-in modules with the node: scheme could be required correctly. This change makes more use of this API instead of BuiltinModule.canBeRequiredByUsers() and BuiltinModule.canBeRequiredWithoutScheme() to reduce chances of such bugs.

[nodejs-github-bot]

Review requested:

• @nodejs/loaders
• @nodejs/modules
• @nodejs/startup

[GeoffreyBooth]

And the ESM loader?

[nodejs-github-bot]

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

[RaisinTen]

@GeoffreyBooth there are still some places both in the ESM loader as well as the CJS loader where canBeRequiredByUsers and canBeRequiredWithoutScheme are being used together, so these could potentially be replaced with normalizeRequirableId:

ESM -

node/lib/internal/modules/esm/hooks.js

Lines 638 to 644 in 6fb10ca

	(builtinName) => {
	if (BuiltinModule.canBeRequiredByUsers(builtinName) &&
	BuiltinModule.canBeRequiredWithoutScheme(builtinName)) {
	return require(builtinName);
	}
	throw new ERR_INVALID_ARG_VALUE('builtinName', builtinName);
	},

CJS -

node/lib/internal/modules/cjs/loader.js

Lines 916 to 945 in 6fb10ca

	if (StringPrototypeStartsWith(request, 'node:')) {
	// Slice 'node:' prefix
	const id = StringPrototypeSlice(request, 5);
	if (!BuiltinModule.canBeRequiredByUsers(id)) {
	throw new ERR_UNKNOWN_BUILTIN_MODULE(request);
	}
	const module = loadBuiltinModule(id, request);
	return module.exports;
	}
	const filename = Module._resolveFilename(request, parent, isMain);
	const cachedModule = Module._cache[filename];
	if (cachedModule !== undefined) {
	updateChildren(parent, cachedModule, true);
	if (!cachedModule.loaded) {
	const parseCachedModule = cjsParseCache.get(cachedModule);
	if (!parseCachedModule || parseCachedModule.loaded)
	return getExportsForCircularRequire(cachedModule);
	parseCachedModule.loaded = true;
	} else {
	return cachedModule.exports;
	}
	}
	if (BuiltinModule.canBeRequiredWithoutScheme(filename)) {
	const mod = loadBuiltinModule(filename, request);
	return mod.exports;
	}

but I wasn't sure how that could be done, so I kept this PR isolated to only these changes. If you have any suggestions on how that could be done, I'd be happy to implement that.

[nodejs-github-bot]

Landed in 7fe4745