Rxjs: switchAll, mergeAll etc are not type safe

Created on 14 Jun 2018  路  5Comments  路  Source: ReactiveX/rxjs

Bug Report

Current Behavior

const input: Observable<number> = of(1, 2, 3)
const output = input.pipe(switchAll())
output.subscribe(value => console.log(value))

No type error, but runtime error:

You provided '1' where a stream was expected. You can provide an Observable, Promise, Array, or Iterable.

Reproduction

Expected behavior
This should give a type error like number is not assignable to ObservableInput<T>

Environment

  • RxJS version: 6.2.1

Possible Solution
I tried a few things, but it seems like type inference just fails here. TypeScript needs to infer a type based on the _usage_ of the return value here.

Workaround
Use switchMap(x => x) instead (switchMap(identity) also breaks inference)

TS types

Most helpful comment

Overall, the typing around the pipe method has been perpetually problematic.

Perhaps we can get @ahejlsberg or @mhegazy's attention and this can be solved either by helping us get it right, or with some solution to the problem on the TypeScript side of things.

All 5 comments

The problem seems to have been introduced in 6.2.1 with this commit - which uses the signature that I suggested.

Prior to that commit, the snippet would have effected this error:

[ts]
Argument of type 'OperatorFunction<ObservableInput<{}>, {}>' is not assignable to parameter of type 'OperatorFunction<number, {}>'.
  Type 'number' is not assignable to type 'ObservableInput<{}>'.

The commit fixed an error with the pipe signature, but it could be improved. A more refined rest-parameters signature for pipe would be:

pipe<R>(op1: OperatorFunction<T, any>, ...operations: OperatorFunction<any, any>[]): Observable<R>;

Such a signature would avoid matching the first parameter with any, but, unfortunately, that's as far as this approach could be taken. It would not solve the problem if switchAll where, say, the second of two parameters. For that situation, a signature like this would be required:

pipe<A, R>(op1: OperatorFunction<T, A>, op2: OperatorFunction<A, any>, ...operations: OperatorFunction<any, any>[]): Observable<R>;

However, such a signature requires the inclusion of another type parameter. And given that the caller of such a rest-parameter signature will want to explicitly specify R, the signature isn't really usable.

I don't know if it's going to be possible to support a rest-parameters signature with which the caller can specify only the result type (R) and proper type safety for the parameters.


As an aside, this is a regression that would have been caught with an appropriate dtslint test - see #3823. Something like this would have caught it:

const input: Observable<number> = of(1, 2, 3);
const output = input.pipe(switchAll()); // $ExpectError

I don't like the idea of degrading type safety for common cases like this just to support more than n parameters to pipe(). FWIW, the TypeScript compiler cheats in many places itself, e.g. will only check circular types n levels deep (7 I think). I would rather have pipe() only take a maximum of n parameters (let's say 10) than have type safety silently degrade. A user can always call pipe() twice.

That said, would https://github.com/Microsoft/TypeScript/pull/24897 eliminate the need for all the overloads? We should definitely try that out

I've seen the TypeScript PR that you've referenced and looking more closely at it is on my TODO list. However, I suspect that it won't help in the pipe case because of the input-output relationship that exists between consecutive parameters. Hopefully, I'm wrong.

My preferred solution is to abandon the only-specifiy-R signature and add another signature that appends the rest parameter to a parameters-up-to-I signature.

That is, remove this:

pipe<R>(
  ...operations: OperatorFunction<any, any>[]
): Observable<R>;

And add this:

pipe<A, B, C, D, E, F, G, H, I, R>(
  op1: OperatorFunction<T, A>,
  op2: OperatorFunction<A, B>,
  op3: OperatorFunction<B, C>,
  op4: OperatorFunction<C, D>,
  op5: OperatorFunction<D, E>,
  op6: OperatorFunction<E, F>,
  op7: OperatorFunction<F, G>,
  op8: OperatorFunction<G, H>,
  op9: OperatorFunction<H, I>,
  ...operations: OperatorFunction<any, any>[]
): Observable<R>;

Instead of specifying a single R type parameter, callers would have to apply a type assertion, like this:

const result = of("T").pipe(
    mapTo("A"),
    mapTo("B"),
    mapTo("C"),
    mapTo("D"),
    mapTo("E"),
    mapTo("F"),
    mapTo("G"),
    mapTo("H"),
    mapTo("I"),
    mapTo("R")
) as Observable<"R">; // Otherwise inferred to be Observable<{}>

Overall, the typing around the pipe method has been perpetually problematic.

Perhaps we can get @ahejlsberg or @mhegazy's attention and this can be solved either by helping us get it right, or with some solution to the problem on the TypeScript side of things.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

benlesh picture benlesh  路  3Comments

Agraphie picture Agraphie  路  3Comments

cartant picture cartant  路  3Comments

cartant picture cartant  路  3Comments

unao picture unao  路  4Comments