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

Feature request: support web assembly target #585

Open
Bluebugs opened this issue Jan 27, 2022 · 12 comments
Open

Feature request: support web assembly target #585

Bluebugs opened this issue Jan 27, 2022 · 12 comments

Comments

@Bluebugs
Copy link

It should be possible to support web assembly as a target for this library. The only way to support wasm target is to use a websocket library that support it as a target.

The current websocket library github.com/gorilla/websocket does not support and won't support wasm soon (gorilla/websocket#432). gorilla/websocket is also looking for a maintainer (gorilla/websocket#370) even if still does see some development activity (https://github.com/gorilla/websocket/graphs/code-frequency).

github.com/nhooyr/websocket does have support for wasm and is actively maintained. The library is also simpler and follow go net API more closely. It will actually simplify this library code switching to this other websocket library. The work for just a change can be seen here: 583

@MattBrittan
Copy link
Contributor

MattBrittan commented Jan 27, 2022

Thanks for the PR on this. Have you been able to successfully run your modified version of the library within a browser?

I'm really interested in feedback on this change from those using Websockets (because I don't). Whilst the Gorilla Websocket package is looking for a new maintainer it's still fairly active, widely used (github stats say used by 45.3k projects) and stable. github.com/nhooyr/websocket is a lot newer (first release 2019), not as widely used (github only lists one user) and the vast majority of commits are from the main author (however the API does look clean and it offers a comparison with the Gorilla library).

Gorilla websockets uses the BSD 2-Clause "Simplified" License and github.com/nhooyr/websocket the MIT License so I don't forsee any issues on that front.

@Bluebugs
Copy link
Author

Yes, I have been using it to actually run the application described in this blog inside a browser: https://fynelabs.com/2022/01/10/easy-mqtt-application-with-fyne/ . I don't have a convenient way to host it at the moment, but if that would help, I could figure out a place to host it.

@MattBrittan
Copy link
Contributor

Thanks @Bluebugs - just wanted to confirm that it was all working (as I have not tried using this with tinygo/WASM).

@Bluebugs
Copy link
Author

Sorry, I have been using go main compiler not tinygo. tinygo is on my list of things to experiment with, but I haven't gotten to it yet.

@Bluebugs
Copy link
Author

Bluebugs commented Feb 3, 2022

Anything I could do to help get this issue move forward?

@MattBrittan
Copy link
Contributor

@Bluebugs adding some tests is likely to speed up the process (without these I'll have to test manually). Having said that I don't see a reason to rush this because there is a workaround (use ClientOptions.SetCustomOpenConnectionFn).

My aim would be to make a new release before accepting this. It's a relatively major change so I'd like to see it used by those pulling @master for a while before unleashing it on everyone.

@Bluebugs
Copy link
Author

Bluebugs commented Feb 4, 2022

@MattBrittan how would you recommend me going at those tests. Do you have a mqtt server setup with websocket I can use in the context of the automated tests?

@MattBrittan
Copy link
Contributor

@Bluebugs yep. The tests are on the fvt folder and there is a docket folder under that (the idea being to spin up mosquito and run the tests). You can just add websockets to the mosquito confiig file.

@Bluebugs
Copy link
Author

Bluebugs commented Feb 4, 2022

Ok, I will look into that in a week or so as I have a lot for next week.

@MattBrittan
Copy link
Contributor

I've had another quick look at this. My main concern is that nhooyr.io/websocket has a significant number of dependencies and changing to this significantly increases the size of resulting executables. As a test I cloned the current library and your PR and compiled cmd.sample on both; the resulting executable sizes (windows) were:

6327808 sample.exe <- master
7107584 sample.exe <- your PR

So that's a 12% increase (around 800kb) with the move to nhooyr.io/websocket (and this change impacts all users). For most packages this would not be much of an issue (go executables are pretty big anyway) but as this library gets used on fairly low spec devices (I run it on Teltonika routers) it's something I want to keep an eye on. Based on this I'm reluctant to accept this PR - I would expect that ClientOptions.SetCustomOpenConnectionFn should provide a fairly easy to implement workaround without impacting all users.

Note: If any serious issues are identified with the gorilla package then I'd be happy to move (but do note that a new version of that package was released in Feb).

@Bluebugs
Copy link
Author

Bluebugs commented Aug 2, 2022

I didn't realize this would have been a concern as this module doesn't compile with tinygo last time I checked. I would be interested to see with a real application how much that does really represent as the sample application doesn't do very much, but I understand your concern. I will try to see how it fair with SetCustomOpenConnectionFn.

@MattBrittan
Copy link
Contributor

Thanks - please let me know how you get on (would be happy for such a module to be contributed; might add a plug-in folder off the repo for this kind of thing).
I'm not saying no to the PR but am trying to weigh up the benefits vs costs. If there is an easy workaround that meets your needs, which I suspect there is, then that removes any immediate need to make the change. I need to consider the impact of any change on average users (who are probably not even using web sockets) so think checking the impact on output size is of relevance (in this case I was concerned by number of dependencies nhooyr.io/websocket has; realise that almost all of these relate to tests but it means go mod tidy pulls in quite a bit!).
My use-case may be a little unusual as I just use the standard compiler; runs fine on a MIPS 74Kc with 128kb RAM (but I do keep an eye on the size due to fairly low bandwidth comms.

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

No branches or pull requests

2 participants