Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(scan): fixed declarations to properly support different return types #4598

Merged
merged 1 commit into from May 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions compat/operator/reduce.ts
Expand Up @@ -2,9 +2,9 @@ import { Observable } from 'rxjs';
import { reduce as higherOrderReduce } from 'rxjs/operators';

/* 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, R>(this: Observable<T>, accumulator: (acc: R, value: T, index: number) => R, seed: R): Observable<R>;
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): Observable<R>;
/* tslint:enable:max-line-length */

/**
Expand Down Expand Up @@ -51,7 +51,7 @@ export function reduce<T, R>(this: Observable<T>, accumulator: (acc: R, value: T
* @method reduce
* @owner Observable
*/
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: T | R, value: T, index?: number) => (T | R), seed?: R): Observable<T | R> {
// providing a seed of `undefined` *should* be valid and trigger
// hasSeed! so don't use `seed !== undefined` checks!
// For this reason, we have to check it here at the original call site
Expand All @@ -61,5 +61,5 @@ export function reduce<T, R>(this: Observable<T>, accumulator: (acc: R, value: T
return higherOrderReduce(accumulator, seed)(this);
}

return higherOrderReduce(accumulator)(this);
return higherOrderReduce<T, T | R>(accumulator)(this);
}
12 changes: 6 additions & 6 deletions compat/operator/scan.ts
Expand Up @@ -3,9 +3,9 @@ import { Observable } from 'rxjs';
import { scan as higherOrderScan } from 'rxjs/operators';

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

/**
Expand Down Expand Up @@ -46,10 +46,10 @@ export function scan<T, R>(this: Observable<T>, accumulator: (acc: R, value: T,
* @owner Observable
*/
export function scan<T, R>(this: Observable<T>,
accumulator: (acc: T | Array<T> | R, value: T, index: number) => T | Array<T> | R,
seed?: T | R): Observable<R> {
accumulator: (acc: T | R, value: T, index: number) => T | R,
seed?: T | R): Observable<T | R> {
if (arguments.length >= 2) {
return higherOrderScan(accumulator, seed)(this) as Observable<R>;
return higherOrderScan(accumulator, seed)(this);
}
return higherOrderScan(accumulator)(this) as Observable<R>;
return higherOrderScan<T, T | R>(accumulator)(this);
}
14 changes: 13 additions & 1 deletion spec-dtslint/operators/reduce-spec.ts
Expand Up @@ -15,5 +15,17 @@ it('should infer correctly for accumulator of type array', () => {

it('should accept seed parameter of the same type', () => {
const a = of(1, 2, 3).pipe(reduce((x, y, z) => x + 1, 5)); // $ExpectType Observable<number>
const b = of(1, 2, 3).pipe(reduce((x, y, z) => x + 1, '5')); // $ExpectError
const b = of(1, 2, 3).pipe(reduce((x, y, z) => x + 1, [])); // $ExpectError
});

it('should accept seed parameter of the seed array type', () => {
const a = of(1, 2, 3).pipe(reduce((x, y, z) => { x.push(y); return x; }, [4])); // $ExpectType Observable<number[]>
// Array must be typed...
const b = of(1, 2, 3).pipe(reduce((x, y, z) => { x.push(y); return x; }, [])); // $ExpectError
});

it('should accept seed parameter of a different type', () => {
const a = of(1, 2, 3).pipe(reduce((x, y, z) => x + '1', '5')); // $ExpectType Observable<string>
const bv: { [key: string]: string } = {};
const b = of(1, 2, 3).pipe(reduce((x, y, z) => ({ ...x, [y]: y.toString() }), bv)); // $ExpectType Observable<{ [key: string]: string; }>
});
31 changes: 31 additions & 0 deletions spec-dtslint/operators/scan-spec.ts
@@ -0,0 +1,31 @@
import { of, Observable } from 'rxjs';
import { scan } from 'rxjs/operators';

it('should enforce parameter', () => {
const a = of(1, 2, 3).pipe(scan()); // $ExpectError
});

it('should infer correctly ', () => {
const a = of(1, 2, 3).pipe(scan((x, y, z) => x + 1)); // $ExpectType Observable<number>
});

it('should infer correctly for accumulator of type array', () => {
const a = of(1, 2, 3).pipe(scan((x: number[], y: number, i: number) => x, [])); // $ExpectType Observable<number[]>
});

it('should accept seed parameter of the same type', () => {
const a = of(1, 2, 3).pipe(scan((x, y, z) => x + 1, 5)); // $ExpectType Observable<number>
const b = of(1, 2, 3).pipe(scan((x, y, z) => x + 1, [])); // $ExpectError
});

it('should accept seed parameter of the seed array type', () => {
const a = of(1, 2, 3).pipe(scan((x, y, z) => { x.push(y); return x; }, [4])); // $ExpectType Observable<number[]>
// Array must be typed...
const b = of(1, 2, 3).pipe(scan((x, y, z) => { x.push(y); return x; }, [])); // $ExpectError
});

it('should accept seed parameter of a different type', () => {
const a = of(1, 2, 3).pipe(scan((x, y, z) => x + '1', '5')); // $ExpectType Observable<string>
const bv: { [key: string]: string } = {};
const b = of(1, 2, 3).pipe(scan((x, y, z) => ({ ...x, [y]: y.toString() }), bv)); // $ExpectType Observable<{ [key: string]: string; }>
});
4 changes: 2 additions & 2 deletions spec/operators/reduce-spec.ts
Expand Up @@ -292,7 +292,7 @@ describe('reduce operator', () => {

type('should accept array typed reducers', () => {
let a: Observable<{ a: number; b: string }>;
a.pipe(reduce<{ a: number; b: string }>((acc, value) => acc.concat(value), []));
a.pipe(reduce((acc, value) => acc.concat(value), []));
});

type('should accept T typed reducers', () => {
Expand Down Expand Up @@ -322,7 +322,7 @@ describe('reduce operator', () => {

type('should accept R typed reduces when R is an array of T', () => {
let a: Observable<number>;
const reduced = a.pipe(reduce<number>((acc, value) => {
const reduced = a.pipe(reduce((acc, value) => {
acc.push(value);
return acc;
}, []));
Expand Down
6 changes: 3 additions & 3 deletions spec/operators/scan-spec.ts
Expand Up @@ -230,12 +230,12 @@ describe('scan operator', () => {

type('should accept array typed reducers', () => {
let a: Observable<{ a: number; b: string }>;
a.pipe(reduce<{ a: number; b: string }>((acc, value) => acc.concat(value), []));
a.pipe(scan((acc, value) => acc.concat(value), []));
});

type('should accept T typed reducers', () => {
let a: Observable<{ a?: number; b?: string }>;
a.pipe(reduce((acc, value) => {
a.pipe(scan((acc, value) => {
value.a = acc.a;
value.b = acc.b;
return acc;
Expand All @@ -244,7 +244,7 @@ describe('scan operator', () => {

type('should accept R typed reducers', () => {
let a: Observable<{ a: number; b: string }>;
a.pipe(reduce<{ a?: number; b?: string }>((acc, value) => {
a.pipe(scan<{ a?: number; b?: string }>((acc, value) => {
value.a = acc.a;
value.b = acc.b;
return acc;
Expand Down
12 changes: 6 additions & 6 deletions src/internal/operators/reduce.ts
Expand Up @@ -6,9 +6,9 @@ import { OperatorFunction, MonoTypeOperatorFunction } from '../types';
import { pipe } from '../util/pipe';

/* tslint:disable:max-line-length */
export function reduce<T, R>(accumulator: (acc: R, value: T, index: number) => R, seed: R): OperatorFunction<T, R>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there's this messed up scenario I think we need to test for:

of(1).pipe(reduce((acc, value, i) => i === 0 ? [] : [..value], { seed: 'LOL' })) The problem is that acc gets typed wrong.

I'm sure that scan has the same issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we assume that seed and ReturnType<typeof accumulator> are the same type, if they differ we get an error because accumulator returns an invalid value. Thoughts?

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

/**
Expand Down Expand Up @@ -62,20 +62,20 @@ export function reduce<T, R>(accumulator: (acc: R, value: T, index: number) => R
* @method reduce
* @owner Observable
*/
export function reduce<T, R>(accumulator: (acc: R, value: T, index?: number) => R, seed?: R): OperatorFunction<T, R> {
export function reduce<T, R>(accumulator: (acc: T | R, value: T, index?: number) => T | R, seed?: T | R): OperatorFunction<T, T | R> {
// providing a seed of `undefined` *should* be valid and trigger
// hasSeed! so don't use `seed !== undefined` checks!
// For this reason, we have to check it here at the original call site
// otherwise inside Operator/Subscriber we won't know if `undefined`
// means they didn't provide anything or if they literally provided `undefined`
if (arguments.length >= 2) {
return function reduceOperatorFunctionWithSeed(source: Observable<T>): Observable<R> {
return function reduceOperatorFunctionWithSeed(source: Observable<T>): Observable<T | R> {
return pipe(scan(accumulator, seed), takeLast(1), defaultIfEmpty(seed))(source);
};
}
return function reduceOperatorFunction(source: Observable<T>): Observable<R> {
return function reduceOperatorFunction(source: Observable<T>): Observable<T | R> {
return pipe(
scan((acc: R, value: T, index: number): R => accumulator(acc, value, index + 1)),
scan<T, T | R>((acc, value, index) => accumulator(acc, value, index + 1)),
takeLast(1),
)(source);
};
Expand Down
4 changes: 2 additions & 2 deletions src/internal/operators/scan.ts
Expand Up @@ -4,9 +4,9 @@ import { Subscriber } from '../Subscriber';
import { OperatorFunction, MonoTypeOperatorFunction } from '../types';

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

/**
Expand Down