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

Support bind and bindTo for Parser monad #31

Merged
merged 3 commits into from
Aug 27, 2020
Merged

Conversation

CYBAI
Copy link
Contributor

@CYBAI CYBAI commented Aug 24, 2020

Hi @gcanti, while upgrading a private repo to use bind and bindTo introduced in fp-ts@2.8.1, I just noticed that parser-ts doesn't support these two methods.

I wonder if it's good to also introduce them to parser-ts? Also, with introducing bind and bindTo to Parser monad, we're able to drop the fp-ts-contrib dependency for the examples/command.ts in this repo.

Also, if this PR is acceptable, should I update the package.json version in this PR? or it will be handled while publishing?

Thank you! 🙇

CYBAI added 3 commits August 24, 2020 11:24
With using the methods from Parser monad, we don't need to depend on
`fp-ts-contrib` anymore in this package.
@IMax153
Copy link
Collaborator

IMax153 commented Aug 24, 2020

Hey @CYBAI!

Thank you very much for the PR!

I also find myself frequently using do notation with parser-ts since it lends itself to simpler parsing of data into data structures. From my perspective, this PR looks good 👍.

I think a larger question that will need to be addressed with @gcanti is how to handle monadic libraries within the fp-ts ecosystem that also want to make use of the pipeable do notation. That is to say, for libraries based upon fp-ts that define their own Monad typeclasses:

  • Will each library need to specify their own implementation of bind and bindTo?
  • Or should those helpers be made public in fp-ts/lib/function?

As for your question about bumping the version of parser-ts, I believe that @gcanti will take care of that when publishing the updated version.

@IMax153 IMax153 self-requested a review August 24, 2020 14:41
@IMax153
Copy link
Collaborator

IMax153 commented Aug 27, 2020

@gcanti - I am going to merge this PR as long as you have no objections.

I am curious to hear your thoughts on my comment above though regarding how ecosystem libraries should handle the new pipeable do notation.

@CYBAI
Copy link
Contributor Author

CYBAI commented Aug 27, 2020

I am curious to hear your thoughts on my comment above though regarding how ecosystem libraries should handle the new pipeable do notation.

I'm also curious about this and would be happy to migrate to a better solution in the future!

@gcanti
Copy link
Owner

gcanti commented Aug 27, 2020

I am going to merge this PR as long as you have no objections.

LGTM, thanks @CYBAI

I am curious to hear your thoughts on my comment above though regarding how ecosystem libraries should handle the new pipeable do notation.

Like in this PR (for the moment), I might export bind_ and bindTo_ from fp-ts/function in the future

@IMax153 IMax153 merged commit c4c9267 into gcanti:master Aug 27, 2020
@CYBAI CYBAI deleted the bind branch August 28, 2020 00:47
@IMax153
Copy link
Collaborator

IMax153 commented Aug 28, 2020

@CYBAI & @gcanti - thank you! Merged 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants