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

Switch to ring/ring-jetty-adapter #217

Open
6 tasks
jacobobryant opened this issue Jul 13, 2024 · 0 comments
Open
6 tasks

Switch to ring/ring-jetty-adapter #217

jacobobryant opened this issue Jul 13, 2024 · 0 comments

Comments

@jacobobryant
Copy link
Owner

Now that the Ring spec + official Ring Jetty adapter supports websockets, we could switch to that instead of using info.sunng/ring-jetty9-adapter. The version of that lib that Biff uses--0.17.2--uses a non-Ring-spec API for websockets (since the Ring websocket spec didn't exist at the time it was created). So switching to ring-jetty-adapter would be a breaking change unless we can have Biff's use-jetty component start translating the old ring-jetty9-adapter websocket response format into the Ring spec format. Also, Biff users may have code that requires ring.adapter.jetty9 directly; e.g. the starter app uses ring.adapter.jetty9/send! for sending websocket messages. Though maybe that function would actually still work even if we switched to ring-adapter-jetty?

If we didn't want to deal with breaking changes, we could upgrade to a 0.22.x version of ring-jetty9-adapter at least, which has the same websocket API. I see that 0.17.x is no longer maintained (version matrix). However I think it'd be better to just go ahead and switch to the Ring spec. Even the latest version of ring-jetty9-adapter has switched its websocket format to that. It'll be easier for users if they aren't surprised by the non-standard websocket API.

Looks like ring-jetty-adapter still supports Java 11, so no breaking changes there.

Should we just leave ring-jetty9-adapter in Biff's deps for a while? If that works, there'd be zero breaking changes. We could have use-jetty keep using ring-jetty9-adapter, at least by default. Either provide a different Biff component that uses ring-jetty-adapter, or add a config option that tells use-jetty which one to use. In this case, we would upgrade ring-jetty9-adapter to a 0.22.x release, which--like ring-jetty-adapter--depends on Jetty 11.x.

So the todo list here:

  • Add ring-jetty-adapter to Biff's dependencies.
  • Update the starter app.
  • See how much backwards compatibility we can provide--maybe we can get away without any breaking changes. If we don't go that route, remove the ring-jetty9-adapter dependency.
  • Update the tutorial.
  • Update any other docs that include websocket code (e.g. check the reference docs).
  • Draft a release highlighting the breaking change(s), if there are any.
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

1 participant