-
Notifications
You must be signed in to change notification settings - Fork 12
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
Feature/async mappers Result & Maybe #179
base: master
Are you sure you want to change the base?
Feature/async mappers Result & Maybe #179
Conversation
@ChrisHadar Thanks. It is going to be a little bit before I fully review this. I am swamped at the moment and need to carve some time to think about this and the approach. Thanks! |
This looks really good to me: has tests, no breaking changes, and very useful. You think you would have time @patrickmichalina at some point to review this? |
@greym0uth this PR contains async/await which is not something I would want in a lazily evaluated library. Because I see that I have not focused on getting this reviewed. But thanks for the reminder and I will try to pencil it in. |
@patrickmichalina Ah I see yea, perhaps @ChrisHadar could change it to use promises instead of the async/await keywords then? |
Also @patrickmichalina why don't you want async/await keywords? It's been a core feature of typescript since 2.1 and since these are TS files anything evaluating them should have async/await keyword support. Rollup can polyfill the build if it doesn't already (I think it translates to promises in the resulting JS already). |
Hi folks -- is there anything I can do to help move this PR forward? We're at the point of potentially maintaining our own version of this package in order to support this syntax. |
@vermiculus I will try get this over the fence |
I've added methods to both Result and Maybe which will automatically unwrap and wrap Promises with more intuitive behavior when mapping async functions