Repository navigation
feat: accept the .wasm bytes or a compiled module in loadModule() - #192
Closed
pyramation wants to merge 2 commits into
Closed
pyramation wants to merge 2 commits into
pyramation wants to merge 2 commits into
Conversation
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. No blocking issues — approved ✅; 2 nitpicks below. 🧹 Nitpicks (2) — 🟢 2 low
This PR lets callers bypass the default file-based WASM loader by passing pre-compiled bytes to
Reviewed commit: 1c4fdae |
…ule() Co-authored-by: Gldywn <14254051+Gldywn@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Some hosts can't read
libpg-query.wasmthemselves. One example is an Electron custom protocol registered withoutsupportFetchAPI, which is the Beekeeper Studio plugin case from #191. On those hosts there is currently no way to load the module, in any version. This PR lets the host pass the file in once, in v13–v18:The input is compiled once and stored as
compiled. Every instance is then created from it, including the synchronous replacement after a failed call (replaceInstance) and the retry after the out-of-memory cap. The wrapper never reads the file after that. If the default loader is still in flight from import time, its result is discarded oncewasmhas been provided.The same change goes into both templates:
templates/full/index.tsfor v18 andtemplates/index.tsfor v13–v17. The generatedversions/*/src/index.tsare regenerated withpnpm run copy:templates.This is stacked on #190 because it reuses that PR's
compiledandreplaceInstancepath. It does the same job as #191 (thanks @Gldywn) without an async per-reload hook. It doesn't change import-time loading, soparseSyncbehaves as before.Other details:
loadModuletakes a rest parameter, soloadModule.lengthstays0. That keepsbefore(loadModule)working.globalThis.WebAssembly, so the.d.tstype-checks withlib: ["es2022"]and no DOM or Node types.Tests
test/load-module.test.jsis in every version. It runs a copy of the wrapper that has nolibpg-query.wasmnext to it, and covers:WebAssembly.Moduleand from a typed arrayResults:
fix: combined WASM hardening (stack guard, v18 errors, scan OOM, sync recovery, ESM, native parity) #190 head without this change: 3 of 6 fail.
With this change, the WASM was built locally for each version and every suite passes:
Electron 37, with the page served from
plugin://withoutsupportFetchAPI:Aborted(both async and sync fetching of the wasm failed)loadModule({ wasm: bytes }): loads, a deep query throws, and the next parse succeeds.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