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

feat(combineLatest): add N-args signature for observable inputs #5488

Merged

Conversation

ggradnig
Copy link
Contributor

Description:
Adds a type signature to provide an array with more than six elements to combineLatest without losing the order of the input observable types. We recently had this problem at a client project and needed to resort to nesting combineLatest.

The solution makes use of TypeScript readonly tuples. They allow for an array [a, b] to have the type [A, B] instead of (A | B)[]. With this type, we can make use of ObservedValueTupleFromArray to get all the observed types of the tuple in the correct order.

This comes with the "drawback" that the input array must be declared as const, else TypeScript assumes a union type per default. It would work with spreading syntax though like in combineLatestWith, but this signature was deprecated for combineLatest.

For this to work, the conditional type ObservedValueTupleFromArray must check for the narrow readonly T[] type, but this shouldn't be an issue as T[] extends it.

Related issue:
#5066

observable/combineLatest.ts - needs N-args signature for observable inputs? See also #5209

#4410

When using combineLatest with an array of more than six
observable inputs, the result observable has a union type
of all observed types. Now, by using the 'as const' assertion,
the output observable has a correct tuple type that mirrors the
input observables.
@ggradnig ggradnig force-pushed the feature/n-args-for-combine-latest branch from b910edb to ab60906 Compare June 13, 2020 17:13
@cartant cartant self-requested a review June 16, 2020 01:02
@rraziel
Copy link
Contributor

rraziel commented Jun 26, 2020

@ggradnig this one could be useful to you as well: #5363

@ggradnig
Copy link
Contributor Author

ggradnig commented Jun 26, 2020

Yeah thanks for the hint, but this pretty much already exists in https://github.com/cartant/rxjs-etc
The reason I can't use that here is that I want to spread the result array directly into another function. Can't do that easily with objects...
This PR just adds a type signature without any runtime changes to the existing combineLatest

@benlesh benlesh merged commit fcc47e7 into ReactiveX:master Sep 2, 2020
@benlesh
Copy link
Member

benlesh commented Sep 2, 2020

Thank you @ggradnig!

@ggradnig ggradnig deleted the feature/n-args-for-combine-latest branch September 2, 2020 15:38
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

Successfully merging this pull request may close these issues.

3 participants