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

feat(filter, find, first, last): support type guard predicates that don't requiring casting #2119

Merged
merged 4 commits into from
Nov 7, 2016
Merged

feat(filter, find, first, last): support type guard predicates that don't requiring casting #2119

merged 4 commits into from
Nov 7, 2016

Conversation

rob3c
Copy link
Contributor

@rob3c rob3c commented Nov 7, 2016

Description:
These updates allow the use of type guard predicates to flow narrowed types through Observable operator chains using the filter, find, first, and last operators without requiring casting. Unfortunately, direct use of typeof x === 'string' style boolean predicates are not able to be supported yet in this context, even though typescript understands how to use them in other contexts (e.g. if/else statements and such). However, type guard functions can still reduce a lot of boilerplate casting noise with these new definitions. Examples for each operator appear below.

// x is `Observable<string | number>`
let x = Observable.from([1, 'aaa', 3, 'bb']);

// This type guard will narrow a `string | number` to a string in the examples below
let 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!)
let guardedFilter = x.filter(isString).filter(s => s.length === 2); // Observable<string>
// In contrast, this type of regular boolean predicate still maintains the original type
let boolFilter = x.filter(s => typeof s === 'number'); // Observable<string | number>

// After the type guard `find` predicate, the type is narrowed to string
let guardedFind = x.find(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>  
// In contrast, a boolean predicate maintains the original type
let boolFind = x.find(x => typeof x === 'string'); // Observable<string | number>

// After the type guard `first` predicates, the type is narrowed to string
let guardedFirst1 = x.first(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>
let guardedFirst2 = x.first(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)
let boolFirst1 = x.first(x => typeof x === 'string', null, ''); // Observable<string | number>
// `first` still uses the `resultSelector` return type, if it exists.
let boolFirst2 = x.first(x => typeof x === 'string', s => ({str: `${s}`}), {str: ''}); // Observable<{str: string}>

// Same for `last`:
let guardedLast1 = x.last(isString).filter(s => s.length > 1).map(s => s.substr(1)); // Observable<string>
let guardedLast2 = x.last(isString, s => s.substr(0)).filter(s => s.length > 1); // Observable<string>
let boolLast1 = x.last(x => typeof x === 'string', null, ''); // Observable<string | number>
let boolLast2 = x.last(x => typeof x === 'string', s => ({str: `${s}`}), {str: ''}); // Observable<{str: string}>

Related issue (if exists):
This enhances the initial attempt to integrate type guards for narrowing types in operator chains in #1936. I had initially asked about a solution to this in microsoft/TypeScript#7657 (comment), but arrived at this solution before there was any response.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.595% when pulling 07ecd5e on rob3c:fix-type-guard-propagation into bf3c043 on ReactiveX:master.

@kwonoj
Copy link
Member

kwonoj commented Nov 7, 2016

Change looks fine to me, /cc @david-driscoll

@david-driscoll
Copy link
Member

LGTM

1 similar comment
@kwonoj
Copy link
Member

kwonoj commented Nov 7, 2016

LGTM

@kwonoj kwonoj merged commit 922d04e into ReactiveX:master Nov 7, 2016
tetsuharuohzeki added a commit to tetsuharuohzeki/rxjs that referenced this pull request Nov 30, 2016
tetsuharuohzeki added a commit to tetsuharuohzeki/rxjs that referenced this pull request Nov 30, 2016
…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).
@rob3c rob3c mentioned this pull request Dec 3, 2016
tetsuharuohzeki added a commit to tetsuharuohzeki/rxjs that referenced this pull request Dec 6, 2016
tetsuharuohzeki added a commit to tetsuharuohzeki/rxjs that referenced this pull request Dec 6, 2016
…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).
@intellix
Copy link

intellix commented Jun 20, 2017

Having issues with this in TS 2.4 within my Angular codebase.

@angular/router exports the following type:

export type Event = NavigationStart | NavigationEnd | NavigationCancel | NavigationError |
    RoutesRecognized | RouteConfigLoadStart | RouteConfigLoadEnd;

Pre 2.4 I was doing:

navigations: Observable<NavigationEnd> = this.router.events.filter(event => event instanceof NavigationEnd);

But now get:

Type 'Observable<Event>' is not assignable to type 'Observable<NavigationEnd>'.
  Type 'Event' is not assignable to type 'NavigationEnd'.
    Type 'NavigationStart' is not assignable to type 'NavigationEnd'.
      Property 'urlAfterRedirects' is missing in type 'NavigationStart'.

Which looks to me like it's not understanding my instanceof filter, but it only started giving me trouble in 2.4 which apparently strengthens Generics somewhat: https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#improved-inference-for-generics

Update: With some help in the TS Gitter room from NaridaL I've got this workaround:

navigations: Observable<NavigationEnd> = this.router.events.filter((event): event is NavigationEnd => event instanceof NavigationEnd);

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants