Declare ModuleWrap::ResolveModuleCallback as public instead of private

[caoccao]

What is the problem this feature will solve?

An embedder is not able to fallback to the built-in Node.js ModuleWrap::ResolveModuleCallback if that embedder calls InstantiateModule with its own ResolveModuleCallback.

For instance:

import * as test from 'https://test.com/test.mjs';
import * as http from 'node:http';

With the built-in ModuleWrap::ResolveModuleCallback, line 1 will fail with:

Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data are supported by the default ESM loader. Received protocol 'https:'

With the embedder's ResolveModuleCallback, line 2 will fail with:

Error: 'node:http' is not found.

What is the feature you are proposing to solve the problem?

The embedder is able to fallback to the built-in Node.js ModuleWrap::ResolveModuleCallback if it fails to find a suitable external module.

The change is very simple: to declare ModuleWrap::ResolveModuleCallback as public instead of private so that the embedder's ResolveModuleCallback is able to reference and call the built-in Node.js ModuleWrap::ResolveModuleCallback.

I'm using v18.12.1. It's appreciated that the change is applied to the latest v18.

What alternatives have you considered?

No.

[aduh95]

Have you considered using --experimental-network-imports instead? Also there's https://nodejs.org/api/esm.html#loaders which is a more fitted way to achieve what you are trying to do.

[caoccao]

Have you considered using --experimental-network-imports instead? Also there's https://nodejs.org/api/esm.html#loaders which is a more fitted way to achieve what you are trying to do.

Thank you for the info. Actually, that was just an easy sample using https. The actual ResolveModuleCallback could load modules from database where the location could be the primary key of a certain row. In this case, the Node.js facility for sure doesn't fit in. Also, in some scenarios, the ResolveModuleCallback could only allow authorized network locations. There are a lot of customizations that could only be done in a custom ResolveModuleCallback, however, by doing so, we lose the ability of loading the built-in modules. Hope this makes sense.

[bnoordhuis]

I suspect plain exposing ModuleWrap::ResolveModuleCallback() isn't a viable long-term solution because of all the implicit state involved with module loading. It's just one piece of the puzzle.

The loaders API that @aduh95 linked to is probably the way to go for now.

[caoccao]

I suspect plain exposing ModuleWrap::ResolveModuleCallback() isn't a viable long-term solution because of all the implicit state involved with module loading. It's just one piece of the puzzle.

The loaders API that @aduh95 linked to is probably the way to go for now.

Thank you for the confirmation. I'm afraid that doesn't serve the embedder I'm working on, because the custom ResolveModuleCallback is implemented by C++ and the module resolution process is restricted to the guest scripts in my use cases.

Here are 2 options I'm thinking of for your reference.

1. Enhance ModuleWrap to accept a chain of ResolveModuleCallback.
2. I'll create a fork with ModuleWrap::ResolveModuleCallback() declared as public. Of course, I won't merge that change back.

[bnoordhuis]

(2) isn't a bad option for now because loaders are experimental and likely to change.

(1) could work but the devil is in the details and here too any such API may need to change again in the future.

[caoccao]

(2) isn't a bad option for now because loaders are experimental and likely to change.

(1) could work but the devil is in the details and here too any such API may need to change again in the future.

Fair enough. Thank you for sharing the insights. I'll take option 2.

[caoccao]

Update

It's hard to fall back to ModuleWrap::ResolveModuleCallback because it internally tries to get the dependent module wrap which is absent.

  ModuleWrap* dependent = GetFromModule(env, referrer);
  if (dependent == nullptr) {
    THROW_ERR_VM_MODULE_LINK_FAILURE(
        env, "request for '%s' is from invalid module", specifier_std);
    return MaybeLocal<Module>();
  }

It seems Node.js is only friendly with pure JS modules that will be registered as dependent module wraps. Other C++ modules don't seem to be working that way.

[caoccao]

Update: I created synthetic modules calling require internally and this issue was resolved. There's no need to interact with the ModuleWrap::ResolveModuleCallback any more.