Authorise different returned type for recoverWith

[AugustinLF]

Summary

Hi folks, Currently the TS definitions of recoverWith expect the recovered stream to be of the same type of the one that errors. This feels off, since we'll often see patterns when recovering from error could for instance mean that we want a type signalling the error. This is especially true if you use a union of returned value, mapping to one of the values will prevent you to recover with another one.

For instance in my case, using redux-most:

type Action =
  | FetchSuccess
  | FetchFailure

const stream: Stream<Action> =
  most
      .fromPromise(fetchData())
      .map(response =>fetchSuccess(response.data))
      .recoverWith((error: Error) => most.of(fetchError(error)))
)

This will error, stating that Stream<FetchFailure> (returned by fetchError) is incompatible with Stream<FetchSucess> (returned by fetchSuccess). But in my case, what matters is that both are compatible with Stream<Action>. I can temporarily fix this by annotating .map((response): Action =>fetchSuccess(response.data)), but that should probably not be necessary.

I've seen that the typings seems to be the same in @most/core, so if you'd want me to also update them, let me know.

I didn't see any types tests, and the recoverWith don't say anything about the type of the returned stream, so I don't think that should update anything else but the definitions file.

Todo

• [X] Unit tests for new or changed APIs and/or functionality
• [X] Documentation, including examples

[briancavalier]

Hey @AugustinLF, thanks for the PR, and thanks for including a real example! We're definitely open to this idea both in most 1.x and in @most/core.

I think the return type will need to be a union, though, similar to @most/core's merge. To explain why it will need to be a union, consider this scenario:

const s1 = /* stream of number */
const s2 = s1.recoverWith(e => of('hello'))

Imagine if s1 produces 3 number events and then fails. Clearly, the type of s2 must be number | string.

I think this actually fits perfectly with your original example, since you'd naturally end up with Stream<FetchSuccess | FetchError> which is exactly Stream<Action>.

Did that make sense?

[AugustinLF]

Thanks for the quick answer @briancavalier, you're indeed right, I indeed only considered recoverWith as the final instruction on a stream, and not in the case where it would actually not recover.

I added a commit to switch to a union type.

BTW the build seem to fail for unrelated reasons.

[briancavalier]

Released in most 1.8.0.

[vlinder]

I think this is what broke our typings where we used recoverWith(empty). It could no longer infer the type in the stream.