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

Add support for user-defined type guards to filter predicate #75

Merged
merged 1 commit into from Nov 17, 2017

Conversation

Projects
None yet
2 participants
@OliverJAsh
Contributor

OliverJAsh commented Nov 10, 2017

This enables the following:

const isNumber = (x: string | number): x is number => true
Option.of(1 as string | number)
  .filter(isNumber)
  .map(x => {
    const y: number = x
  })

https://www.typescriptlang.org/docs/handbook/advanced-types.html#user-defined-type-guards

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Nov 10, 2017

Codecov Report

Merging #75 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #75   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          24     24           
  Lines        2628   2628           
  Branches      434    434           
=====================================
  Hits         2628   2628
Impacted Files Coverage Δ
packages/funfix-core/src/disjunctions.ts 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b0192e...dfd5153. Read the comment docs.

codecov bot commented Nov 10, 2017

Codecov Report

Merging #75 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #75   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          24     24           
  Lines        2628   2628           
  Branches      434    434           
=====================================
  Hits         2628   2628
Impacted Files Coverage Δ
packages/funfix-core/src/disjunctions.ts 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b0192e...dfd5153. Read the comment docs.

@alexandru

This comment has been minimized.

Show comment
Hide comment
@alexandru

alexandru Nov 10, 2017

Member

@OliverJAsh this is nice, but I'd like to avoid overloads if possible, because Funfix doesn't do overloads.

I think a good candidate for this functionality is collect — which in Scala is usually a filter + map by means of a PartialFunction. This seems to be close.

I don't know what we can do for Flow at the moment. Worst case, we simply don't expose it to Flow. And we can ask around and it can come as a second commit.

Member

alexandru commented Nov 10, 2017

@OliverJAsh this is nice, but I'd like to avoid overloads if possible, because Funfix doesn't do overloads.

I think a good candidate for this functionality is collect — which in Scala is usually a filter + map by means of a PartialFunction. This seems to be close.

I don't know what we can do for Flow at the moment. Worst case, we simply don't expose it to Flow. And we can ask around and it can come as a second commit.

@alexandru

This comment has been minimized.

Show comment
Hide comment
@alexandru

alexandru Nov 10, 2017

Member

Actually, wait before doing changes, I have to think about it.

If we can't support it on Flow, maybe the overload is better.

Member

alexandru commented Nov 10, 2017

Actually, wait before doing changes, I have to think about it.

If we can't support it on Flow, maybe the overload is better.

@OliverJAsh

This comment has been minimized.

Show comment
Hide comment
@OliverJAsh

OliverJAsh Nov 10, 2017

Contributor

@alexandru Sure, let me know what you end up deciding. Interested to hear your reasoning, and learn how this works in other languages!

Contributor

OliverJAsh commented Nov 10, 2017

@alexandru Sure, let me know what you end up deciding. Interested to hear your reasoning, and learn how this works in other languages!

@OliverJAsh

This comment has been minimized.

Show comment
Hide comment
@OliverJAsh

OliverJAsh Nov 17, 2017

Contributor

Hey @alexandru, did you have any further thoughts on this?

Contributor

OliverJAsh commented Nov 17, 2017

Hey @alexandru, did you have any further thoughts on this?

@alexandru

This comment has been minimized.

Show comment
Hide comment
@alexandru

alexandru Nov 17, 2017

Member

It’s fine the way it is.

Member

alexandru commented Nov 17, 2017

It’s fine the way it is.

@alexandru alexandru merged commit 95ed25b into funfix:master Nov 17, 2017

3 checks passed

codecov/patch Coverage not affected when comparing 1b0192e...dfd5153
Details
codecov/project 100% remains the same compared to 1b0192e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@OliverJAsh

This comment has been minimized.

Show comment
Hide comment
@OliverJAsh

OliverJAsh Dec 11, 2017

Contributor

Hi @alexandru, any ideas when this will be released, so we can remove our local patch?

Contributor

OliverJAsh commented Dec 11, 2017

Hi @alexandru, any ideas when this will be released, so we can remove our local patch?

@alexandru

This comment has been minimized.

Show comment
Hide comment
@alexandru

alexandru Dec 12, 2017

Member

Sorry for the delay, I've been procrastinating because I was hoping to get other TS-related changes in. Will do a release tomorrow.

Member

alexandru commented Dec 12, 2017

Sorry for the delay, I've been procrastinating because I was hoping to get other TS-related changes in. Will do a release tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment