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

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

Open
aquark opened this issue Jan 2, 2020 · 3 comments

Comments

@aquark
Copy link

aquark commented Jan 2, 2020

I'm reopening #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 #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
Copy link

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
Copy link
Collaborator

cartant commented Feb 19, 2020

zip needs the same consideration, too.

@rraziel
Copy link
Contributor

rraziel commented Dec 7, 2020

@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

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 a pull request may close this issue.

4 participants