ReactiveX / rxjs

A reactive programming library for JavaScript
https://rxjs.dev
Apache License 2.0
30.6k stars 3k forks source link

forkJoin should emit empty array or object given empty array or object as input #5209

Open aquark opened 4 years ago

aquark commented 4 years ago

I'm reopening https://github.com/ReactiveX/rxjs/issues/2816 because the response wasn't satisfactory.

RxJS version: 6.5.2

Code to reproduce:

forkJoin([]).pipe(
  tap(values => console.log(values)),
).subscribe()

Expected behavior:

Console logs []

Actual behavior:

Nothing is logged.

Additional information:

Compare the behavior of Promise.all:

Promise.all([]).then(values => console.log(values)) // logs []

This behavior is important for reasoning inductively and for composability. For example, say I have a list of search terms to expand to search results and then I want to concatenate all the results. The natural way to write this is:

forkJoin(searchTerms.map(term => getSearchResults(term))).pipe(
  map(allResults => allResults.flat()),
)

You would expect an empty list of search terms to produce an empty list of results, however with current forkJoin behavior you need to handle that with a special case:

forkJoin(searchTerms.map(term => getSearchResults(term))).pipe(
  map(allResults => allResults.flat()),
  defaultIfEmpty(new Array<SearchResult>()),
)

I believe that in every use case where the length of the input to forkJoin is unknown at compile time, it's simpler for the consuming code if forkJoin returns empty output on empty input, so the consuming code doesn't need to treat empty input as a special case; it can iterate over the output value the same way regardless. This is an example of the null object pattern, where for example it's better for an operation which produces a list to produce an empty list in case there is no output rather than a null value which must be handled specially.

In the original issue https://github.com/ReactiveX/rxjs/issues/2816 it was argued that "Forkjoin doesn't consider any other case except all sources emits value, so empty array is also short-curcuit to completion immediately." However when there are no sources, it is logically true that "every source emits a value", and it is not true that "some source doesn't emit a value" (see the behavior of Array.every and Array.some for example), so one would therefore expect forkJoin to emit a value.

This reasoning also applies to forkJoin({}), which should emit an empty object {}.

startswithaj commented 4 years ago

Yeah, this is a recurring problem for us when using combineLatest. We've implemented our own wrapper function.

For details on our use case see:

https://stackblitz.com/edit/rxjs-playground-royl-vxadk4?file=index.ts

cartant commented 4 years ago

zip needs the same consideration, too.

rraziel commented 3 years ago

@cartant I'm looking at combineLatest and zip for #5066 Has this new behavior been validated? So I know if I can make those changes too