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

make EitherAsync interface more versatile #192

Closed
wants to merge 5 commits into from

Conversation

chenguo
Copy link
Contributor

@chenguo chenguo commented Jun 5, 2020

In my project I've been integrated Purify mostly to get the benefits of Either and Maybe. When I hit some async parts of our code I saw some potential ways to make EitherAsyncs easier to work with.

Specifically:

  • make EitherAsync extend Promise<Either>
  • map and mapLeft now work with async map functions
  • chain and chainLeft now work with chain functions that return Either or Promise<Either>

Quick example. Assume you have the following operations you might want to do on your data:

async function asyncThing(n: number): Promise<number> {
  return n + 1;
}

function eitherThing(n: number): Either<string, number> {
  return Right(n + 1);
}

async function asyncEitherThing(n: number): Promise<Either<string, number>> {
  return eitherThing(n);
}

Monadic chaining with the current interface would look something like:

const n: Either<string, number> = Right(10);

const result = liftEither(n)
  .chain(value => liftEither(eitherThing(value)))
  .chain(value => fromPromise(() => asyncEitherThing(value)))
  .map(asyncThing);

  (await result.run()).map(async (n: Promise<number>) => {
    console.log('number', await n);
  });

With the new interface, you can do:

const n: Either<string, number> = Right(10);

const result = liftEither(n)
  .chain(eitherThing)
  .chain(asyncEitherThing)
  .map(asyncThing);

(await result).map((n: number) => console.log('number', n));

@gigobyte
Copy link
Owner

gigobyte commented Jun 5, 2020

Wow, thanks for the contribution!

I think I really like the changes to chain, not so sure about the R | PromiseLike<R> part in map though. I will have to checkout your branch and play with the implementation.

@chenguo
Copy link
Contributor Author

chenguo commented Jun 5, 2020

Yeah look forward to your feedback.

For map, the goal for me was to make it so we don't have an EitherAsync<L, Promise<R>>, essentially I think it's natural to flatten the promises and make those EitherAsync<L, R>. The current code actually already behaves like that (all I had to change is the type signature), it's just Typescript wasn't aware that Promises were handled.

I'm open to however you think is best to implement this, just as long as it's easy to work with async mapping functions.

@KevinMGranger
Copy link
Contributor

KevinMGranger commented Jun 5, 2020

One of the main criticisms in the FP community of promises is how they break certain monadic laws. One might deliberately want something like EitherAsync<L1, EitherAsync<L2, R>> so that a series of possibly fallible steps can be handled depending upon which step failed. Making map automatically flatten the promise just like then does has similar issues.

Making chain work with a promise-producing function is a nice idea. But isn't making map work with promise-returning functions redundant at that point?

@chenguo
Copy link
Contributor Author

chenguo commented Jun 6, 2020

One of the main criticisms in the FP community of promises is how they break certain monadic laws.

Interesting, how so? Promises themselves are monads, right? But maybe there's some weird corner cases I'm not thinking of.

Making chain work with a promise-producing function is a nice idea. But isn't making map work with promise-returning functions redundant at that point?

I see what you mean, I see chain as a flatMap that can handle an Either-like return-type, whereas map just handles a naked value. When I made my changes I was mostly just trying to make Promises less painful, so a map can handle a Promise<R>, and a chain handles a Promise<Either<L2, R>. They'll flatten the Promise part since EitherAsync is already a Promise, hence I regard it as pointless, but the underlying dealings with a R vs a Either<L2, R> still holds. So you would map an async function getNextNumber(n): Promise<number> but you would chain an async function getNextNumberOrError(n): Promise<Either<Error, number>>

One might deliberately want something like EitherAsync<L1, EitherAsync<L2, R>>

Hmm I see what you mean here. If you do something like:

liftEither(Right(10))
    .map(n => liftEither(Right(n + 5)));

You get a EitherAsync<L1, Either<L2, R>> rather than EitherAsync<L1, EitherAsync<L2, R>> since the Promise portion of an EitherAsync gets flattened automatically. To that I say... is that maybe ok? The important thing is both L1 and L2 are kept around, and that the entire thing is an EitherAsync so there's still deferred execution.

@gigobyte
Copy link
Owner

gigobyte commented Jun 6, 2020

Promises themselves are monads, right?

They are not. Check out this controversial thread from years ago

@chenguo
Copy link
Contributor Author

chenguo commented Jun 19, 2020

Are there any changes you want to make? If you have an idea of the interface you want but just don't have time, I'd be happy to bang it out.

@gigobyte
Copy link
Owner

gigobyte commented Jun 20, 2020

Hey, I copied some of the changes you made, unfortunately the discrepancies are too big for me to merge this PR, I hope that's okay with you. I want to thank you again for your contribution, you can see the new implementation of EitherAsync in master. To summarize:

  • EitherAsync now implements PromiseLike (Not Promise, like in this PR)
  • The function passed to chain is (value: R) => PromiseLike<Either<L, R2>>), and since EitherAsync implements this you can pass either an EitherAsync or a Promise. This was much needed and a welcome change.
  • I didn't add the autolifting feature you proposed (map and chain automatically lifting a value Either -> Promise<Either>)
  • I moved liftPromise, fromPromise and liftEither inside the EitherAsync namespace. I wanted to do this from the start but you needed to use them a lot so I exposed them directly as named exports.

@gigobyte gigobyte closed this Jun 20, 2020
@chenguo
Copy link
Contributor Author

chenguo commented Jun 22, 2020

Awesome, thanks, is there a new deployed beta version? We've been using an internal dev deploy to work off of.

I didn't add the autolifting feature you proposed (map and chain automatically lifting a value Either -> Promise)

That's fair enough. What do you think would be best practice for mapping over some async operations from an EitherAsync?

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