esm: Make custom loaders specific

[viktor-ku]

Hello, folks!

The whole story with ESM HTTPS Modules got me to a point where I am about to write custom loader for node. And I have one little issue with it (but maybe I am wrong?). I need to write a loader that will handle the whole import process of all types (builting, external .js, etc.)?

Why can't I write one specific loader that will extend node.js import policy rather than replace it?

If I could I'd like to write my specific loader like this:

node-https-loader.js:

export function test(specifier): boolean {
    return specifier.startsWith('https:');
}

export function resolve(specifier, parentModule, defaultResolve) {
    // Will be called only if test() above returns `true`
}

In addition to resolve function we have test also. With the latter we can test the specifier to see if it can be resolved with the given loader.

To optimize loading time any custom loaders could be tested after builtin modules, relative files, absolute files, checks.

This approach could give developers flexibility to use multiple custom loaders (e.g. yml and https at the same time)

I know that it breaks existing loaders idea but maybe we can do something about it?

If you like this idea I can invest my time to create PR

Thanks,
Viktor Ku

[bmeck]

Hiya @viktor-ku ,

The Modules WG is currently reviewing things and Loader are subject a potentially large refactor to how they work. I'm going to try and make a meeting explicitly on loaders nodejs/modules#135 and would love if you have any input you can offer.

CC: @nodejs/modules

I will state that in #18914 I designed it not around a separate test predicate, but in making loaders explicitly call to parent loaders. This allows loaders to not just accept/reject loading but also manipulate things as they pass through the loader chain and back up to the modules implementation.

[devsnek]

@viktor-ku you can just do

export async function resolve(specifier, parentModule, defaultResolve) {
  if (!specifier.startsWith('https:')) {
    return defaultResolve(specifier, parentModule);
  }

  // handle your own stuff here
}

[viktor-ku]

@devsnek Yeah I can.

But it will force everyone who's writing custom loaders to do this check at the top or elsewhere.

@bmeck I think it's better to call loader only if we know that we need to resolve an import of some kind

[demurgos]

I think that adding a separate test function would be redundant: you can still do the test (and more) in resolve.
I don't think that many people will write custom loaders, but I think that minimizing the number of functions and avoiding overlap between them is more important to get a simpler design.

A possible benefit for having the test defined outside of resolve would be static analysis. test could be an object providing a declarative way to check if a loader is applicable. It would make the role of both test and resolve easier to define: "use test if you can describe declaratively when to use the loader, fall back to resolve if you need something more powerful". If the loaders are mostly checking against extensions, protocols or subdirectories it may help. Still, I doubt this kind of analysis may be useful: even if you could (sometime) statically determine if a loader is used or not, ultimately you cannot analyze what the loader will do because it's code. You'll have configure your tools anyway to understand what's going on if you want static analysis.

In short, a test function would overlap with resolve and increase complexity and going with a declarative test API would not be enough for static analysis. I think it's better to keep a single resolve function.

[viktor-ku]

@demurgos Yeah. I guess my idea is not great so I am closing this issue.

Thanks everyone ❤️