Proposal: Moving hooks on thread

[GeoffreyBooth]

Following up #200 (comment), this is a draft proposal for option 3 from #201. This builds on #198 and assumes that that proposal would be implemented first.

Based on nodejs/node#43245.

[guybedford]

This looks like a great start, although would be nice to see a little more design around the resolve and load caller APIs here.

[ghost]

Some support for this.

My loader is an import interceptor. The main use case is full mocks in tests. On the principle "don't hide errors", I compare the given mock to the shape of original module. The attempt fails if the mock tries to add a name that doesn't exist, omit a name that does exist, or set a value for a name that's ambiguous. If the mock does any of these things then the test passes while production fails.

// util.mjs
export function frizzle () {}

// dazzle.mjs
import { frizzle, frazzle } from './util.mjs'
export class Dazzle { /* use frizzle and frazzle */ }

// test.mjs
const frizzle = sinon.stub()
const frazzle = sinon.stub() // not present in the authentic module
pose('./util.mjs', { frizzle, frazzle })
const { Dazzle } = await import('./dazzle.mjs')
// PhantomExport: Attempted to add an absent name to the module

Achieving this requires analyzing the full module graph. A naive approach would simply load each module but I'd prefer to avoid that. Where the entire module is mocked no exports can ever be called so instantiating is a waste, it may take significant time if it fronts a large graph of eg 100 modules, and there may load time modification of state that I'd like to isolate the test from.

I saw an APM rep in one of the meetings say they really need to load the authentic module so they can wrap it with tracing functions. Just want to register here that I kind of have the opposite use case. I'd specifically like to analyze all the source without evaluating it.

This hook enables exactly these things, so I'm in strong support.

[joyeecheung]

I am not sure if I understand the use case correctly but sounds like it can be addressed with the link hook proposed in #198, which is invoked after module compilation and before evaluation, at that point you have the real export names parsed by V8, but the modules are not evaluated yet so you can perform the tests there and if it throws, the graph would not be evaluated at all. That also saves you the cost of parsing the modules to get the export names by yourself, since you can piggypack on the names readily parsed by V8.

[ghost]

It would be awesome, that's definitely the most complicated piece. It's really close. I'd need the list of ambiguous names to fully replace it. Would really prefer to let the engine handle this.

[ghost]

It's seeming better and better the more I mull. I misunderstood link at first as posteval precache, giving access to the real exports but allowing last minute override. But actually it's just the shape and allows preeval override. It's perfect.

It will work without the ambiguous names, that's really just a special case of undefined.

[blikest]

Having used node:module.register for a while now for my custom hooks I see that it is marked as deprecated (https://nodejs.org/api/module.html#moduleregisterspecifier-parenturl-options), yet moving hooks on thread is still here on the agenda.

I also saw mentioned somewhere that a pending issue with threaded loaders has been their duplication in workers. This happens to be exactly why i came looking, because I have had this issue managed so far by removing the --import flag from execArgv option in Worker instantiation.

This leaves workers with the default loader though, so what I was fishing for is some way to "register" a loader on an already existing MessagePort, that I would be ready to transfer around to workers as I store the one the primary thread passes to "register". Would be as easy as worker.postMessage({loaderPort},{transferList}) -> this.on("message",({data})=>module.register(data.loaderPort)). (or perhaps module.assign and a corresponding 'distribute' hook next to 'initialize' since registration is complete in this case)

Is having such procedural control over the loader thread still in perspective with the new synchronous registerHooks, or threading will become internal even if it returns?

[GeoffreyBooth]

The idea behind moving the hooks onto the main thread is that there won't be a separate thread for them anymore.