Make StreameRequest.sink a StreamSink #965
Conversation
Closes #727 This class was not designed for extension, but there is at least one extending implementation. I don't see any overrides of this field, so no existing usage should be broken.
|
Ah interesting, this causes new |
Yeah, this change is technically breaking. If we assume that it is non-breaking because no one implements this class then you could change it to |
pkgs/http/test/io/client_test.dart
Outdated
| ..sink.add('{"hello": "world"}'.codeUnits) | ||
| ..sink.close(); | ||
| request.sink.add('{"hello": "world"}'.codeUnits); | ||
| await request.sink.close(); |
There was a problem hiding this comment.
None of the awaits in the tests are necessary, right? Otherwise this would be a breaking change.
There was a problem hiding this comment.
Only necessary for lints - although it looks like adding the await can be breaking, so we may want to reconsider the risk of this change. It would be safe to use unawaited() exclusively instead.
| @@ -32,7 +32,7 @@ class StreamedRequest extends BaseRequest { | |||
| /// buffered. | |||
| /// | |||
| /// Closing this signals the end of the request. | |||
| EventSink<List<int>> get sink => _controller.sink; | |||
| StreamSink<List<int>> get sink => _controller.sink; | |||
There was a problem hiding this comment.
I think that you need to update the example in the documentation for this class.
|
cc @jakemac53 - this change isn't directly breaking, but with a typical linting setup that uses |
Prefix all lines of the commment-in-example-in-comment with `//`. Fix grammar. Remove "generally". Remove redundant mention of the not completing future - it's more clear to state it above the `unawaited` in the example code.
Closes #727
This class was not designed for extension, but there is at least one
extending implementation. I don't see any overrides of this field, so no
existing usage should be broken.