Skip to content
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

Promisify emscripten modules & fix webp examples #817

Merged
merged 4 commits into from
Sep 29, 2020
Merged

Promisify emscripten modules & fix webp examples #817

merged 4 commits into from
Sep 29, 2020

Conversation

manzt
Copy link
Contributor

@manzt manzt commented Sep 1, 2020

It seems a promise-based API was added to modularized Emscripten builds in response to emscripten-core/emscripten#5820:

PR: emscripten-core/emscripten#10697

Since consolidating C++ builds (#781), the emscripten/emsdk Docker image produces module factory functions that return a promise for the emscripten module. This PR:

  • Fixes broken webp examples: codecs/webp/enc/example.html & codecs/webp/dec/example.html by using the new promise-based API.
  • Adds type annotations to the module factory functions for each C++ codec (now they return a promise).
  • Simplifies initEmscriptenModule to just call module factory.

PS: Thanks for all the work here. I've modeled the C++ builds for one of my projects on squoosh's codecs, which is how I ran into this. Much appreciated!

@RReverser
Copy link
Contributor

Oh I didn't see this PR, sorry @manzt! I did something very similar in 4c4f05f (#829), although you have updated examples as well.

I think we can merge this PR first, and rebase mine on top. @surma WDYT?

@surma
Copy link
Collaborator

surma commented Sep 29, 2020

Go for it.

@RReverser RReverser merged commit 63ac34a into GoogleChromeLabs:dev Sep 29, 2020
@jakearchibald
Copy link
Collaborator

Heh, I also did this in the rollup-build branch 🙃

@manzt manzt deleted the manzt/promisify-emscripten-modules branch September 30, 2020 17:09
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.

None yet

4 participants