Reduce with array input defaults the return type to an array of arrays

[PrimalZed]

RxJS version: 5.3.0

Code to reproduce:

Observable.of([0, 1, 2, 3],[4, 5, 6, 7])
.reduce((acc, value) => acc.concat(value), [])
.map(numbers => numbers.map(n => n++))
.subscribe(value => console.log(JSON.stringify(value)));

Expected behavior: Can take an input type T[] and return type T[] without specifying typing.

Actual behavior: On input type T[], defaults return type T[][].

Additional information:
This was introduced with the fix to #2382
My current workaround is to simply specify the types, e.g. .reduce<number[], number[]>(...)

[kwonoj]

/cc @aluanhaddad for visibility.

[aluanhaddad]

@PrimalZed The issue you are experiencing comes from the type of of, not from reduce. The reduce issue, where all seed parameters were optional, with Array<ObservableElementType> taking precedence was probably hiding the underlying issue.
For reference, here is the error I receive.

[PrimalZed]

@aluanhaddad the type from of was intentional to be Observable<number[]>. The reduce is to combine all the arrays returned by the observable into a single array, with Array.prototype.concat.

That error is the problem I'm experiencing, yes. The return type of reduce is Observable<number[][]> rather than the desired Observable<number[]>.

Again, it's easily resolved by specifying some typing somewhere, but it seems like it should be able to pick it up. Maybe a type definition of reduce<T>(this: Observable<T[]>, accumulator: (acc: T[], value: T[], index: number) => T[], seed: T[]): Observable<T[]> should be added? Or is this use case just too fringe to deserve a type definition to support it?

[aluanhaddad]

@PrimalZed I see what you are getting at now. I hadn't noticed where map was being called when I looked earlier. The problem is actually that the wrong signature has now taking priority for nested arrays when the seed is an empty array.

TLDR: your use case is covered by the general reduce<T, R> but this is no longer being selected because of the array specialization. I think the specialization needs to be removed.

In other words the following typechecks

import { Observable } from 'rxjs/Observable';

Observable.of([0, 1, 2, 3], [4, 5, 6, 7])
  .reduce((acc, value) => acc.concat(value), [0])
  .map(numbers => numbers.map(n => n++))
  .subscribe(value => console.log(JSON.stringify(value)));

while this, your example, does not

import { Observable } from 'rxjs/Observable';

Observable.of([0, 1, 2, 3], [4, 5, 6, 7])
  .reduce((acc, value) => acc.concat(value), [])
  .map(numbers => numbers.map(n => n++))
  .subscribe(value => console.log(JSON.stringify(value)));

but it should.

Removing the specialization

function reduce<T>(
    this: Observable<T>,
    accumulator: (acc: T[], value: T, index: number) => T[],
    seed: T[]
): Observable<T[]>;

actually fixes type inference for your use case, but it may break others. I do not understand why the overload was even provided in the first place.

For reference, it used to be specified as

function reduce<T>(
    this: Observable<T>,
    accumulator: (acc: T[], value: T, index: number) => T[],
    seed?: T[]
): Observable<T[]>;

which was definitely wrong.

The overload actually selected by your example in previous version was

function reduce<T>(
    this: Observable<T>,
    accumulator: (acc: T, value: T, index: number) => T,
    seed?: T
): Observable<T>;

which is still present but the Observable<T[]> specific overload taking a required array is now a better candidate.

I think it should be removed altogether but I do not understand why it was added which is why I did not remove it.

[david-driscoll]

The overload was added so that common case of reduce to an array of T would be easily supported without explicit casting. It looks like in this case however it falls over.

// without the overload
Observable.of(0, 1, 2, 3)
  .reduce((acc, value) => acc.concat(value), []); // [] would be {}[] and would require a cast to T[] to work correctly.

It appears that reducing the precedence of the array variant we can get things to work. See the playground here or the code below. All that was done was that reduce<T>(this: Observable<T>, accumulator: (acc: T[], value: T, index: number) => T[], seed: T[]): Observable<T[]>; was moved down one line.

interface ObservableStatic {
    of<T>(...args: T[]): Observable<T>;
}
interface Observable<T> {
    map<T, R>(this: Observable<T>, project: (value: T, index: number) => R, thisArg?: any): Observable<R>;
    subscribe(fn: Function);
    reduce<T>(this: Observable<T>, accumulator: (acc: T, value: T, index: number) => T, seed?: T): Observable<T>;
    reduce<T>(this: Observable<T>, accumulator: (acc: T[], value: T, index: number) => T[], seed: T[]): Observable<T[]>;
    reduce<T, R>(this: Observable<T>, accumulator: (acc: R, value: T, index: number) => R, seed: R): Observable<R>;
}

let Observable: ObservableStatic;

Observable.of([0, 1, 2, 3], [4, 5, 6, 7])
    .reduce((acc, value) => acc.concat(value), [])
    .map(numbers => numbers.map(n => n++))
    // .subscribe(value => console.log(JSON.stringify(value)));

Observable.of(0, 1, 2, 3, 4, 5, 6, 7)
    .reduce((acc, value) => { acc.push(value); return acc; }, [])
    .map(numbers => numbers.map(n => n++))
    // .subscribe(value => console.log(JSON.stringify(value)));

[PrimalZed]

Thanks @david-driscoll, I was wondering if the type definitions worked as a "pick first matching" sort of thing. So all we have to do is change the order of the type definitions.

[aluanhaddad]

@david-driscoll thanks that explains it. Otherwise you need to write [] as number[] to make your example typecheck under --noImplicitAny

@PrimalZed Essentially it picks the first matching overload yes.

[kwonoj]

closed via #2523.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.