Open sffc opened 1 year ago
The workaround for this is to edit diplomat-wasm.mjs and change typeof fetch
to typeof window
. Not sure if this is the best solution long term.
Do we still need two separate code paths? If node has fetch
now, why not use it?
It seems that fetch
doesn't play nice with filesystem URLs?
What if we special case to the node file api for filesystem URLs (and if you try that on the web you get an error anyway)
That way we don't actually need to detect Node vs not, we can just load URLs the correct way, and on systems where file URLs can't be loaded we can show an error.
@lucacasonato is working on the long term solution in spec world, but in the mean time, he says we have two options:
process
variableSo I attempted to use the fetch
solution but Node.js 18 fetch
has a really bad bug (the one @robertbastian linked above). I temporarily worked around the bug by calling delete globalThis.fetch
so that Node.js uses the fs
code path, but this is basically the same as sniffing via the process
variable (forcing Node.js to use a different code path).
I wanted to put one more possible solution on the table. Both Node.js and Webpack support WASM ESM as an experimental feature. We could just use WASM ESM and ask anyone using the icu4x library to use a runtime/bundler that supports it.
Not opposed to that, sounds interesting
@zbraniecki Can you elaborate on whether your client would be able to use WASM ESM?
wasm-bindgen supports a flag that appears to switch between methods of loading:
https://rustwasm.github.io/docs/wasm-bindgen/reference/deployment.html#bundlers
The implementation is in this file:
https://github.com/rustwasm/wasm-bindgen/blob/main/crates/cli-support/src/js/mod.rs
It appears to support the following types of wasm loading:
fs.readFileSync
and import.meta.url
fs.readFileSync
and __dirname
Deno.readFile
and import.meta.url
fetch
and new URL(document.currentScript.src, location.href)
fetch
and import.meta.url
It also supports ESM but it's not clear to me that it supports WASM ESM or just ESM for the JS exports it makes.
Yo, any updates on this?
For tests we were able to get around this by including a test.setup file with:
delete (global as { fetch?: unknown }).fetch;
Side note, I did copy paste the draft code in #498 and was able to remove the test setup file!
Currently most work in the JS backend is going into the JS2 backend. We're not using JS (or JS2) right now so there's no effort into fixing this kind of bug, we'd welcome patches to fix this. I'm not familiar enough with the Node stdlib to fix this without doing some research.
My current code, not checked-in yet, with feedback from @nicolo-ribaudo, is
if (globalThis.process?.getBuiltinModule) {
// Node (>=22)
const fs = globalThis.process.getBuiltinModule('fs');
const wasmFile = new Uint8Array(fs.readFileSync(cfg['wasm_path']));
const loadedWasm = await WebAssembly.instantiate(wasmFile, imports);
wasm = loadedWasm.instance.exports;
} else if (globalThis.process) {
// Node (<22)
const fs = await import('fs');
const wasmFile = new Uint8Array(fs.readFileSync(cfg['wasm_path']));
const loadedWasm = await WebAssembly.instantiate(wasmFile, imports);
wasm = loadedWasm.instance.exports;
} else {
// Browser
const loadedWasm = await WebAssembly.instantiateStreaming(fetch(cfg['wasm_path']), imports);
wasm = loadedWasm.instance.exports;
}
Node.js 18 has a global
fetch
function, sotypeof fetch
used in diplomat-wasm.mjs won't work.