Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge into package:async #111

Open
mit-mit opened this issue Jun 18, 2020 · 6 comments
Open

Merge into package:async #111

mit-mit opened this issue Jun 18, 2020 · 6 comments

Comments

@mit-mit
Copy link
Member

mit-mit commented Jun 18, 2020

We should consider merging this into https://pub.dev/packages/async

cc @natebosch

@natebosch
Copy link
Member

Also tracking at dart-lang/async#117

@natebosch
Copy link
Member

Taking a note just in case - if we end up deciding not to do this for some reason (I don't expect we would) we should consider reviving dart-lang/site-www#2974

@mit-mit
Copy link
Member Author

mit-mit commented Jun 11, 2024

@lrhn wdyt about merging this in as package:async/stream_transform.dart ?

@lrhn
Copy link
Member

lrhn commented Jun 11, 2024

I migth want to go through the operations a little more thoroughly before accepting them in package:async.

(A quick check showed the transformByHandlers's _defaultHandleValue does a runtime cast. I'd probably let it drop the value instead. Possibly have a different operation for transforming to the same type that defaults to preserving the value. I'm sure most of it is fine, but I'll want to make sure.)

natebosch added a commit that referenced this issue Jun 12, 2024
Towards #111

The default callback is not used in practice outside of tests, but it
included a cast to satisfy the type signature which looks worrying for
this type of library.

Make the callback required and add explicit callbacks (without a cast
since the types are consistent) in the tests which used it.
@natebosch
Copy link
Member

A quick check showed the transformByHandlers's _defaultHandleValue does a runtime cast. I'd probably let it drop the value instead.

The default callback isn't used outside of tests, and there it uses consistent types and doesn't need the cast. We can mark it required and avoid the misleading cast.

@lrhn
Copy link
Member

lrhn commented Jun 13, 2024

I checked more, and there are definitely things I'd want to update (using .wait instead of Future.wait, not checking if subscription.cancel() returns null, which it can't since 2.12, and maybe some restructuring of some of the code to make it work better with Stream.multi streams. And maybe optimizations.

I did try chaingng Future.value(initial).asStream() to Stream.value(intiial) in startWith. That broke some tests becuase of a difference in timing, so the tests may need to be made less timing sensitive, if the behavior cannot be.
(Not sure all combinations of broadcast/non-broadcast streams really make sense.)

natebosch added a commit that referenced this issue Jun 13, 2024
Towards #111

The default callback is not used in practice outside of tests, but it
included a cast to satisfy the type signature which looks worrying for
this type of library.

Make the callback required and add explicit callbacks (without a cast
since the types are consistent) in the tests which used it.
natebosch added a commit that referenced this issue Jun 13, 2024
natebosch added a commit that referenced this issue Jun 13, 2024
Towards #111

During the null safety migration it was possible for
`StreamSubscription.cancel()` to return `null` in some cases. Now that
this package is only used with sound null safe code it isn't possible
for these nulls to flow through anymore so it is not necessary to filter
for them.
natebosch added a commit that referenced this issue Jun 18, 2024
Towards #111

During the null safety migration it was possible for
`StreamSubscription.cancel()` to return `null` in some cases. Now that
this package is only used with sound null safe code it isn't possible
for these nulls to flow through anymore so it is not necessary to filter
for them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants