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

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

Closes #4959
@cartant cartant requested a review from benlesh Aug 11, 2019
@kwonoj
kwonoj approved these changes Aug 11, 2019
@rkirov

This comment has been minimized.

Copy link

commented Aug 12, 2019

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

Copy link
Member

left a comment

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

@cartant

This comment has been minimized.

Copy link
Collaborator Author

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
5 checks passed
5 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: dtslint Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: typescript3 Your tests passed on CircleCI!
Details
@lock lock bot locked as resolved and limited conversation to collaborators Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.