From 7cee02653cd6a3e93db5c9db7e9c47fdc5c17eec Mon Sep 17 00:00:00 2001 From: Aluan Haddad Date: Fri, 17 Feb 2017 12:35:58 -0500 Subject: [PATCH 1/4] fix(reduce): adjust overload ordering Fixes #2338 --- src/operator/reduce.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/operator/reduce.ts b/src/operator/reduce.ts index 10b98b62b2..6c342fd1e8 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: R, value: T, index: number) => R, seed?: R): 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; /* tslint:enable:max-line-length */ /** From 28b3ea9f79120157e00d54e6cd77825bab8f029c Mon Sep 17 00:00:00 2001 From: Aluan Haddad Date: Fri, 17 Feb 2017 14:21:42 -0500 Subject: [PATCH 2/4] Make seed required when not of type T --- src/operator/reduce.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/operator/reduce.ts b/src/operator/reduce.ts index 6c342fd1e8..050db1a014 100644 --- a/src/operator/reduce.ts +++ b/src/operator/reduce.ts @@ -3,8 +3,8 @@ import { Operator } from '../Operator'; import { Subscriber } from '../Subscriber'; /* tslint:disable:max-line-length */ -export function reduce(this: Observable, accumulator: (acc: R, value: T, index: number) => R, seed?: R): 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: 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; /* tslint:enable:max-line-length */ From ae156d76aa0119011b3bf391f6f416d761b5a1e9 Mon Sep 17 00:00:00 2001 From: Aluan Haddad Date: Mon, 20 Feb 2017 23:23:02 -0500 Subject: [PATCH 3/4] 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. --- spec/operators/reduce-spec.ts | 69 +++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 4 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 +}); From 99fd39d636b2d75100f475a5cf1f3350f588bd15 Mon Sep 17 00:00:00 2001 From: Aluan Haddad Date: Tue, 21 Feb 2017 00:11:50 -0500 Subject: [PATCH 4/4] 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. --- src/operator/reduce.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/operator/reduce.ts b/src/operator/reduce.ts index 050db1a014..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: R, value: T, index: number) => R, seed: R): 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; /* tslint:enable:max-line-length */ /**