softwaremill / diffx

Pretty diffs for scala case classes
Apache License 2.0
342 stars 30 forks source link

Documentation for customizing subclass instances when deriving parent instances #60

Closed jatcwang closed 4 years ago

jatcwang commented 4 years ago

Would be good if there are some docs on how to customize instances when derivation is involved. For example a user might try to use ignore on id field of a subclass like this:

sealed trait Parent
case class A(id: String, value: Int) extends Parent
case class B(id: String, value: Int) extends Parent

implicit val diffA: Diff[A] = Derived[Diff[A]].ignore(_.id)

val result: Parent = A("id1", 12)
val expected: Parent = A("id2", 12)

result should matchTo(expected)

However confusingly their Diff[A] instance doesn't work. They need to do this instead:

implicit val diffA: Derived[Diff[A]] = Derived(Diff.gen[A].ignore(_.id))

In order for the customized instance to be picked up by the magnolia derivation. (By using Diff.gen We also get a compiler warning from magnolia about "using fallback derivation for String" which is a separate issue but should be documented/fixed)

Do you think this is the proper way to resolve this? If so I'm happy to add to the docs.

ghostbuster91 commented 4 years ago

Yes, unfortunately you are right. Ignoring part of subclass is much more confusing than I wanted it to be :(

(By using Diff.gen We also get a compiler warning from magnolia about "using fallback derivation for String" which is a separate issue but should be documented/fixed)

We should probably report it to magnolia.

Do you think this is the proper way to resolve this? If so I'm happy to add to the docs.

I think that the proper way to solve it, would be to tell magnolia to use Diff instances instead of Derived[Diff] like I did in other builtin Derived[Diff] instances. IMO this is connected to https://github.com/propensive/magnolia/issues/107 which is still not solved. So this path is unavailable, at least for now.

PRs are always welcome, if you could improve the docs I will be happy to merge it :)