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 Mapper version of EndpointModule #1090

Open
sergeykolbasov opened this issue Feb 25, 2019 · 4 comments
Open

Make Mapper version of EndpointModule #1090

sergeykolbasov opened this issue Feb 25, 2019 · 4 comments
Labels
Milestone

Comments

@sergeykolbasov
Copy link
Collaborator

During playing over #1089 , I found out that our Mapper implicits fail to resolve for cases of A => F[Response | Output[B]] when F is some complex type like:

  • StateT[F, State, ?]
  • ReaderT[F, A, B]
  • WriterT[F, Log, ?]

I suspect it somehow relates to complex mapping we have for F ~> G in those cases and compiler fails to go that deep in the rabbit hole, as for simple functions A => Response it works fine.

Suggestion to fix it is to make Mapper instances available as a trait and extend it in EndpointModule, so it would simplify compiler work for a single type F:

Something like:

trait MapperInstances[F[_]] {
  implicit def mapperFromKindToEffectOutputFunction[A, B](f: A => F[Output[B]])(implicit F: Monad[F]): Mapper.Aux[G, A, B] =
    instance(_.mapOutputAsync(a =>f(a)))

  //rest of the conversions
}

Also, it might be valuable to introduce Endpoint.mapK for transformation of the effect of Endpoint:

def mapK[G[_]](f: F ~> G): Endpoint[G, A]
@sergeykolbasov
Copy link
Collaborator Author

/cc @joroKr21

@joroKr21
Copy link
Collaborator

👍 for mapK, it's very useful.

I'm not sure about Mapper though. I think the core of the problem using the magnet pattern in the first place (esp. when type parameters are involved). It makes it difficult to understand what you can pass to a method expecting a Mapper and almost impossible for type inference to work correctly. Even using regular functions is difficult - you have to specify the parameter types explicitly even though the endpoint already has all the necessary type information.

@vkostyukov vkostyukov added this to the Finch 1.0 milestone Feb 25, 2019
@joroKr21
Copy link
Collaborator

Should we also have an Endpoint alias in EndpointModule? Might make migration easier. But on the other hand confuse users. Wdyt?

@sergeykolbasov
Copy link
Collaborator Author

sergeykolbasov commented Feb 28, 2019

Actually, I start thinking that probably we should make Mapper rather an optional thing that is available only via import, and encourage users to use corresponding map* functions instead. At least, it's quite clear from the signature of endpoint methods.

@vkostyukov @rpless what are your thoughts on this?

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

No branches or pull requests

3 participants