Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Update to handle Socket implements Stream<Uint8List> #65

Merged
merged 1 commit into from
Jul 9, 2019

Conversation

tvolkert
Copy link
Contributor

@tvolkert tvolkert commented Jul 9, 2019

A recent change in the Dart SDK updated Socket to
implement Stream<Uint8List> rather than Stream<List<int>>.
This forwards compatible change calls StreamTransformer.bind()
rather than Stream.transform(), thus putting the stream in a
covariant position and allowing for the transition to Uint8List.

dart-lang/sdk#36900

A recent change in the Dart SDK updated `Socket` to
implement `Stream<Uint8List>` rather than `Stream<List<int>>`.
This forwards compatible change calls `StreamTransformer.bind()`
rather than `Stream.transform()`, thus putting the stream in a
covariant position and allowing for the transition to `Uint8List`.

dart-lang/sdk#36900
@tvolkert
Copy link
Contributor Author

tvolkert commented Jul 9, 2019

@mraleph @grouma @kevmoo

@tvolkert
Copy link
Contributor Author

tvolkert commented Jul 9, 2019

I don't have permission to merge or publish 😕

@kevmoo kevmoo merged commit 2342e4e into dart-archive:master Jul 9, 2019
@kevmoo
Copy link
Contributor

kevmoo commented Jul 9, 2019

Going to land #66 then will publish

@@ -709,7 +709,7 @@ class WebSocketImpl extends Stream with _ServiceObject implements StreamSink {
_readyState = WebSocket.OPEN;

var transformer = _WebSocketProtocolTransformer(_serverSide);
_subscription = stream.transform(transformer).listen((data) {
_subscription = transformer.bind(stream).listen((data) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to be necessary long term or only during the transition?

Copy link
Contributor Author

@tvolkert tvolkert Jul 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. If we decide to follow-up with a change to make Encoding extend Codec<String, Uint8List>, then this will only be needed during a transition period. however, such a change would be very breaking in its own right, so we'd need to evaluate whether it was warranted.
  2. If decide not to follow-up with such a change, then either this change (using StreamTransformer.bind() rather than Stream.transform()) or using Stream.cast<List<int>>.transform() will be necessary going forward.

I tend to prefer the option of updating Encoding, but then again, I tend towards clean APIs at the expense of breakages / churn. I haven't yet opened that decision up for discussion - waiting for the dust to settle with the existing round of breakages 🙂

@tvolkert tvolkert deleted the socket branch July 9, 2019 23:10
mosuem pushed a commit to dart-lang/http that referenced this pull request Dec 11, 2024
…b_socket_channel#65)

A recent change in the Dart SDK updated `Socket` to
implement `Stream<Uint8List>` rather than `Stream<List<int>>`.
This forwards compatible change calls `StreamTransformer.bind()`
rather than `Stream.transform()`, thus putting the stream in a
covariant position and allowing for the transition to `Uint8List`.

dart-lang/sdk#36900
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

4 participants