Expose loader class to userland

[jkrems]

I recently experimented with setting up loading modules in a separate context. I found the abstraction offered by vm.SourceTextModule fairly lacking, at least for this use case. The most practical way I found to actually load & link a graph of modules was to copy and clean up the Loader and LoaderJob classes from node core.

Even after doing that there are features that cannot be easily replicated, e.g. the error decoration. And, of course, forking the loader risks that it doesn't get any bug fixes etc..

What are the groups feelings on exposing the Loader class (with a carefully chosen API surface) to userland? To be clear: This isn't about the default loader instance. It's about the utility types that can be used to properly set up module loading and linking in a separate graph.

Reference: https://github.com/jkrems/loader/blob/4fbbf053410824233d91ffb1536ce6ba88f3bb63/resource-worker/module_loader.js

[bmeck]

I'd want to audit to check that users can't mutate the default loader (including proto pollution), but exposing the type it seems fine to me as long as you don't instrument the active realm's loader. Would the idea here be to automatically set the "parent" to the current realm's loader instead of the default node loader? Will need to think a bit on that.

[GeoffreyBooth]

Would this resolve nodejs/node#49446?

I also wonder if this would lead to lots of bug reports where people think that this does allow modifying the actual instantiated loader that Node is using, similar to how lots of instrumentation tools hijack fs and http to add their tracking logic.

[bmeck]

@GeoffreyBooth Likely they are separate issues and people creating their own Loader instance (not --loader integration) would need to call out to a different resolution behavior as a fallback somehow normally.

[ljharb]

Is the issue that you want the hierarchy, or just the implementations?

Perhaps we could expose a Loader factory, so they’d do extends Loader()?

[jkrems]

I also wonder if this would lead to lots of bug reports where people think that this does allow modifying the actual instantiated loader that Node is using

Since the API would be something along the lines of new vm.ModuleLoader(), I think that risk should be fairly low. People who know to use the vm module usually don't expect that it interacts with node's internals.

Would this resolve nodejs/node#49446?

I don't think so. But it's interesting in general what if any defaults the API would have. See also Bradley's question above about parent loaders.