-
Notifications
You must be signed in to change notification settings - Fork 77
feat: reworked build process and package structure #131
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?
Conversation
* added a Dockerfile to create sqlite build environment * added a sqlite build script * added a GitHub Actions workflow which runs tests, check types and validates formatting * added a GitHub Actions workflow which builds Docker image, builds sqlite for wasm, extracts wasm file and bindings and creates a PR to main branch with the updated bindings and wasm file * added Vitest with browser tests for sahpool and oo1 and node tests for sqlite3-node * added tsdown for dead-code-elimination and basic build steps * added lefthook for formatting on push
|
I might have gone a bit overboard with this 😆 In any case, I've reworked most of the package. I've also added a workflow which runs tests and typecheck on push, as well as a manual workflow which builds sqlite and bindings, then opens a PR if there's a change. This should probably only be run on new releases, but there's nothing stopping you from running it more often (see example of it running here: https://github.com/jurerotar/sqlite-wasm/actions/runs/20877842382). Docker build and sqlite build take aprox. 2 minutes to run, which I'm sure could be optimized further by someone who knows a bit more than me about this stuff 😄 I added tsdown, which removes comments and eliminates some unused code (I think most of these files are about 25-30% lighter after build). Target is es2023, which I think is safe-enough, but let me know if we should set it to an earlier version. One more thing I want to do, but I want your blessing first, @tomayac, is removing the demo files. Since we now have tests, folks can just look at those and see an exact usage. This isn't a hill I care to die on though, so either way is fine. |
|
THANK YOU!!! :-D |
|
I get this error when trying out the Node.js version: Specifically running this block of code in Module$1["instantiateWasm"] = function callee(imports, onSuccess) {
const sims = this;
const uri = Module$1.locateFile(sims.wasmFilename, "undefined" === typeof scriptDirectory ? "" : scriptDirectory);
sims.debugModule("instantiateWasm() uri =", uri, "sIMS =", this);
const wfetch = () => fetch(uri, { credentials: "same-origin" });
const finalThen = (arg) => {
arg.imports = imports;
sims.instantiateWasm = arg;
onSuccess(arg.instance, arg.module);
};
return (WebAssembly.instantiateStreaming ? async () => WebAssembly.instantiateStreaming(wfetch(), imports).then(finalThen) : async () => wfetch().then((response) => response.arrayBuffer()).then((bytes) => WebAssembly.instantiate(bytes, imports)).then(finalThen))();
}.bind(sIMS);To reproduce, I ran:
import sqlite3InitModule from "@sqlite.org/sqlite-wasm";
export const sqlite3 = await sqlite3InitModule();
console.log(sqlite3.version.libVersion);Maybe you expect this according to your Node.js demo: // Mock fetch because Emscripten's Node.js loader still uses it and it fails on file:// URLs
globalThis.fetch = async (url) => {
if (url.toString().endsWith('.wasm')) {
const buffer = readFileSync(
new URL('../dist/sqlite3.wasm', import.meta.url),
);
return new Response(buffer, {
headers: { 'Content-Type': 'application/wasm' },
});
}
throw new Error(`Unexpected fetch to ${url}`);
};I can patch Anyway, I’m very glad it’s workable now! I just wonder if there’s a better way than patching |
|
@jlarmstrongiv, thank you for checking it out! 😄 You're right, patching |
…1aef970db9de6d0dc21) (#7) Co-authored-by: jurerotar <28565137+jurerotar@users.noreply.github.com>
|
Alright, after some digging in to sqlite build process and some experimentation, I think I've solved this one. The issue seems to have originated from this script, which was being injected in to bundler-friendly, node & sqlite.mjs scripts. While it doesn't seem to affect bundler-friendly and sqlite.mjs, it broke node due to it using fetch on a local file. @sgbeal (thank you!) was kind enough to exclude this injection on sqlite3-node.mjs and tests are passing now! I'm pretty sure we can exclude this script from being injected in to other files as well and save some bandwidth this way, but that's not critical now 😄 |
|
To summarize an email on the topic: the overrides you've removed are relevant for the upstream tree but are not currently relevant here. There are no concrete plans to make them relevant here, but it might happen via future day-to-day maintenance. |
Uh oh!
There was an error while loading. Please reload this page.