-
Notifications
You must be signed in to change notification settings - Fork 7
#21 Make xz-decompress compatible with Cloudflare Workers
#22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,7 @@ | |
| }, | ||
| "files": [ | ||
| "dist/package/**", | ||
| "dist/native/xz-decompress.wasm", | ||
| "types.d.ts" | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,15 @@ const ReadableStream = globalThis.ReadableStream | |
| // This won't be reached in modern browsers, and bundlers will ignore due to 'browser' field in package.json: | ||
| || require('stream/web').ReadableStream; | ||
|
|
||
| // Try to compile the wasm module eagerly at import time. This fails in runtimes | ||
| // like Cloudflare Workers that block WebAssembly.compile entirely; those runtimes | ||
| // must call XzReadableStream.setWasmModule() with a pre-compiled module instead. | ||
| let _wasmModulePromise = (async () => { | ||
| const base64Wasm = xzwasmBytes.replace('data:application/wasm;base64,', ''); | ||
| const wasmBytes = Uint8Array.from(atob(base64Wasm), c => c.charCodeAt(0)).buffer; | ||
| return WebAssembly.compile(wasmBytes); | ||
| })().catch(() => null); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compilation still needs to be lazy in all cases |
||
|
|
||
| const XZ_OK = 0; | ||
| const XZ_STREAM_END = 1; | ||
|
|
||
|
|
@@ -97,35 +106,67 @@ export class XzReadableStream extends ReadableStream { | |
| static _moduleInstance; | ||
| static _contextMutex = new ContextMutex(); | ||
|
|
||
| /** | ||
| * Provide a pre-compiled WebAssembly.Module for runtimes that block | ||
| * dynamic compilation (e.g. Cloudflare Workers). | ||
| */ | ||
| static setWasmModule(wasmModule) { | ||
| _wasmModulePromise = Promise.resolve(wasmModule); | ||
| XzReadableStream._moduleInstance = null; | ||
| XzReadableStream._moduleInstancePromise = null; | ||
| } | ||
|
|
||
| static async _getModuleInstance() { | ||
| const base64Wasm = xzwasmBytes.replace('data:application/wasm;base64,', ''); | ||
| const wasmBytes = Uint8Array.from(atob(base64Wasm), c => c.charCodeAt(0)).buffer; | ||
| const wasmOptions = {}; | ||
| const module = await WebAssembly.instantiate(wasmBytes, wasmOptions); | ||
| XzReadableStream._moduleInstance = module.instance; | ||
| const compiledModule = await _wasmModulePromise; | ||
| if (!compiledModule) { | ||
| throw new Error( | ||
| 'WebAssembly compilation is not available in this runtime. ' + | ||
| 'Call XzReadableStream.setWasmModule(module) with a pre-compiled WebAssembly.Module before use.' | ||
| ); | ||
| } | ||
| XzReadableStream._moduleInstance = await WebAssembly.instantiate(compiledModule); | ||
| } | ||
|
|
||
| constructor(compressedStream) { | ||
| let xzContext; | ||
| let unconsumedInput = null; | ||
| let finalized = false; | ||
| let initError = null; | ||
| const compressedReader = compressedStream.getReader(); | ||
|
|
||
| super({ | ||
| async start(controller) { | ||
| await XzReadableStream._contextMutex.acquire(); | ||
| function finalizeOnce() { | ||
| if (finalized) return; | ||
| finalized = true; | ||
| if (xzContext) { | ||
| xzContext.dispose(); | ||
| xzContext = null; | ||
| } | ||
| XzReadableStream._contextMutex.release(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm very suspicious of the changes in the stream machinery here. Seems like a totally separate issue, and I suspect the agent has just got confused and run into a separate issue because there's no reason Workers should have different stream issues than normal JS. Lets drop all of this and do a PR just for module loading. If there is a separate issue with the stream behaviour, we should be able to reproduce it in Node with a failing test, and then fix it independently in another PR. For now lets revert all of that and just fix the module part here. |
||
| } | ||
|
|
||
| try { | ||
| if (!XzReadableStream._moduleInstance) { | ||
| await (XzReadableStream._moduleInstancePromise || (XzReadableStream._moduleInstancePromise = XzReadableStream._getModuleInstance())); | ||
| } | ||
| xzContext = new XzContext(XzReadableStream._moduleInstance); | ||
| } catch (error) { | ||
| XzReadableStream._contextMutex.release(); | ||
| throw error; | ||
| const initPromise = (async () => { | ||
| await XzReadableStream._contextMutex.acquire(); | ||
| try { | ||
| if (!XzReadableStream._moduleInstance) { | ||
| await (XzReadableStream._moduleInstancePromise || (XzReadableStream._moduleInstancePromise = XzReadableStream._getModuleInstance())); | ||
| } | ||
| xzContext = new XzContext(XzReadableStream._moduleInstance); | ||
| } catch (error) { | ||
| initError = error; | ||
| XzReadableStream._contextMutex.release(); | ||
| } | ||
| })(); | ||
|
|
||
| super({ | ||
| async start() { | ||
| await initPromise; | ||
| if (initError) throw initError; | ||
| }, | ||
|
|
||
| async pull(controller) { | ||
| await initPromise; | ||
| if (initError) throw initError; | ||
|
|
||
| try { | ||
| if (xzContext.needsMoreInput()) { | ||
| if (unconsumedInput === null || unconsumedInput.byteLength === 0) { | ||
|
|
@@ -144,26 +185,20 @@ export class XzReadableStream extends ReadableStream { | |
| xzContext.resetOutputBuffer(); | ||
|
|
||
| if (nextOutputResult.finished) { | ||
| xzContext.dispose(); | ||
| XzReadableStream._contextMutex.release(); | ||
| finalizeOnce(); | ||
| controller.close(); | ||
| } | ||
| } catch (error) { | ||
| if (xzContext) { | ||
| xzContext.dispose(); | ||
| } | ||
| XzReadableStream._contextMutex.release(); | ||
| finalizeOnce(); | ||
| throw error; | ||
| } | ||
| }, | ||
| cancel() { | ||
| async cancel() { | ||
| await initPromise; | ||
| try { | ||
| if (xzContext) { | ||
| xzContext.dispose(); | ||
| } | ||
| return compressedReader.cancel(); | ||
| } finally { | ||
| XzReadableStream._contextMutex.release(); | ||
| finalizeOnce(); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes that internal path part of the public API contract for this module. It's also a bit of an awkward fallback generally, especially in downstream modules that want to wrap this package.
I have an interesting alternative approach: what if we create a pure JS fallback? We can mechanically compile the existing inline WASM to asm.js with
wasm2jsand just ship the JS equivalent. Browsers no longer optimize asm nowadays, so it's a bit slower, but it's pure JS so it'll run anywhere. That removes the WASM requirement completely. Performance hit is unlikely to matter unless your app is decompressing huge XZs in batch all day long. Makes usage and deployment way simpler, covers lots of other cases cleanly, and it'd be easy to bring back the optional WASM file approach. Means no special APIs or funky WASM deployment steps required.I'd suggest we still use the current model by preference, but pull in a precompiled pure JS equivalent when it's unavailable.
Would that work for you?