Skip to content

feat: Add allValuesFrom function #6774

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Wyzard256
Copy link
Contributor

Description:

This PR adds an allValuesFrom function that's similar to firstValueFrom and lastValueFrom, but the promise resolves to an array of all the values emitted by the observable, instead of just the first or last. (Unlike firstValueFrom and lastValueFrom, it's not an error if the observable completes without emitting anything: the promise just resolves to an empty array in that case.)

The implementation is a trivial combination of the toArray operator with either firstValueFrom or lastValueFrom (they're equivalent when used with toArray), and applications can easily accomplish the same thing directly by using an expression like firstValueFrom(anObservable.pipe(..., toArray())), but the interaction between the two might not be clear to a reader — especially since the toArray() could be at the end of a lengthy multi-line pipe, distant from the firstValueFrom. Putting the combination into a function named allValuesFrom makes it obvious what the result will be. And although applications can easily implement the function themselves, it's closely related to firstValueFrom and lastValueFrom so I think it makes sense for the library to provide it.

The jsdoc and tests are adapted from the ones for firstValueFrom and lastValueFrom, so similar in structure.

This is similar to firstValueFrom and lastValueFrom, but the promise
resolves to an array of all the values emitted by the observable,
instead of just the first or last.  The implementation is trivial and
applications can easily get the same effect by directly using toArray
with firstValueFrom/lastValueFrom, but a named function makes it more
clear to a reader what the combination does.
@kwonoj
Copy link
Member

kwonoj commented Jan 15, 2022

Same as prior feature implementation requests (#5939 (comment), #5089, and others)

Core team strongly suggest to create new features via userland custom operators instead of including it in core as we generally trying to reduce core api surface, also with pipe it is straightforward to create a new operator as separate library. If a new features should require core lib change we'd like to ask to have some details what part of core lib change is required & usecases it should be included in core.

@kwonoj kwonoj added the blocked label Jan 15, 2022
@Wyzard256
Copy link
Contributor Author

Makes sense for totally-new features, but this is more like a small extension to a feature that's already present. It'd seem weird to have allValuesFrom live in a different library than firstValueFrom and lastValueFrom since they're so similar and related.

(Applications can easily implement this function directly, though, so it's not a big problem to have it absent from the library.)

@kwonoj
Copy link
Member

kwonoj commented Jan 15, 2022

this is more like a small extension to a feature that's already present

We actually started to deprecate some of those extensions even. For example mapTo is deprecated on behalf of map. Our general recommendation is if a feature can be implemented trivially via existing operator it'd better to be used in each app / or userland. The reason core api surface is not changing rapidly is since we already have user's relying on those features which makes deprecation / breaking changes really hard. That's also one reason core team attempts to not to add new api surface for the features can be composed with existing operators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants