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

Various inconsistencies around sync/async in TryStreamExt and StreamExt #2518

Closed
farnz opened this issue Nov 19, 2021 · 1 comment
Closed
Labels
A-stream Area: futures::stream C-feature-request

Comments

@farnz
Copy link

farnz commented Nov 19, 2021

This is related to #2470 but has some complexity because it's Streams, not Futures. It also touches on the same area as #2237 but has more reasoning.

For StreamExt, we want every combinator to somehow exist in two forms - map and then are a good example of the two forms - where one form is sync like map (FnMut(Self::Item) -> T) and the other is async like then (FnMut(Self::Item) -> impl Future). Where we only have one form, we can use an async { ... } block to enable us to use the async form for a sync call.

Looking at the documentation for StreamExt, it's only map and then that provide this duality - futures lacks similar duality for filter, filter_map, flat_map, fold, any, all, scan, skip_while, take_while, take_until, for_each and for_each_concurrent. I think it would be worth considering whether we want to have the dual sync/async forms for every one of these, whether map and then are different because they are special, or whether we should consider losing map and just having then.

TryStreamExt is more complicated. As well as the sync/async duality, we also have three possible input types (Self::Item, Self::Item::Ok and Self::Item::Err) and three possible return types (Result<T, E>, T and E). This leads to us needing 18 variants to cover all the options for each of the 12 combinators that StreamExt provides (counting for_each as a special case of for_each_concurrent. I don't think that providing 216 combinators blindly is a good idea, which leaves us wanting to think about what subset of the 216 is needed.

Talking to co-workers, two that are most often wanted are a then_ok to be the async dual of map_ok, and something to be the sync dual of try_filter_map. We've also had a little surprise because try_filter_map is actually a combination of and_then and try_filter, not of map or map_ok and try_filter, which led to annoyances around types. #2375 has other people being surprised by try_filter having an unexpected return type, too, where they wanted one of the other 18 choices that could have been made.

@taiki-e
Copy link
Member

taiki-e commented Jun 18, 2023

Closing in favor of #2755.

@taiki-e taiki-e closed this as completed Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stream Area: futures::stream C-feature-request
Projects
None yet
Development

No branches or pull requests

2 participants