node icon indicating copy to clipboard operation
node copied to clipboard

esm loaders: getBuiltin() does not work with node:test

Open cjihrig opened this issue 2 years ago • 2 comments

Version

latest main

Platform

all

Subsystem

esm, loaders

What steps will reproduce the bug?

Create an ESM loader, and in the globalPreload() hook, use getBuiltin() to access node:test (test without the node: scheme also does not work).

How often does it reproduce? Is there a required condition?

Always reproduces.

What is the expected behavior? Why is that the expected behavior?

node:test can be used with getBuiltin()

What do you see instead?

TypeError [ERR_INVALID_ARG_VALUE]: The argument 'builtinName' is invalid. Received 'node:test'

Additional information

I imagine this is related to the use of BuiltinModule.canBeRequiredWithoutScheme(builtinName) in getBuiltin().

My use case is to attach the message port to the node:test module. For now, I have worked around this by attaching the port to globalThis instead. Once https://github.com/nodejs/loaders/issues/147 is resolved, I won't need to do this, but I wanted to raise the issue in case the loaders team thinks it's important.

cjihrig avatar Jun 21 '23 20:06 cjihrig

I assume that it would probably be trivial to add support for the node: prefix to getBuiltin(), assuming that fixes this, if you want to spend the time. But yeah, the whole sloppy-mode script that seems like CommonJS but isn’t really . . . this is what we’re planning on removing, because of issues like this.

cc @nodejs/loaders

GeoffreyBooth avatar Jun 22 '23 05:06 GeoffreyBooth

I was just looking at that util a couple days ago. It reads/validates from a list; test is probably just missing from that list. I don't know why it wouldn't be included—oversight or deliberate decision.

I expect register (including the message stuff) to land around mid-July.

JakobJingleheimer avatar Jun 22 '23 08:06 JakobJingleheimer