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

compile WebAssembly in CI #486

Closed
wants to merge 4 commits into from
Closed

compile WebAssembly in CI #486

wants to merge 4 commits into from

Conversation

tombl
Copy link

@tombl tombl commented Sep 7, 2024

The current wasm build is broken because it's untested in CI. This PR fixes the issue and adds a wasm build.

(I can back out any changes if unwanted, but I'm hoping to follow up this PR with one to automate releases and add npm publishing.)

@tombl
Copy link
Author

tombl commented Sep 7, 2024

although now that I look at it, build_wasm.ts probably needs resyncing with https://github.com/nodejs/undici/blob/main/build/wasm.js.

@ShogunPanda
Copy link
Contributor

@tombl In general it looks good me. The only thing I don't like is having bash scripts, which are harder to debug.
Can you please sync the undici build script with build_wasm.ts? (if you feel more comfortable, you can also change it to be build_wasm.js).

@Angelmmiguel
Copy link

Hey! I've just found this PR after facing the #559 issue. Is this still WIP? I'm happy to jump in and try to solve what's missing here.

Thanks!

@tombl
Copy link
Author

tombl commented Jan 7, 2025

Thanks for the reminder on this PR, just copied over undici's build script and tweaked it to fit. Should be all good now?

@ShogunPanda
Copy link
Contributor

Hello!
To be honest I would prefer not to change anything else than the changes in the workflow file. Can you please restore Dockerfile and similar so that the workflow uses the original script?

@ShogunPanda
Copy link
Contributor

Hi again!
@tombl Thanks for your contribution, I've included it in #567, where I also refactored the entire workflow a bit. Closing this.

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