Skip to content

Conversation

@jurerotar
Copy link
Contributor

I think this should fix failing build-wasm workflow runs

Copy link
Collaborator

@tomayac tomayac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tomayac tomayac merged commit 313c0f8 into sqlite:main Jan 20, 2026
8 checks passed
@tomayac
Copy link
Collaborator

tomayac commented Jan 20, 2026

@jurerotar
Copy link
Contributor Author

It's a different issue now. Branch was actually created, but it can't open a PR, because we need to check a box in the settings.

Here's the branch: https://github.com/sqlite/sqlite-wasm/compare/update-sqlite-wasm-d4dc275ebc2941a850d1a3efebe3e706e4f1967f?expand=1

This needs to be checked for the PR to be opened automatically though:
image

@tomayac
Copy link
Collaborator

tomayac commented Jan 20, 2026

@sgbeal Can you open https://github.com/sqlite/sqlite-wasm/settings and toggle the checkbox from the above screenshot? It 404s for me, which is strange, as I thought I was the repo owner, but maybe you are? I honestly forgot who set this up.

@sgbeal
Copy link
Collaborator

sgbeal commented Jan 20, 2026

/settings is also 404 for me. Edit: the settings button along the top is also 404.

@tomayac
Copy link
Collaborator

tomayac commented Jan 20, 2026

Apparently a wider issue: https://github.com/orgs/community/discussions/176383. *sigh

@sgbeal
Copy link
Collaborator

sgbeal commented Jan 20, 2026

Apparently a wider issue: https://github.com/orgs/community/discussions/176383. *sigh

From Oct. 2025 and still being reported. Sigh.

@jurerotar
Copy link
Contributor Author

I'm sorry, I didn't know about this 😢 I think we can still open PRs manually, since branch with changes is still created successfully until this is resolved.

@tomayac
Copy link
Collaborator

tomayac commented Jan 20, 2026

I'm sorry, I didn't know about this 😢 I think we can still open PRs manually, since branch with changes is still created successfully until this is resolved.

SGTM, can you do this and I can review?

@jurerotar
Copy link
Contributor Author

I can't, sorry. It says I'm not a collaborator :(

Does this work for you? https://github.com/sqlite/sqlite-wasm/compare/sqlite:main...sqlite:update-sqlite-wasm-d4dc275ebc2941a850d1a3efebe3e706e4f1967f?expand=1

@tomayac
Copy link
Collaborator

tomayac commented Jan 20, 2026

I can't, sorry. It says I'm not a collaborator :(

No lie, I wanted to make you one yesterday, and the Settings 404'ed, and I thought it was just a temporary glitch. So we're stuck for now.

Does this work for you? https://github.com/sqlite/sqlite-wasm/compare/sqlite:main...sqlite:update-sqlite-wasm-d4dc275ebc2941a850d1a3efebe3e706e4f1967f?expand=1

This seems to have worked: #135. Can you go over it and check if everything you expect is there?

@jurerotar
Copy link
Contributor Author

Thank you for the vote of confidence!

It's good to go! 😄

@tomayac
Copy link
Collaborator

tomayac commented Jan 20, 2026

@tomayac
Copy link
Collaborator

tomayac commented Jan 20, 2026

@jurerotar Could I ask you to let me know which Issues can be closed now? It seems like you currently have more insights than I do.

@jurerotar
Copy link
Contributor Author

jurerotar commented Jan 20, 2026

Absolutely! I'm just testing the release on some of my projects and so far it seems to work perfectly! 😄

Give me a couple more minutes to make sure everything is fine, please, but after that we should be good to close the following:

Directly related:
#123
#118

This one I think is just out of scope: #104
A change for this one was merged, I think it's good to close, although an author should probably confirm: #102
I'm still working on this one: #53
Author said it's good to close: #133

@jurerotar
Copy link
Contributor Author

There's an issue with bundler-friendly builds. I think I know what it is, but I'm not exactly sure why it worked before when I tested it locally.

I think this injection of locateFile messes with the bundlers: https://github.com/sqlite/sqlite/blob/d4dc275ebc2941a850d1a3efebe3e706e4f1967f/ext/wasm/api/pre-js.c-pp.js#L70-L94

If this locateFile is never assigned, file is actually correctly found:

image

I'm currently investigating if this whole pre-js could be omitted from npm builds entirely. I'll keep you updated.

@tomayac
Copy link
Collaborator

tomayac commented Jan 20, 2026

Thank you!

@sgbeal
Copy link
Collaborator

sgbeal commented Jan 20, 2026

Re. locateFile(), see: https://sqlite.org/src/info/c8e6be9241

Edit: that's not a fix for this (what is?) but it's related.

@sgbeal
Copy link
Collaborator

sgbeal commented Jan 20, 2026

I'm currently investigating if this whole pre-js could be omitted from npm builds entirely.

  • pre-js.c-pp.js can be left out but the canonical test suite likely won't run (no biggie in this case). i will attempt to filter this file out entirely for bundler builds, but can make even fewer promises than before about whether it will work afterwards.
  • extern-post.* is required because it overrides the Emscripten-defined sqlite3InitModule() so that it can bootstrap the library.

@sgbeal
Copy link
Collaborator

sgbeal commented Jan 20, 2026

  • pre-js.c-pp.js can be left out but the canonical test suite likely won't run (no biggie in this case). i will attempt to filter this file out entirely for bundler builds, but can make even fewer promises than before about whether it will work afterwards.

Please try https://sqlite.org/src/info/982a91abc0, which makes pre-js disappear from bundler builds.

@jurerotar
Copy link
Contributor Author

jurerotar commented Jan 20, 2026

I'm testing this locally with manually omitted contents of pre-js.c-pp.js, and it seems to work fine (but it also worked fine the last time I tried, where this hasn't been omitted 😢).

image

Stephan, if you'd be kind enough to omit this from npm builds, let's try it.

You beat me to it, thank you!

@sgbeal
Copy link
Collaborator

sgbeal commented Jan 20, 2026

You beat me to it, thank you!

Not entirely - it was only elided for bundler-friendly builds, not sqlite3.mjs (which is an ESM, non-bundler build). i can't remove them from all esm builds without breaking (at a minimum) tests. However, since https://sqlite.org/src/info/c8e6be9241e3e178, i believe/strongly suspect that pre-js is innocuous for non-bundler use, as people who need to override locateFile() now have a way to do so (with the caveat that it is explicitly not guaranteed to continue to work in the future, for reasons explained in the forum post that checkin links to).

@jurerotar
Copy link
Contributor Author

jurerotar commented Jan 20, 2026

Can confirm your changes to be working, @sgbeal! Thank you!

@tomayac, we should run the workflow, create a new release and mark current one as deprecated.

I have the PR here: #136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants