pipe does not properly support rest parameters in some cases.

[benlesh]

I encountered this while updating Angular CLI.

In particular, the update to the base.ts file in there, where I'm adding as any was necessary because the spreading of an array of operators into pipe was not working.

EDIT: It seems that we can't support rest params without breaking inference enforcement from operator to operator in the pipe. So here's a proposal:

Proposed Feature

Add an overload of pipe that accepts OperatorFunction<any, any>[] and returns Observable<R>:

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

// usage

const operations = [map(x => x + x), filter(x => x < 100), mergeMap(n => timer(n))];

of(42).pipe(operations)

[kwonoj]

My memory's not clear, but didn't we replace those signature into else for some reason? 872b0ec#diff-eeb9d77f742ad0df7e932697062fc44e /cc @cartant

[cartant]

See #3841 - adding this signature essentially removes type checking. #3945 was the fix the problems that signature introduced.

[benlesh]

@kwonoj ... I think the issue is that TypeScript is not honoring line 321 there. It needs to be added to the signature.

[cartant]

Regarding spreading, see #3989

[benlesh]

Yup, I see the issue now. Adding a proposed change to the OP.

[jgbpercy]

I'm probably missing something obvious, but over the weekend I had a play around with making the args to all the Observable pipe overloads optional, and as far as I could see that had the effect of restoring pipe spread arg typing that is at least as good as it was pre v6.3 (and not causing a regression on #3841 )

All the dtslint tests seemed to pass, and this wouldn't require special-casing an array argument to pipe, so for eg I believe that Angular ...sinks wouldn't need a change. If this does break typing in some way I'm not seeing, I'd be up for adding a PR for dtslint tests that cover this?

So it would look like

  pipe(): Observable<T>;
  pipe<A>(op1?: OperatorFunction<T, A>): Observable<A>;
  pipe<A, B>(op1?: OperatorFunction<T, A>, op2?: OperatorFunction<A, B>): Observable<B>;
  pipe<A, B, C>(op1?: OperatorFunction<T, A>, op2?: OperatorFunction<A, B>, op3?: OperatorFunction<B, C>): Observable<C>;
  pipe<A, B, C, D>(op1?: OperatorFunction<T, A>, op2?: OperatorFunction<A, B>, op3?: OperatorFunction<B, C>, op4?: OperatorFunction<C, D>): Observable<D>;
  pipe<A, B, C, D, E>(op1?: OperatorFunction<T, A>, op2?: OperatorFunction<A, B>, op3?: OperatorFunction<B, C>, op4?: OperatorFunction<C, D>, op5?: OperatorFunction<D, E>): Observable<E>;
// and so on

I didn't have time to turn this into a PR, and I didn't really test properly under TS 2.8 (only 3.0), but I got as far as making these notes:

import { of, Observable, OperatorFunction } from 'rxjs';
import { mapTo, map, switchMap, switchAll } from 'rxjs/operators';

const basic = of(0);

// Works same either way (type breaks down past 9 args, as expected)
const nonSpread = basic.pipe(
  mapTo('1'),
  mapTo(2),
  mapTo('3'),
  mapTo(4),
  mapTo('5'),
  mapTo(6),
  mapTo('7'),
  mapTo(8),
  mapTo('9'),
  // mapTo(10), // if uncommented, type of nonSpread becomes Observable<{}> as expected
  // mapTo('11'),
  // mapTo(12),
);

// Not allowed currently, allowed with optional args, and types correctly.
const maps = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12].map(x => map<number, number>(y => y * x));
const pipedByMaps = basic.pipe(...maps);

// Not allowed currently, allowed with optional args, and types as Observable<string | number>,
// which is about as good as you can expect
const maps2 = [1, '2', 3, '4', 5, '6', 7, '8', 9, '10', 11, '12'].map(x => map(y => x));
const pipedByMaps2 = basic.pipe(...maps2);

// In the general case, when spreading an array with various types of OperatorFunctions, the
// OperatorFunction type params can be the most specific common type, i.e. the best possible
// union type, that encompasses all input and output types.
// Obviously, this is less useful than the precise typing of pipe with less than 10 non-spread args.
interface WithId { id: number; }
type BestUnion = string | number | WithId;
const variousOperatorsArray: OperatorFunction<BestUnion, BestUnion>[] = [
  mapTo('1'),
  mapTo(2),
  mapTo({ id: 3 })
];
const pipedByVariousArray = basic.pipe(...variousOperatorsArray);

// If the "array" is typed as a tuple, however, then the correct type will result (under TS 3.x)
const variousOperatorsTuple: [
  OperatorFunction<number, string>,
  OperatorFunction<string, number>,
  OperatorFunction<number, WithId>
] = [mapTo('1'), mapTo(2), mapTo({ id: 3 })];
const pipedByVariousTuple = basic.pipe(...variousOperatorsTuple);

// This is allowed with or without optional args with non-strict TS. Believe it is currently an
// error with strictNullChecks TS, and would not be with optional args, but it's not a mistake
// that seems worth worrying too much about.
const pipedByUndefined = basic.pipe(
  undefined,
);

// While the above is an unlikely mistake, the following is a semi-plausible mistake that would
// not error under strictNullChecks
// But it's not currently possible at all without optional args, so you don't really lose much
const arrayThatCameFromSomewhere = [map(x => x.toString()), switchMap(x => of(x), undefined)];
const pipedBySpreadWithUndefined = basic.pipe(...arrayThatCameFromSomewhere);

// From https://github.com/ReactiveX/rxjs/issues/3841
// Still has hoped-for error with or without optional args
const input: Observable<number> = of(1, 2, 3);
const output = input.pipe(switchAll()); // Has expected error: Type 'number' is not assignable to type 'ObservableInput<{}>'.
output.subscribe(value => console.log(value));

[cartant]

IMO, adding an array overload signature would be the preferred solution. It's simple and easy to understand and will effect errors for undefined values, etc.

And TypeScript 3.0 supports using the spread syntax in a fully type-safe manner. So eventually this will be a non-issue.

[cartant]

Also, I'm not entirely sure why making them optional should effect such different behaviour. It's interesting, but I'd have some concerns that the behaviour could change. Especially, if the reason for it isn't easily explained.

[qm3ster]

Maybe

X.pipe(
  ...some_adapters,
  an_adapter(),
  ...some_more_adapters,
)

should be currently solved by

X.pipe(
  pipeFromArray(some_adapters),
  an_adapter(),
  pipeFromArray(some_more_adapters),
)

?
Would the performance be atrocious?