search

japgolly / scalajs-react

Facebook's React on Scala.JS
https://japgolly.github.io/scalajs-react/
Apache License 2.0
1.64k stars 232 forks source link

WIP - Double reusability implicit #1013

Closed JamesMcIntosh closed 2 years ago

JamesMcIntosh commented 2 years ago

WIP - I've still to test this change locally.

I wasn't sure if there was a reason for excluding an implicit Double, I've needed it a couple of times recently so thought I'd have a quick dig in the code to see where the other implicits came from.

japgolly commented 2 years ago

Hey @JamesMcIntosh , thanks for the PR.

Unfortunately there is a reason that Reusability[Double] isn't provided out-of-the-box, it's not uncommon to have a bit of tolerance when comparing doubles. In fact there's a Reusability.double which takes a tolerance. Eg. if some AJAX call returns 4.005134978165 and then on next call returns 4.005134971648 the majority of components would not need a re-render but having an out-of-the-box Reusability[Double] instance of silently just cause a re-render. Not having an implicit instance makes it a userland decision, and not just that, ensures that when userland decisions are made, they're applied as intended (as opposed to accidentally forgetting an import and having the default silently kick in).

japgolly commented 2 years ago

For java.time stuff, users can import Reusability.TemporalImplicitsWithoutTolerance._. Maybe we should add a Reusability.NumbersWithoutTolerance._ import too?

japgolly commented 2 years ago

Closing in favour of #1029 which adds Reusability.DecimalImplicitsWithoutTolerance

JamesMcIntosh commented 2 years ago

@japgolly Thanks for explaining the reasoning behind Reusability.double(tolerance) and creating the new DecimalImplicitsWithoutTolerance

japgolly commented 2 years ago

@JamesMcIntosh np and thanks for raising this. It's released in 2.0.1 now so enjoy!