Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Updates for Dart 2.0 core library changes (wave 2.2) #45

Merged
merged 3 commits into from
Feb 10, 2018
+28 −14
Merged

Updates for Dart 2.0 core library changes (wave 2.2) #45

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5556b36
Updates for Dart 2.0 core library changes (wave 2.2)
leafpetersen Jan 31, 2018
62ac241
CHANGLOG markdown fix
leafpetersen Jan 31, 2018
66f2e20
orElse changes and bump minimum sdk
jakemac53 Feb 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
## 2.0.4

* Add support for Dart 2.0 library changes to Stream and StreamTransformer.
Changed classes that implement `StreamTransformer` to extend
`StreamTransformerBase`, and changed signatures of `firstWhere`, `lastWhere`,
and `singleWhere` on classes extending `Stream`. See also
https://github.com/dart-lang/sdk/issues/31847
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I'd write this

* ... See also [issue 31847][sdk#31847].

[sdk#31847]: https://github.com/dart-lang/sdk/issues/31847

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


## 2.0.3

* Fix a bug in `StreamQueue.startTransaction()` and related methods when
Expand Down
2 changes: 1 addition & 1 deletion lib/src/result/capture_transformer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import 'capture_sink.dart';
///
/// The result of the transformation is a stream of [Result] values and no
/// error events. Exposed by [Result.captureStream].
class CaptureStreamTransformer<T> implements StreamTransformer<T, Result<T>> {
class CaptureStreamTransformer<T> extends StreamTransformerBase<T, Result<T>> {
const CaptureStreamTransformer();

Stream<Result<T>> bind(Stream<T> source) {
Expand Down
2 changes: 1 addition & 1 deletion lib/src/result/release_transformer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'result.dart';
import 'release_sink.dart';

/// A transformer that releases result events as data and error events.
class ReleaseStreamTransformer<T> implements StreamTransformer<Result<T>, T> {
class ReleaseStreamTransformer<T> extends StreamTransformerBase<Result<T>, T> {
const ReleaseStreamTransformer();

Stream<T> bind(Stream<Result<T>> source) {
Expand Down
2 changes: 1 addition & 1 deletion lib/src/single_subscription_transformer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import 'dart:async';
/// This also casts the source stream's events to type `T`. If the cast fails,
/// the result stream will emit a [CastError]. This behavior is deprecated, and
/// should not be relied upon.
class SingleSubscriptionTransformer<S, T> implements StreamTransformer<S, T> {
class SingleSubscriptionTransformer<S, T> extends StreamTransformerBase<S, T> {
const SingleSubscriptionTransformer();

Stream<T> bind(Stream<S> stream) {
Expand Down
20 changes: 12 additions & 8 deletions lib/src/typed/stream.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,18 @@ class TypeSafeStream<T> extends Stream<T> {
Stream<S> expand<S>(Iterable<S> convert(T value)) =>
_stream.expand(_validateType(convert));

Future firstWhere(bool test(T element), {Object defaultValue()}) =>
_stream.firstWhere(_validateType(test), defaultValue: defaultValue);

Future lastWhere(bool test(T element), {Object defaultValue()}) =>
_stream.lastWhere(_validateType(test), defaultValue: defaultValue);

Future<T> singleWhere(bool test(T element)) async =>
(await _stream.singleWhere(_validateType(test))) as T;
Future<T> firstWhere(bool test(T element),
{Object defaultValue(), T orElse()}) =>
_stream.firstWhere(_validateType(test),
defaultValue: defaultValue ?? orElse);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be defaultValue: defaultValue, orElse: orElse. I don't want this class to have any logic beyond what's necessary to get the types to work out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to (need to?) land this before @lrhn lands the breaking change in the SDK, so currently there is no orElse on stream to pass this to. I will file an issue to follow up with a PR removing the defaultsTo argument and this logic here after the SDK changes land.

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding from talking with @srawlins at Dart Conf was that we were just going to have this package briefly be in a broken state on master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. Yes, I'm fine with that solution if you prefer. It does reduce the number of PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@keertip @srawlins If I make all of the changes in one go here and leave it on a branch, does that work for testing internal code? I think that means you would have to test with an atomic CL, since you couldn't roll this package before you roll the SDK. You would then presumably also have to roll this package with the SDK when we finally land. Does that work, or should we do these changes in two passes?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that works with testing internal code. It... should work for SDK as well, right? orElse can land, and aync's version can bump in DEPS, all in the same changelist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking about it, I guess I could push multiple commits to this branch, and then we could roll internal to the first tagged commit (this version), then roll internal to the second tagged commit (the final version). As far as this package is concerned, there is just one PR with a bunch of commits, but internally we can point to different hashes in this branch at different points in the process.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need multiple commits? Can't both the SDK and internal atomically add orElse and bump their dependency on async in the same commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want internal to be green before we land the changes in the SDK.

To get internal green, we need to run presubmits, fix, iterate.

It makes running presubmits easier if we can roll a dev.20 compatible version internally.

So I think having two commits to reference here for now would be better. Since we're referencing them off the branch, I don't think it should matter in the long run - we'll just leave this branch sitting here for now, and land the PR atomically as usual once everything is ready.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍


Future<T> lastWhere(bool test(T element),
{Object defaultValue(), T orElse()}) =>
_stream.lastWhere(_validateType(test),
defaultValue: defaultValue ?? orElse);

Future<T> singleWhere(bool test(T element), {T orElse()}) async =>
(await _stream.singleWhere(_validateType(test))) ?? orElse();

Future<S> fold<S>(S initialValue, S combine(S previous, T element)) =>
_stream.fold(
Expand Down
2 changes: 1 addition & 1 deletion lib/src/typed_stream_transformer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ StreamTransformer<S, T> typedStreamTransformer<S, T>(

/// A wrapper that coerces the type of the stream returned by an inner
/// transformer.
class _TypeSafeStreamTransformer<S, T> implements StreamTransformer<S, T> {
class _TypeSafeStreamTransformer<S, T> extends StreamTransformerBase<S, T> {
final StreamTransformer _inner;

_TypeSafeStreamTransformer(this._inner);
Expand Down
4 changes: 2 additions & 2 deletions pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
name: async
version: 2.0.3
version: 2.0.4
author: Dart Team <misc@dartlang.org>
description: Utility functions and classes related to the 'dart:async' library.
homepage: https://www.github.com/dart-lang/async
environment:
sdk: ">=2.0.0-dev.15.0 <2.0.0"
sdk: ">=2.0.0-dev.20.0 <2.0.0"
dependencies:
collection: "^1.5.0"
dev_dependencies:
Expand Down