Looks interesting. Two questions:

1. If I understand correctly, the mocking test code doesn't really need setImportMetaCallback, and it could do without. While this capability could be interesting, I'm not sure why it's needed (you said something about an http loader?).

2. While the port to communicate is interesting, communicating via global variables (while admittedly ugly), is still easier to work with, and synchronous in nature. So why add this capability? My guess is that it's for a future where the loader will be in a worker, which is fine. But in that case, the messageport communication won't help deal with the problem that the "client" (the mock function) needs to pass functions (the mock function) to the loader, which cannot be done (AFAIK) using messages (because what's passed needs to be serialized).

If I understand correctly, the mocking test code doesn't really need setImportMetaCallback, and it could do without. While this capability could be interesting, I'm not sure why it's needed (you said something about an http loader?

setImportMetaCallback in the test is being used to create a 1st class channel from the preload code and the module instances. You can see in the generateModule output that it is reading the references off of import.meta.mock.

Per the HTTPS loader PR the actual behavior of import.meta.url on the web differs from files and recreating HTTPS semantics is not currently possible with a loader without manual prefixing of modules with things like import.meta.url = ... new value here ...;\n. See https://github.com/nodejs/node/pull/36328/files#diff-b1de5e9ce1e9b411001c63d11a5092dbba32e1586269a20dc11408375eb2f16cR135-R137

While the port to communicate is interesting, communicating via global variables (while admittedly ugly), is still easier to work with, and synchronous in nature. So why add this capability? My guess is that it's for a future where the loader will be in a worker, which is fine. But in that case, the messageport communication won't help deal with the problem that the "client" (the mock function) needs to pass functions (the mock function) to the loader, which cannot be done (AFAIK) using messages (because what's passed needs to be serialized).

I actually think the problem here is in part my using a MessagePort which wants to asynchronously dispatch events. I generally think we need to move loaders off thread and have faced pushback in the past due to startup penalty of doing so; however, I wrote this with off-thread in mind. Using globals is easier! However, this avoids leaking globals and allows communication to be done in a way that cannot be tampered with or prevented by user code between Loader <-> Preload Code <-> Module Instances. Definitely want to make the API work better so might have to use a different communications API than MessagePort for now. Currently the functions are never passed to the loader via a 1st class reference, they are only passed as a list of exports and the global preload code is managing the first class references. All communication with the loader is done via message ports.

setImportMetaCallback in the test is being used to create a 1st class channel from the preload code and the module instances. You can see in the generateModule output that it is reading the references off of import.meta.mock.

Yes, but it has enough information to just use mockedModules by itself, without recourse to using meta.mock, no?

recreating HTTPS semantics is not currently possible with a loader without manual prefixing of modules with things like import.meta.url = ...

🙏

Currently the functions are never passed to the loader via a 1st class reference, they are only passed as a list of exports and the global preload code is managing the first class references. All communication with the loader is done via message ports.

Of course you're right. And to think I wrote something like this... (well, it was a while ago 😊). So, OK, removing my objection on thread-separated loaders.

now that consolidation PR is landed, rebasing, looks like we need a slight alteration to handle multiple nesting importMetaCallbacks

removing draft status since there aren't any pending design comments and hoping to land early next week

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

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

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

Reading through CI reliability and the failed CI runs, I think the cause for the failures isn't this PR.

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

Landed in 1998e83