do not stub imports that are part of the target world

[karthik2804]

This PR makes sure that if an import is present in the target world, it does not get stubbed out. I went with a different approach than what was suggested in the issue comment as I noticed the module only contained the wasi imports if they are a part of the target world and was simpler than having to prepare the wit. Let me know if I am missing something.

fixes #105

[guybedford]

There are two ways that imports can be included in the output binary:

1. Imports are pulled in by the engine itself
2. World imports being converted into imports by the splicing code

The stubbing code here acts on the binary that is the union of the above. Therefore the stubbing code cannot know for a given import if it was used by the engine or if it is used by the target world.

Therefore saying that features only stub out when the final binary doesn't use them excludes engine imports pulled in that we do want to stub. For example libc builtins that get pulled in that are unused.

This means that this PR risks outputting a binary with extra engine imports that were supposed to be stubbed. I think you will find that the "pure component" case of no features is thus no longer working with this PR, and that pure components have imports again.

This list of these engine bulitins that should always be stubbed can be determined by building a binary against any non-WASI world. The problem is that this list should be excluded against any WASI imports of the world being targetted. And thus we need to parse the target world to do the exclusion as I described in #105 (comment).

I'm open to counter arguments if you can prove any of the above otherwise.

[guybedford]

I just checked this PR and can confirm the pure component case is still okay, but the edge cases are still not quite consistent I think. To dig into the nuances:

• Preview1 builtins are always stubbed (stub_preview1())
• It just so happens that filesystem, CLI and sockets are not at all used by StarlingMonkey builtins, so as a result we will never have this ambiguous union case with world imports. If StarlingMonkey builtins ever bind to these subsystems in future we will get these ambiguity cases.
• This then leaves random, CLI, clocks and HTTP as cases where having the feature disabled will still stub out the world. This results in the case where you must set features: ['random'] if you are binding to a world that uses random.

So the consequence is that for example having HTTP disabled by default will result in not being able to output to an HTTP world, unless you explicitly enable the HTTP feature as well.

Right now, features is something that relates to the engine itself, and gets unioned with the target world. It seems like it would be odd to have to align the features usage with the exact target world usage.

For example consider if we add support for websockets in future. This is a "websockets" feature. But whether websockets is enabled or not shouldn't affect weather I can target a world that uses websockets.

[karthik2804]

I think the above tracks in my head. I am happy to take a stab at using an approach similar to splicing if that is the recommended approach.

[guybedford]

Thanks for being open to that! It's just a few lines of code. An example of walking the exports of a WIT world is here - https://github.com/bytecodealliance/ComponentizeJS/blob/main/crates/spidermonkey-embedding-splicer/src/lib.rs#L143. Where instead of exports, imports can be used. Those strings can then be directly used as the exclude strings when performing stubbing.

[karthik2804]

I have updated the code to resolve the target world imports and use that to conditionally stub imports. I wanted to check if this is the desired approach. If this is on the right approach I will rebase the PR and clean up the unwraps.

[guybedford]

Amazing, thank you for putting this together, LGTM.

[guybedford]

Thinking about this some more: I suppose an alternative here would just be to stub the WASI imports before doing the splicing, ie before the engine gets passed in here - https://github.com/bytecodealliance/ComponentizeJS/blob/main/src/componentize.js#L52.

Then any world imports would be fully additive without needing to go through the world exclusion process.

This also seems like it would work better with composition workflows where right now as soon as something is used it is no longer stubbed.

Sorry for being slow on this one, took me a minute to think it through.

[karthik2804]

No worries. I will take a stab at updating it to the above approach tomorrow.

On another note, Working on this also made me realize we can potentially solve #104 similarly. We could use @babel/parser and @babel/traverse to parse the source file and stub any of the imports not in use by the guest code but part of the target world.

[guybedford]

Definitely share in more detail what you're thinking about the stubbing in that issue. Note that we already use this lexer to analyze imports and exports in https://github.com/bytecodealliance/ComponentizeJS/blob/main/src/componentize.js#L124.

[karthik2804]

Is the suggested idea of calling stubWasi before spliceBindings something like the following?

  const features = []; // Intionally  leaving out the part of checking for enabled features.
  const stubbedEngine = stubWasi(await readFile(engine), features);

  let { wasm, jsBindings, importWrappers, exports, imports } = spliceBindings(
    sourceName,
    stubbedEngine,
    witWorld,
    maybeWindowsPath(witPath),
    worldName,
    false
  );

When using that to build the example I get the following,

Error: calling the Reactor initialization function

Caused by:
    0: error while executing at wasm backtrace:
           0: 0x7fbf - <unknown>!<wasm function 20>
           1: 0x76b206 - <unknown>!<wasm function 11512>
    1: wasm trap: wasm `unreachable` instruction executed
file:///Users/karthik_ganeshram/Work/wasm-tools-contrib/ComponentizeJS/src/componentize.js:219
    throw new Error(err);
          ^

Error: Failed to initialize the compiled Wasm binary with Wizer:
Wizering failed to complete
    at componentize (file:///Users/karthik_ganeshram/Work/wasm-tools-contrib/ComponentizeJS/src/componentize.js:219:11)
    at async file:///Users/karthik_ganeshram/Work/wasm-tools-contrib/ComponentizeJS/example/componentize.js:7:23

Node.js v21.7.3

I will look into it more.

[guybedford]

Right, stubbing can't happen before Wizering. And there is some trickiness around environment variables and FS access here, where those subsystems would need to be specifically stubbed as Wizer stubs. Difficult to come up with a consistent model here! Perhaps the approach you implemented here is in fact a good one for now.

I can review and land if you like in the mean time, before we explore alternatives further.

[karthik2804]

It would be great to review and land this, it would help with the usage of extension libraries in JS to provide node APIs until we get them directly from starlingmonkey. I can fix the above suggestion and also remove the unwraps in a bit.

[karthik2804]

This should now be ready for a review.