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(types): add Boolean signature to filter #4961

Merged
merged 2 commits into from Aug 26, 2019

Conversation

cartant
Copy link
Collaborator

@cartant cartant commented Aug 11, 2019

Closes #4959

Description:

This PR adds another signature overload to filter to match Boolean predicates. This solves the problem identified in #4959 and is sufficiently specific that I cannot foresee it effecting further problems.

I've added a conventional test and a dtslint test.

Other operators that have a similar issue are: find, first, last and takeWhile. If this fix is approved, I'll have a look at what can be done for those (first and last are a little more complicated, as they support a default value).

@benlesh @rkirov

Related issue (if exists): #4959

@cartant cartant requested a review from benlesh August 11, 2019 00:26
@rkirov
Copy link

rkirov commented Aug 12, 2019

Looks good to me, I will downstream this to google internal to unblock the TS3.5 migration.

Copy link
Member

@benlesh benlesh left a comment

Choose a reason for hiding this comment

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

Should we also do this for other operators with predicates? Like first, last, etc?

@cartant
Copy link
Collaborator Author

cartant commented Aug 22, 2019

Should we also do this for other operators with predicates? Like first, last, etc?

Yeah. We should use this approach for find, first, last and takeWhile - those are the only other operators that accept a user-defined type guard. I was waiting to hear back from Rado, as some of those other operators are a little more complicated - as they support default values - and didn't want to prep type changes if they weren't gonna do the job. But if the Google code base is now synced, I guess we should look at change 'em, too.

@benlesh benlesh merged commit 259853e into ReactiveX:master Aug 26, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 25, 2019
@cartant cartant deleted the issue-4959 branch September 24, 2020 07:09
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.

[TS] filter(Boolean) loses type information, TypeScript 3.5 makes it worse
4 participants