-
Notifications
You must be signed in to change notification settings - Fork 25
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
Support both WASM and GopherJS #28
base: master
Are you sure you want to change the base?
Conversation
Replace gopherwasm/js with syscall/js Use gopherjs 1.12-2's support for syscall/js to make the WASM code work in GopherJS as well.
@jancona I've integrated WASM compilation into nhooyr.io/websocket v1.6.0, I think you'll find it useful. |
Thanks @nhooyr I'll take a look. |
This PR is currently broken on Go 1.13 because of golang/go#32402. I can put together a fix, but I won't bother if there's no interest in merging this. Could someone associated with the project (@dmitshur or others) comment on whether this is worth pursuing? |
@johanbrandhorst What're your thoughts on this? |
I think it's worthwhile getting this in but I don't have merge access either unfortunately :(. |
This PR is based on @nightexcessive's
wasm
branch (see #27). It:gopherwasm/js
with Go 1.12syscall/js
syscall/js
to make the WASM code work in GopherJS as wellBased on the discussions in #27, I made the main
websocket
package no longer depend onwebsocketjs
. (I moved the necessary code intowebsocket
.)websocketjs
is kept unchanged for backwards compatibility, but is GopherJS-only. The end result should be that no breaking changes are made to the either package, but per @dmitshur's suggestion, the low-levelwebsocketjs
is not provided for WASM.Let me know if you'd prefer that I make the PR against the
wasm
branch instead ofmaster
. I'm also happy to cleanup the commit history if you think that's appropriate.