zio / interop-cats

ZIO instances for cats-effect type classes
Apache License 2.0
157 stars 67 forks source link

Fails to summon Applicative[ZIO[Any, E, *]] #185

Open heksesang opened 4 years ago

heksesang commented 4 years ago

As seen in the Scastie below, the compiler fails to summon an instance of Applicative for ZIO[Any, E, *], while it can do it for ZIO[Any, Throwable, *]:

https://scastie.scala-lang.org/jKTDL41xRp6XgCBhhJsbxw

Think it's related to the ArrowChoice instances that are used to derive the Applicative, but it should be possible to provide an Applicative for that out of the box, considering how easy just writing it by hand is. ;)

heksesang commented 4 years ago

Seems the issue is related to importing both catz._ and catz.core._, which causes it to import the same instances twice.

It's not obvious that this would cause double import issues, and it does not help that catz.mtl._ has to be imported manually because it's not imported by catz._, causing catz.core._ and catz.mtl._ to behave different in regards to importing.

IMO this should be fixed so importing catz._ and catz.core._ at the same time should not be a problem.

neko-kai commented 4 years ago

@heksesang

IMO this should be fixed so importing catz. and catz.core. at the same time should not be a problem.

I'm afraid this is impossible (while keeping them in separate objects) because wildcard imports put everything in high priority scope. If we put them all into one object, prioritization via traits would work - but it may be non-trivial to ensure that this big object is compiled in such a way that missing Optional dependencies won't break it.

We can also do that by mixing in new traits to create specificity, e.g.

trait LessSpecific
trait MoreSpecific extends LessSpecific

// in catz extend everything with MoreSpecific
// in catz.core extend everything with LessSpecific

Obviously all of these solutions are unobvious, but may be worth it to increase UX. @ghostdogpr thoughts?

neko-kai commented 4 years ago

OTOH importing both catz._ and catz.core._ is completely redundant, because the former already contains all the instances. Not sure whether it makes sense to encourage that and not just put a warning in the README (like cats does for not mixing cats.implicits._ & cats.syntax/cats.instances imports)

heksesang commented 4 years ago

Is there a case where you'd ever want to import just catz.core._ (where importing catz._ instead would actually be problematic)?

neko-kai commented 4 years ago

@heksesang Yeah, the original use case was when you only depend on cats-core and don't have cats-effect on the classpath nor need the cats-effect instances, but still need cats-core instances - https://github.com/zio/interop-cats/issues/94

heksesang commented 4 years ago

Best would be to have one import that could ensure that it didn't rely on cats-effect unless it's really necessary, so that if you only have cats-core and do import zio.interop.catz._, you still get the core stuff without anything breaking due to missing cats-effect.

For example now, I have some code that compiles fine with either catz._ or catz.core._ when I have both cats-effect and cats-core on the classpath. So I had catz._ imported, but when I then removed the dependency on cats-effect from the project, the file fails to compile due to missing symbols. The fact that it can actually compile with just the instances from cats-core, but somehow uses some cats-effect specific instances when apparently not necessary, this adds to the confusion for the user IMO.

If there weren't two imports, but catz._ simply used the instances with the least dependencies when it could, it would cause less confusing cases for the user. You could add or remove cats-effect without having to change imports.

I don't know how easy (or hard) it would be to implement it that way, but the user experience would be better for it I think. :)

neko-kai commented 4 years ago

@heksesang Creating a combined catz object usable both with and without cats-effect on the classpath is AFAIK only possible with the "no more orphans" trick[1][2][3]. If you're interested in pursuing this change, you can open a PR implementing it here, using the previous links as a guideline on implementing this kind of functionality in libraries.