dart-lang / web_socket_channel

StreamChannel wrappers for WebSockets.
https://pub.dev/packages/web_socket_channel
BSD 3-Clause "New" or "Revised" License
412 stars 107 forks source link

Documentation for the 3.x migration #358

Closed jakemac53 closed 1 month ago

jakemac53 commented 1 month ago

I am trying to migration package:build to web_socket_channel 3.x (per 3678), but I am confused as to how to do so.

The changelog only tells you of the breakages (in my case, a removed constructor), but not what I am supposed to use now.

If you go to the WebSocketChannel docs, it still tells you to use the old constructor, which no longer exists (although, dartdoc will link you to it so I guess maybe it exists in some sense, but it isn't directly callable).

What is the replacement for WebSocketChannel.new?

jakemac53 commented 1 month ago

In case it is useful, we are using this in testing, with our own manual stream channels (essentially mocking out the actual connection)

brianquinlan commented 1 month ago

What is the replacement for WebSocketChannel.new?

They idea was to simply remove the WebSocketChannel constructor. Let me look at your tests.

brianquinlan commented 1 month ago

If I understand the tests, the idea is that you have two WebSocketChannels that are connected to each other as client and server.

The test writes to the server WebSocketChannel.sink and expects the results to appear in the client WebSocketChannel.stream.

Would it then make sense sense to have a test helper like (WebSocketChannel, WebSocketChannel) createFakes() that returns a client and a server web socket that are connected like I described?

image

jakemac53 commented 1 month ago

To be perfectly honest, this code is quite old so I have little recollection, but my read is the same as yours generally 🤣 .

Would it then make sense sense to have a test helper like (WebSocketChannel, WebSocketChannel) createFakes() that returns a client and a server web socket that are connected like I described?

I think so, yes. But the old API was more general purpose which was kind of nice (I don't think it's possible for me to implement createFakes myself, which ideally I could?).

I also don't know how much of the deleted code would have to be brought back...

jakemac53 commented 1 month ago

In general, some mechanism for creating a mocked out web socket channel is basically what I am asking for though, where there is no real connection under the covers, just streams/sinks which I control.

brianquinlan commented 1 month ago

I think that you can create those fakes yourself but it would make more sense for those to be part of package:web_socket_channel.

I don't want to resurrect the deleted code. The deleted code was a modified copy of dart:io's WebSocket implementation for 7 years ago that has not undergone active maintenance. Let's see if the fakes solve the problem ;-)

brianquinlan commented 1 month ago

I have a fake implementation that allows the test to pass locally. The github automation seems to have unrelated failures.

I'd suggest moving these fakes to package:web_socket_channel

Any reason not to do that? @natebosch

natebosch commented 1 month ago

Any reason not to do that? @natebosch

It doesn't look like it would add any new dependencies, so I can't think of a strong reason not to.

I do wonder whether it's actually necessary - the intention on WebSocketChannel is to act as a StreamChannel. Is there some place we should be loosening an argument type so that we can implement the fake in the test more naturally?

natebosch commented 1 month ago

Or alternatively, if package:web_socket deserves a fake implementation, we could consider migrating to that package sooner, since that's the long term goal.

natebosch commented 1 month ago

Migrating to StreamChannel has a slight hiccup, but I think migrating to WebSocket in place of WebSocketChannel looks feasible. @brianquinlan - WDYT about spending the effort on a fake for WebSocket?

brianquinlan commented 1 month ago

Yeah, I'll make a fake for WebSocket.