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

Add StreamExtensions.firstOrNull #195

Merged
merged 3 commits into from
Aug 31, 2021
Merged

Add StreamExtensions.firstOrNull #195

merged 3 commits into from
Aug 31, 2021

Conversation

nex3
Copy link
Contributor

@nex3 nex3 commented Aug 27, 2021

By analogy to IterableExtensions.firstOrNull in the collection package.

By analogy to IterableExtensions.firstOrNull in the collection package.
@nex3 nex3 requested a review from natebosch August 27, 2021 23:51
@google-cla google-cla bot added the cla: yes label Aug 27, 2021
@natebosch
Copy link
Contributor

I think we need to hold off on this until we have resolved dart-lang/core#330, and then we can put this in that library.

In the short term we could add this in package:stream_transform which will have a (hopefully) smooth transition once we do incorporate it here.

Co-authored-by: Nate Bosch <nbosch@google.com>
@nex3
Copy link
Contributor Author

nex3 commented Aug 28, 2021

I don't think this would make sense in stream_transformer, since it's not representable as a transformer—it's a stream-to-future transformation, not a stream-to-stream transformation. Also, there's precedent for adding new features here rather than there in #175.

@natebosch
Copy link
Contributor

it's not representable as a transformer—it's a stream-to-future transformation, not a stream-to-stream transformation

That's a good point.

@lrhn - which library would you prefer this goes into?

Copy link
Contributor

@lrhn lrhn left a comment

Choose a reason for hiding this comment

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

I see no problem with adding it to StreamExtensions in package:async. It's the perfect place. The corresponding collection method was in package:collection, so the analogy is perfect.

Code looks fine too.

We could consider adding more extensions along with this, perhaps before releasing 2.9.0, but starting here seems fine.

lib/src/stream_extensions.dart Outdated Show resolved Hide resolved
/// or `null` if it emits a done event first.
Future<T?> get firstOrNull {
var completer = Completer<T?>.sync();
final subscription = listen(null);
Copy link
Contributor

@lrhn lrhn Aug 31, 2021

Choose a reason for hiding this comment

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

Can use cancelOnError: true. Then you can just tear off completer.completeErrorinstead of creating a new closure. It can even be:

  var subscription = listen(null, 
    onError: completer.completeError,
    onDone: completer.complete, // The argument is optional if the type is nullable!
    cancelOnError: true);
  subscription.onData((event) {
    subscription.cancel();
    completer.complete(event);
  });

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 considered doing it that way, but I think the symmetry between the onData and onError events is easier to follow than having one of them manually cancel the subscription and the other cancel it through cancelOnError.

test/stream_extensions_test.dart Show resolved Hide resolved
@nex3 nex3 requested a review from lrhn August 31, 2021 20:27
@natebosch natebosch merged commit ecb3835 into master Aug 31, 2021
@natebosch natebosch deleted the first-or-null branch August 31, 2021 23:07
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 14, 2024
By analogy to IterableExtensions.firstOrNull in the collection package.
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.

3 participants