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

[Circe] Unclear implicit prioritisation rules #89

Closed
vpavkin opened this issue Apr 10, 2018 · 5 comments
Closed

[Circe] Unclear implicit prioritisation rules #89

vpavkin opened this issue Apr 10, 2018 · 5 comments

Comments

@vpavkin
Copy link
Contributor

vpavkin commented Apr 10, 2018

Reproduction:
https://github.com/vpavkin/magnolia/blob/e278256e713ddf3b1aa3365898e930eff168274b/tests/src/main/scala/PriorityIssueTest.scala

Magnolia derivers sometimes override default instances, available from companion. For example, for this type

case class MapContainer(theMap: Map[String, List[Int]])

magnolia will derive coproduct codec for the List on rhs of the Map.

This only happens, when default instances are provided by means of being located in companion. If we import them explicitly (with import io.circe.Encoder._), then magnolia starts to see them.

This is confusing, and hard to notice and debug within deeply nested hierarchies.

@joroKr21
Copy link
Contributor

In the test you import import MagnoliaEncoder.genEncoder which gives it higher priority than implicits in the companion object Encoder according to the regular implicit resolution rules in Scala. Even if we try to change it (which I'm not sure is possible), the net result will probably still be confusing because then people will be surprised why it doesn't follow the standard rules.

@vpavkin
Copy link
Contributor Author

vpavkin commented Apr 15, 2018

@joroKr21 makes sense, but when using circe auto we do kinda the same:

import io.circe.generic.auto._

And it doesn't stay in the way of default codecs.

Although, I have to admit, circe does some tricks with Exported, from end-user perspective it looks the same.

@joroKr21
Copy link
Contributor

Sure, but there is also generic.semiauto which is arguably more useful. I guess the problem here is that you would want to support both use cases.

@vpavkin
Copy link
Contributor Author

vpavkin commented Apr 15, 2018

support for both would be nice, yes, but I actually also mostly use semiauto.
Will explore how magnolia behaves for such purpose, thanks for suggestion!

@vpavkin
Copy link
Contributor Author

vpavkin commented Jun 15, 2018

Closing in favor of #107

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

No branches or pull requests

2 participants