Add Brotli WASM module imports to package.json #40
Conversation
|
|
|
Nice work, thanks @oligamiq! I'm away this week I'm afraid but I'll have a proper look at this and test everything next week and get back to you. |
package.json
Outdated
| "./web/bg.wasm": { | ||
| "types": "./pkg/brotli_wasm_bg.wasm.d.ts", | ||
| "import": "./pkg/brotli_wasm_bg.wasm", | ||
| "default": "./pkg/brotli_wasm_bg.wasm" |
There was a problem hiding this comment.
This doesn't work as /pkg/ doesn't exist.
| @@ -12,6 +12,16 @@ | |||
| "browser": "./index.browser.js", | |||
| "require": "./index.node.js", | |||
| "default": "./index.web.js" | |||
| }, | |||
| "./web": { | |||
There was a problem hiding this comment.
Can you explain more about why we need a separate /web path? The brotli_wasm.js file is already imported in index.web.js - can't we just export the methods directly from there? That would also allow us to also provide a single API to do this that worked for all targets.
I do agree there's improvements to make here, but I'm not sure that just adding more exports is correct. It really seems like we should be able to provide a small set of exports, correctly automatically select the right one for the environment, and export data in the formats required.
If that needs to be "JS export + raw WASM" export for environments that need to manage wasm import & init manually, that makes sense, but I'm not clear why we'd need "JS export + web JS export + web raw WASM export", which seems like what we're doing here.
#31