Skip to content

Commit

Permalink
fix(reduce): proper TypeScript signature overload ordering (#2382)
Browse files Browse the repository at this point in the history
* fix(reduce): adjust overload ordering

Fixes #2338

* Make seed required when not of type T

* Added specs for new and old behavior of reduce overloads and fixed failing specs

This adds tests for both existing behavior, under the old overload, and the new behavior enabled by the new overload signatures of the `reduce` operator.
Not that the specific change to the following test case `'should accept T typed reducers'` that removes the seed value. If a seed value is specified, then the reduction is always from `T` to `R`. This was necessary to make the test pass, but also models the expected behavior more correctly.

The cases for `R` where `R` is not assignable to `T` are covered by new tests added in the commit.

Additionally, I have added additional verification to all of the tests touched by this change to ensure that the values returned are usable as their respective expected types.

* Fix ordering as allowed by newly required seeds to fix failing specs

Fix ordering as allowed by newly required seeds to fix failing tests.
I think there is a good argument to be made that the failing tests were failing correctly, as this is how `Array.prototype.reduce` behaves, but as I have made the seed arguments required, for `R` typed reducers, re-ordering the overloads impacts their specificity allowing, the current behavior, correct or otherwise, to be retained.
  • Loading branch information
aluanhaddad authored and benlesh committed Apr 3, 2017
1 parent 28d0883 commit f6a4951
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 6 deletions.
69 changes: 65 additions & 4 deletions spec/operators/reduce-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,24 +301,85 @@ describe('Observable.prototype.reduce', () => {
});

it('should accept T typed reducers', () => {
type(() => {
let a: Rx.Observable<{ a: number; b: string }>;
const reduced = a.reduce((acc, value) => {
value.a = acc.a;
value.b = acc.b;
return acc;
});

reduced.subscribe(r => {
r.a.toExponential();
r.b.toLowerCase();
});
});
});

it('should accept R typed reducers when R is assignable to T', () => {
type(() => {
let a: Rx.Observable<{ a?: number; b?: string }>;
a.reduce((acc, value) => {
const reduced = a.reduce((acc, value) => {
value.a = acc.a;
value.b = acc.b;
return acc;
}, {});

reduced.subscribe(r => {
r.a.toExponential();
r.b.toLowerCase();
});
});
});

it('should accept R typed reducers when R is not assignable to T', () => {
type(() => {
let a: Rx.Observable<{ a: number; b: string }>;
const seed = {
as: [1],
bs: ['a']
};
const reduced = a.reduce((acc, value) => {
acc.as.push(value.a);
acc.bs.push(value.b);
return acc;
}, seed);

reduced.subscribe(r => {
r.as[0].toExponential();
r.bs[0].toLowerCase();
});
});
});

it('should accept R typed reducers', () => {
it('should accept R typed reducers and reduce to type R', () => {
type(() => {
let a: Rx.Observable<{ a: number; b: string }>;
a.reduce<{ a?: number; b?: string }>((acc, value) => {
const reduced = a.reduce<{ a?: number; b?: string }>((acc, value) => {
value.a = acc.a;
value.b = acc.b;
return acc;
}, {});

reduced.subscribe(r => {
r.a.toExponential();
r.b.toLowerCase();
});
});
});

it('should accept array of R typed reducers and reduce to array of R', () => {
type(() => {
let a: Rx.Observable<number>;
const reduced = a.reduce((acc, cur) => {
console.log(acc);
acc.push(cur.toString());
return acc;
}, [] as string[]);

reduced.subscribe(rs => {
rs[0].toLowerCase();
});
});
});
});
});
4 changes: 2 additions & 2 deletions src/operator/reduce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { Operator } from '../Operator';
import { Subscriber } from '../Subscriber';

/* tslint:disable:max-line-length */
export function reduce<T>(this: Observable<T>, accumulator: (acc: T[], value: T, index: number) => T[], seed: T[]): Observable<T[]>;
export function reduce<T>(this: Observable<T>, accumulator: (acc: T, value: T, index: number) => T, seed?: T): Observable<T>;
export function reduce<T>(this: Observable<T>, accumulator: (acc: T[], value: T, index: number) => T[], seed?: T[]): Observable<T[]>;
export function reduce<T, R>(this: Observable<T>, accumulator: (acc: R, value: T, index: number) => R, seed?: R): Observable<R>;
export function reduce<T, R>(this: Observable<T>, accumulator: (acc: R, value: T, index: number) => R, seed: R): Observable<R>;
/* tslint:enable:max-line-length */

/**
Expand Down

0 comments on commit f6a4951

Please sign in to comment.