nosco / hx

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

Remove atomified <-state, optimizations #42

Closed lilactown closed 5 years ago

lilactown commented 5 years ago

This MR removes the Atomified wrapper returned by <-state. It implements a couple of new features:

  1. <-state returns a tuple [state set-state]
  2. set-state will prevent re-renders triggered by structurally equivalent state objects
  3. set-state can be used with multiple arguments like swap!: (set-state assoc :new-key "new-value")
  4. Adds a new <-value hook that can be used to optimize <-effect, <-memo, etc.
DjebbZ commented 5 years ago

Will take a longer look soon, but after a quick glance, one thing that bothers (if I can say so) is the = check. Are you assuming that the value is a js object or a CLJ data structure? I'm saying this because I've been bitten by a similar thing in Citrus, where we had a long vector of nested maps coming from the backend and only the last item was different, so this = was taking a lot of time and blocking the UI. Should it be better to leave the check out by default and let React do its check (I know the rules documented somewhere), and offer an option to actually do the check?

I haven't looked at the other changes yet in the PR.

lilactown commented 5 years ago

Perhaps you're right. We have an issue over at https://github.com/facebook/react/issues/15154 where we're trying to figure out if there's some extra optimizations we can do w.r.t. state updates.

I think that in practice, the chance that a state operation returns a structurally equivalent but referentially different value is rare. So might be good to keep the hot path clear of that.

orestis commented 5 years ago

I've always thought that the user should be able to supply their own comparison function, because they're best positioned to know what kind of data structures they use on a case-by-case basis.

So I think an API could look like so:

[foo set-foo] (<-state 0) ;; fall back to whatever React is doing with Object.is
[foo set-foo] (<-state :foo keyword-identical?) ;; only going to use keywords
[foo set-foo] (<-state {:some "thing"} =) ;; small maps with big ramifications
[foo set-foo] (<-state {:some "thing"} identical?) ;; big maps, prefer to re-render

(the last case could be omitted as identical? is just ===).

DjebbZ commented 5 years ago

I would go further and provide nothing. The user can just do the custom comparison him/herself before calling set-foo (this is what we ended up doing in our case). The good thing with this solution is that this code doesn't even have to be in hx at all, easy to do outside.

lilactown commented 5 years ago

I think that you will want to be able to do the quality check in the state computation, so that you can accommodate state changes occurring between when you call set-foo and when the computation you dispatched actually occurs. This can't happen now, but will in Concurrent mode.

Another option for users is to use <-reducer instead, like the React team has suggested for most non-toy situations.