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

Migrate from deprecated websocket package #368

Closed
Jmgr opened this issue Aug 3, 2021 · 4 comments · Fixed by #433
Closed

Migrate from deprecated websocket package #368

Jmgr opened this issue Aug 3, 2021 · 4 comments · Fixed by #433

Comments

@Jmgr
Copy link
Contributor

Jmgr commented Aug 3, 2021

Ably-go is currently using golang.org/x/net/websocket as its websocket package. Seeing that this package is now deprecated (see golang/go#18152 and https://pkg.go.dev/golang.org/x/net/websocket) we might want to migrate to another one.

A good alternative could be https://github.com/nhooyr/websocket, as it provides more features (notably the ability to use the browser's WebSockets transparently when targetting wasm, allowing you to write a Go app for the browser that can connect to Ably). Both APIs are not compatible though, so this will require a few code changes.

┆Issue is synchronized with this Jira Uncategorised by Unito

@Rosalita
Copy link
Contributor

Hi @Jmgr I would like to try get this issue back on track.

I can see in the docs for deprecated package that they recommend two packages
https://godoc.org/github.com/gorilla/websocket
https://godoc.org/nhooyr.io/websocket

I can also see that you have a draft PR open here #433 that implements one of those alternatives.

Would it be possible to get your draft PR rebased against latest main branch and ready for review?

@Jmgr
Copy link
Contributor Author

Jmgr commented May 19, 2022

I have rebased the branch, however some tests are not passing anymore.

@Rosalita
Copy link
Contributor

I've taken a look at the failing test and commented on the PR.

@Rosalita
Copy link
Contributor

Rosalita commented Jun 6, 2022

Due to issues with existing tests, we need to complete #541 before we can switch the websocket package.

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

Successfully merging a pull request may close this issue.

2 participants