nosco / hx

A simple, easy to use library for React development in ClojureScript.
MIT License
247 stars 16 forks source link

CLJS data structures are compared by ref in <-effect deps #39

Closed lilactown closed 5 years ago

lilactown commented 5 years ago

This can lead to during the effect more often than desired, since it does not use Clojure's equality semantics.

lilactown commented 5 years ago

Solution is to capture deps in a ref and do the equality check in clojure, pass result to React

lilactown commented 5 years ago

Fixed via https://github.com/Lokeh/hx/commit/ba0d520d45af6be81875890a0ceaa8ad707101c4

orestis commented 5 years ago

I was just reading up on this, came to post about it :)

Doesn’t useState have the same issue? React will skip the render if the returned value is the same as the previous one, but it uses the same algorithm.

lilactown commented 5 years ago

Ah, I didn’t think to check useState as well. Good call out!

I wonder if there is a good way to handle that... I’m not sure how to do the comparison if the value given to the update function is itself a function to be applied, without doing the computation twice.

And using a ref we might end up creating similar bugs as with atomified <-state.

orestis commented 5 years ago

I’ve commented on that react issue as pointed by Roman in Twitter: https://twitter.com/roman01la/status/1107695745493356545

https://github.com/facebook/react/issues/14476

Some similar efforts to bridge Hooks and ClojureScript are here: https://github.com/mhuebert/chia/blob/master/view/src/chia/view/hooks.cljs

roman01la commented 5 years ago

Hello! Do you have any ideas how this can be fixed for useState? I'm starting to think that Hooks API stands in the way of integrating React with ClojureScript when trying to provide idiomatic Clojure API for library users.

Including @mhuebert in this discussion, since he's working on a wrapper as well.

orestis commented 5 years ago

I don't have any specific thoughts in mind yet. I am working though on a real-world application that's built entirely on hx, so I'm using that as a real-life place to validate new ideas.

The thing that is really great with hooks though is that the API surface is rather small, so it's easy to write different wrappers and see how they behave. Their composability is what makes me want to keep pushing this, since doing the same with class-based components was pretty much impossible.

roman01la commented 5 years ago

Actually I think this is an easy fix for useState, if you wrap the hook you can compare values inside of the wrapper before setting a new value onto the hook https://github.com/roman01la/uix/commit/3665c631c120a218b912d662c2ee47c9c39b9c87

orestis commented 5 years ago

Yeah, that's the obvious fix. We discussed at length with @Lokeh whether it makes sense to expose the "ref" semantics of atoms, because they can be confusing if you're expecting normal clojure atom behaviour. See https://github.com/Lokeh/hx/issues/11#issuecomment-472741183

mhuebert commented 5 years ago
   (when (not= new-value value)
    (set-value new-value))

I was trying to think of values for which Clojure would say not= while Object.is would say true (ie. cases where set-value would be an undesired no-op), but the only thing I could think of is js/NaN, which is probably not an edge case to worry about.

orestis commented 5 years ago

I think that's a very good point and a test suite could be setup to explore this. If it's only js/NaN then perhaps it's not worth the hassle :)