feat: API review

[mhmd-azeez]

This PR fixes #4 and addresses some of the points of #6

• Export createPlugin instead of ExtismPlugin.new to be more idiomatic for JS developers.
• Implement CurrentPlugin
• Sync README with Ruby SDK. I decided to include this in the PR since it depends on the new APIs
• Stream incoming wasm plugin data if possible, especially if we're in the browser
• Accept a URL for both ManifestWasmFile and ManifestWasmUrl

[mhmd-azeez]

@chrisdickinson regarding:

add async createPlugin(plugin, options): Plugin as the default export

I changed it to be a named export because as far as I know, you can't import both a named export and a default export in the same line in NodeJS:

const createPlugin, { ExtismPluginOptions } = require("../dist/node/index")

[mhmd-azeez]

@bhelx @chrisdickinson because of the way I had written the tests, I didn't realize that I hadn't implemented CurrentPlugin. It was a deep rabbit hole, but I considered these approaches:

1. Partial application of the host functions:

const options = new ExtismPluginOptions()
    .withFunction("env", "do_stuff", function (cp: CurrentPlugin, offs: bigint) {
        
    })

I couldn't get this to work because when initializing the imports, the number of parameters have to be known. And all of the partial application approaches assume the caller knows how many params the resulting function has.
2. Bind this to CurrentPlugin:

const options = new ExtismPluginOptions()
    .withFunction("env", "do_stuff", function (this: CurrentPlugin, offs: bigint) {
        
    })

This works well (note that this: CurrentPlugin is optional), the only issue is that it doesn't work with arrow functions.

I ended up going with approach 2 because I couldn't find any way to make approach 1 work

[chrisdickinson]

This is an interesting one – I missed the config builder on my first read-through, apologies!

Generally in JS we pass plain objects for options & validate on the receiving end, with a typescript interface as the receiving function signature (e.g., the RequestInit interface for the fetch function.)

Moving to a plain object would allow for the default export, too.

[mhmd-azeez]

Good point, I have turned ExtismPluginOptions into an interface, and now you can use the library like so:

// CommonJS
const { createPlugin } = require("../dist/node/index")

// ES Modules
import createPlugin from '../src/deno/mod.ts'

Note: for CJS, if I had used a default export, the user would have to write something like:

const createPlugin = require("../dist/node/index").default

Please let me know if there is a better way to go about this

[mhmd-azeez]

@chrisdickinson

Stream incoming wasm plugin data if possible, especially if we're in the browser

I have implemented this using WebAssembly.instantiateStreaming. However there are two caveats:

1. If the user specifies a hash for the wasm source, then we have to download the content anyway and WebAssembly.instantiate will be used (since we already have the data in memory)
2. WebAssembly.instantiateStreaming expects the Content-Type of the response to be application/wasm. So I have written some code to make sure the content type is always application/wasm. I have done this because we haven't given the user any way to opt-out of WebAssembly.instantiateStreaming. And it was a blocker for one of the tests that was downloading a plugin from github (this is also used in the readme as an example)

Accept a URL for both ManifestWasmFile and ManifestWasmUrl

I took a look at this too, I faced an issue with NodeJS. While the newer versions have support for fetch, on older versions we are falling back to node-fetch which doesn't support local files (file://). So I decided to leave it as as. Let me know if there is any workaround for that. One workaround would be to check the path scheme, what do you think?

[chrisdickinson]

I took a look at this too, I faced an issue with NodeJS. While the newer versions have support for fetch, on older versions we are falling back to node-fetch which doesn't support local files (file://). So I decided to leave it as as. Let me know if there is any workaround for that. One workaround would be to check the path scheme, what do you think?

It looks like v18 (which is about to enter Maintenance according to the release plan) supports fetch() without flags; v16 went into end-of-life as of this last September. Usually I've seen libraries support the LTS + current versions – it might be okay to drop the node-fetch polyfill / Node 16 support now?

[chrisdickinson]

This is looking really good! Re: the .default – yeah, I just don't see many ways around that. The good news is that it should be invisible to Babel/Typescript users thanks to the __esModuleInterop property. ESM is definitely a sore spot in Node still, though.

[chrisdickinson]

This might be outside the scope of this PR, but could we move this to another standalone module? It would make updating it programmatically a little more straightforward.

[mhmd-azeez]

i have written a little regex magic that updates it. you can run via: make update-kernel. I think it's okay for now

[mhmd-azeez]

@chrisdickinson great suggestions, I have applied them all.

1. Regarding unifying ManifestWasmUrl and ManifestWasmFile:
   We now only have ManifestWasmUrl, for node we are still falling back to fs because even the official fetch doesn't support local files. Users can also create a plugin from a string:

const plugin = await createPlugin("wasm/code.wasm");

2. Reverted node to default export too

[bhelx]

Can we write this from the user's perspective? how do we import from the npm module? If i'm using typescript how do i get the types I need?