search

wasmerio / wasmer-pack

MIT License
49 stars 5 forks source link

Pass in options for constructing a Wasi instead of forcing callers to instantiate it manually #94

Closed syrusakbary closed 1 year ago

syrusakbary commented 1 year ago

Fixed wasi Options

syrusakbary commented 1 year ago

Note: we should probably have tests for the standalone WASI commands as well

Michael-F-Bryan commented 1 year ago

@syrusakbary, are you still working on this PR?

As mentioned on slack...

If we still want to initialize the WASI object inside the load function instead of allowing people to pass in their own instance, then we'd need to propagate that change through to the other templates and make sure integration tests pass in CI again. In particular...

  • Apply the same change to the bindings.index.js template (here)
  • Update the WasiLoadOptions type in bindings.index.d.ts and the RunOptions type in top-level.index.d.ts so the wasi field contains the object passed to new WASI(...) (here and here)
  • Update the snapshot tests and make sure the generated code looks correct (cargo insta review)
  • Update the JavaScript WASI integration test to reflect the new API (here)

That said, I'm not convinced that we need this.

Currently, people can initialize their own WASI instance and pass it in if they want (e.g. so they can write to stdin and read stdout/stderr while the command is running), otherwise we'll instantiate a default instance for them.

The new WASI() constructor doesn't require a WebAssembly.Instance because that's provided when we call wasi.instantiate().

github-actions[bot] commented 1 year ago

⏱ Workflow Timer ⏱

🥳 The run time for "Continuous Integration" has improved a lot by 1min 46s (37.99%) 🥳

The current run time is 2mins 53s while master took 4mins 40s.