Repository navigation
feat: accept the .wasm bytes or a compiled module in loadModule() - #193
Conversation
Hosts that cannot fetch or read libpg-query.wasm (a custom protocol without
fetch support, a bundle that inlines assets) can pass it once with
loadModule({ wasm }). Every later instance, including replacements after a
failed call, is created from it. Applies to v13-v18.
Co-authored-by: Gldywn <14254051+Gldywn@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Review complete. 🟠 1 high · 🟡 1 medium 💬 Inline comments (2)
The change threads a caller-provided WASM module through Emscripten's
Note: the new Reviewed commit: 4640ae9 |
There was a problem hiding this comment.
This PR adds a loadModule({ wasm }) API letting callers supply pre-compiled WASM bytes or a WebAssembly.Module, copied identically across all six versioned packages, plus a subprocess-based test harness for WASM load recovery.
Key findings
- 🟠 Pass the
WebAssembly.Instance, not the result record — index.ts:155 - 🟡 Subprocess failures produce opaque test errors — load-module.test.js:27
…cs: pass only trusted wasm
Provided wasm sets compiled, so the sync replaceInstance recovery path reuses it without fetching; load-module OOM test asserts the repeated-limit error.
Summary
Some hosts can't fetch or read the package's own
libpg-query.wasm, for example an Electron custom protocol registered withoutsupportFetchAPI(Beekeeper Studio plugins) or a bundle that inlines its assets. Until now those hosts couldn't load the parser at all, becauseloadModule()ignored its arguments. With this change they hand the module over once:provided. Each later instance is created from it through Emscripten'sinstantiateWasm, using asyncWebAssembly.instantiateand no fetch. That includes the replacement instance after a failed call (createModule()ininstantiate()).wasm.parseSyncand the stale-instance recovery all behave as before.loadModule.lengthstays0, via a rest parameter.templates/index.ts(v13–17) andtemplates/full/index.ts(v18), and the generatedversions/*/src/index.tsfiles were regenerated withcopy:templates.Builds on the approach in #191 by @Gldywn, who is credited as co-author.
Tests
versions/*/test/load-module.test.jsruns the wrapper from a copy that has nolibpg-query.wasmnext to it:main: 3 of 6 fail.Every version's WASM was built locally and each full suite passed with 0 failures: v18 149/149, v13–17 78 pass / 20 skipped (the full-API tests, which are skipped in the slim builds).
I also checked it in Electron 37 on a
plugin://scheme withoutsupportFetchAPI. A parse succeeded, a deeply nested query threw, and the next parse succeeded again:{"ok":true,"first":1,"threw":true,"afterRecovery":1}.Link to Devin session: https://app.devin.ai/sessions/9bbf3e7a34e9477ea812446ccf8a5fb0
Open in Devin Desktop: https://app.devin.ai/desktop/session/9bbf3e7a34e9477ea812446ccf8a5fb0?variant=devin
Requested by: @pyramation