Skip to content

Commit

Permalink
fix(typings): Use overloading instead of union type for typings filte…
Browse files Browse the repository at this point in the history
…r, find, first, last.

Abstract
------------
- This fixes ReactiveX#2163
  by reverts commit 922d04e.
  - So this reverts ReactiveX#2119 sadly.

Drawback
---------
- We would need specify type parameters to their method
  explicitly if we'd like to narrow the type via `predicate` as type
  guarde function.
  - However, I don't think this is a serious problem because
    we can avoid this drawback with specifying actual types only.
  - And this changes makes our typings more similar to [TypeScript's
    Array
    methods](https://github.com/Microsoft/TypeScript/blob/master/lib/lib.es5.d.ts#L1086-L1097).
  • Loading branch information
tetsuharuohzeki committed Nov 30, 2016
1 parent 4df531f commit 739593f
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 37 deletions.
16 changes: 15 additions & 1 deletion spec/operators/filter-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,25 @@ describe('Observable.prototype.filter', () => {
const isString = (x: string | number): x is string => typeof x === 'string';

// Here, `s` is a string in the second filter predicate after the type guard (yay - intellisense!)
const guardedFilter = x.filter(isString).filter(s => s.length === 2); // Observable<string>
const guardedFilter = x.filter<string | number, string>(isString).filter(s => s.length === 2); // Observable<string>
// In contrast, this type of regular boolean predicate still maintains the original type
const boolFilter = x.filter(s => typeof s === 'number'); // Observable<string | number>
}

{
interface Bar {
bar?: string;
}
class Foo implements Bar {
constructor(public bar: string = 'name') {}
}

let foo: Bar = new Foo(); // <--- type is interface, not the class
Observable.of(foo)
.filter(foo => foo.bar === 'name')
.subscribe(foo => console.log(foo.bar)); // <-- "Property 'bar' does not exist on type '{}'"
}

// tslint:disable enable
});
});
2 changes: 1 addition & 1 deletion spec/operators/find-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ describe('Observable.prototype.find', () => {
const isString = (x: string | number): x is string => typeof x === 'string';

// After the type guard `find` predicate, the type is narrowed to string
const guardedFind = x.find(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>
const guardedFind = x.find<string | number, string>(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>
// In contrast, a boolean predicate maintains the original type
const boolFind = x.find(x => typeof x === 'string'); // Observable<string | number>
}
Expand Down
4 changes: 2 additions & 2 deletions spec/operators/first-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ describe('Observable.prototype.first', () => {
const isString = (x: string | number): x is string => typeof x === 'string';

// After the type guard `first` predicates, the type is narrowed to string
const guardedFirst1 = x.first(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>
const guardedFirst2 = x.first(isString, s => s.substr(0)).filter(s => s.length > 1); // Observable<string>
const guardedFirst1 = x.first<string | number, string>(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>
const guardedFirst2 = x.first<string | number, string>(isString, s => s.substr(0)).filter(s => s.length > 1); // Observable<string>
// Without a resultSelector, `first` maintains the original type (TS can't do this yet)
const boolFirst1 = x.first(x => typeof x === 'string', null, ''); // Observable<string | number>
// `first` still uses the `resultSelector` return type, if it exists.
Expand Down
4 changes: 2 additions & 2 deletions spec/operators/last-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ describe('Observable.prototype.last', () => {
const isString = (x: string | number): x is string => typeof x === 'string';

// After the type guard `last` predicates, the type is narrowed to string
const guardedLast1 = x.last(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>
const guardedLast2 = x.last(isString, s => s.substr(0)).filter(s => s.length > 1); // Observable<string>
const guardedLast1 = x.last<string | number, string>(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>
const guardedLast2 = x.last<string | number, string>(isString, s => s.substr(0)).filter(s => s.length > 1); // Observable<string>
// Without a resultSelector, `last` maintains the original type (TS can't do this yet)
const boolLast1 = x.last(x => typeof x === 'string', null, ''); // Observable<string | number>
// `last` still uses the `resultSelector` return type, if it exists.
Expand Down
11 changes: 7 additions & 4 deletions src/operator/filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@ import { Subscriber } from '../Subscriber';
import { Observable } from '../Observable';
import { TeardownLogic } from '../Subscription';

/* tslint:disable:max-line-length */
// XXX: At typescript@2.0, we need prepare the version which takes `predicate`
// returning `boolean` before to define the one which takes the type guard `predicate`
// so that the type inference works correctly for the case of that `predicate` returning `boolean` simply.
export function filter<T>(this: Observable<T>,
predicate: (value: T, index: number) => boolean,
thisArg?: any): Observable<T>;
export function filter<T, S extends T>(this: Observable<T>,
predicate: ((value: T, index: number) => boolean) |
((value: T, index: number) => value is S),
predicate: (value: T, index: number) => value is S,
thisArg?: any): Observable<S>;
/* tslint:disable:max-line-length */

/**
* Filter items emitted by the source Observable by only emitting those that
Expand Down
11 changes: 7 additions & 4 deletions src/operator/find.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ import { Observable } from '../Observable';
import { Operator } from '../Operator';
import { Subscriber } from '../Subscriber';

/* tslint:disable:max-line-length */
// XXX: At typescript@2.0, we need prepare the version which takes `predicate`
// returning `boolean` before to define the one which takes the type guard `predicate`
// so that the type inference works correctly for the case of that `predicate` returning `boolean` simply.
export function find<T>(this: Observable<T>,
predicate: (value: T, index: number, source: Observable<T>) => boolean,
thisArg?: any): Observable<T>;
export function find<T, S extends T>(this: Observable<T>,
predicate: ((value: T, index: number, source: Observable<T>) => boolean) |
((value: T, index: number, source: Observable<T>) => value is S),
predicate: (value: T, index: number, source: Observable<T>) => value is S,
thisArg?: any): Observable<S>;
/* tslint:disable:max-line-length */

/**
* Emits only the first value emitted by the source Observable that meets some
Expand Down
31 changes: 19 additions & 12 deletions src/operator/first.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,26 @@ import { Operator } from '../Operator';
import { Subscriber } from '../Subscriber';
import { EmptyError } from '../util/EmptyError';

/* tslint:disable:max-line-length */
// XXX: At typescript@2.0, we need prepare the version which takes `predicate`
// returning `boolean` before to define the one which takes the type guard `predicate`
// so that the type inference works correctly for the case of that `predicate` returning `boolean` simply.
export function first<T>(this: Observable<T>,
predicate?: (value: T, index: number,
source: Observable<T>) => boolean): Observable<T>;
export function first<T, S extends T>(this: Observable<T>,
predicate?: ((value: T, index: number, source: Observable<T>) => boolean) |
((value: T, index: number, source: Observable<T>) => value is S)): Observable<S>;
export function first<T>(this: Observable<T>, predicate: (value: T, index: number, source: Observable<T>) => boolean, resultSelector: void, defaultValue?: T): Observable<T>;
export function first<T, S extends T, R>(this: Observable<T>,
predicate: ((value: T, index: number, source: Observable<T>) => boolean) |
((value: T, index: number, source: Observable<T>) => value is S),
resultSelector?: ((value: S, index: number) => R) | void,
defaultValue?: S): Observable<S>;
export function first<T, R>(this: Observable<T>, predicate?: (value: T, index: number, source: Observable<T>) => boolean, resultSelector?: (value: T, index: number) => R, defaultValue?: R): Observable<R>;
/* tslint:disable:max-line-length */

predicate?: (value: T, index: number,
source: Observable<T>) => value is S): Observable<S>;
export function first<T>(this: Observable<T>,
predicate: (value: T, index: number, source: Observable<T>) => boolean,
resultSelector: (value: T, index: number) => T,
defaultValue?: T): Observable<T>;
export function first<T, S extends T>(this: Observable<T>,
predicate: (value: T, index: number, source: Observable<T>) => value is S,
resultSelector: (value: S, index: number) => S,
defaultValue?: S): Observable<S>;
export function first<T, R>(this: Observable<T>, predicate?: (value: T, index: number, source: Observable<T>) => boolean,
resultSelector?: (value: T, index: number) => R,
defaultValue?: R): Observable<R>;
/**
* Emits only the first value (or the first value that meets some condition)
* emitted by the source Observable.
Expand Down
32 changes: 21 additions & 11 deletions src/operator/last.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,28 @@ import { Operator } from '../Operator';
import { Subscriber } from '../Subscriber';
import { EmptyError } from '../util/EmptyError';

/* tslint:disable:max-line-length */
// XXX: At typescript@2.0, we need prepare the version which takes `predicate`
// returning `boolean` before to define the one which takes the type guard `predicate`
// so that the type inference works correctly for the case of that `predicate` returning `boolean` simply.
export function last<T>(this: Observable<T>,
predicate?: (value: T, index: number,
source: Observable<T>) => boolean): Observable<T>;
export function last<T, S extends T>(this: Observable<T>,
predicate?: ((value: T, index: number, source: Observable<T>) => boolean) |
((value: T, index: number, source: Observable<T>) => value is S)): Observable<S>;
export function last<T>(this: Observable<T>, predicate: (value: T, index: number, source: Observable<T>) => boolean, resultSelector: void, defaultValue?: T): Observable<T>;
export function last<T, S extends T, R>(this: Observable<T>,
predicate: ((value: T, index: number, source: Observable<T>) => boolean) |
((value: T, index: number, source: Observable<T>) => value is S),
resultSelector?: ((value: S, index: number) => R) | void,
defaultValue?: S): Observable<S>;
export function last<T, R>(this: Observable<T>, predicate?: (value: T, index: number, source: Observable<T>) => boolean, resultSelector?: (value: T, index: number) => R, defaultValue?: R): Observable<R>;
/* tslint:disable:max-line-length */
predicate?: (value: T, index: number,
source: Observable<T>) => value is S): Observable<S>;
export function last<T>(this: Observable<T>,
predicate: (value: T, index: number,
source: Observable<T>) => boolean,
resultSelector: (value: T, index: number) => T,
defaultValue?: T): Observable<T>;
export function last<T, S extends T>(this: Observable<T>,
predicate: (value: T, index: number, source: Observable<T>) => value is S,
resultSelector: (value: S, index: number) => S,
defaultValue?: S): Observable<S>;
export function last<T, R>(this: Observable<T>,
predicate?: (value: T, index: number, source: Observable<T>) => boolean,
resultSelector?: (value: T, index: number) => R,
defaultValue?: R): Observable<R>;

/**
* Returns an Observable that emits only the last item emitted by the source Observable.
Expand Down

0 comments on commit 739593f

Please sign in to comment.