From f6a4951a5fc5e1f9dce1a7e2127f12ce1ec9654d Mon Sep 17 00:00:00 2001 From: Aluan Haddad Date: Mon, 3 Apr 2017 15:51:40 -0400 Subject: [PATCH] fix(reduce): proper TypeScript signature overload ordering (#2382) * 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. --- spec/operators/reduce-spec.ts | 69 +++++++++++++++++++++++++++++++++-- src/operator/reduce.ts | 4 +- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/spec/operators/reduce-spec.ts b/spec/operators/reduce-spec.ts index 1c228343d5..427342a77e 100644 --- a/spec/operators/reduce-spec.ts +++ b/spec/operators/reduce-spec.ts @@ -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; + const reduced = a.reduce((acc, cur) => { + console.log(acc); + acc.push(cur.toString()); + return acc; + }, [] as string[]); + + reduced.subscribe(rs => { + rs[0].toLowerCase(); + }); }); }); -}); \ No newline at end of file +}); diff --git a/src/operator/reduce.ts b/src/operator/reduce.ts index 10b98b62b2..aefb943955 100644 --- a/src/operator/reduce.ts +++ b/src/operator/reduce.ts @@ -3,9 +3,9 @@ import { Operator } from '../Operator'; import { Subscriber } from '../Subscriber'; /* tslint:disable:max-line-length */ +export function reduce(this: Observable, accumulator: (acc: T[], value: T, index: number) => T[], seed: T[]): Observable; export function reduce(this: Observable, accumulator: (acc: T, value: T, index: number) => T, seed?: T): Observable; -export function reduce(this: Observable, accumulator: (acc: T[], value: T, index: number) => T[], seed?: T[]): Observable; -export function reduce(this: Observable, accumulator: (acc: R, value: T, index: number) => R, seed?: R): Observable; +export function reduce(this: Observable, accumulator: (acc: R, value: T, index: number) => R, seed: R): Observable; /* tslint:enable:max-line-length */ /**