Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Semantics and API of <-state #41

Closed
lilactown opened this issue Mar 19, 2019 · 16 comments
Closed

Semantics and API of <-state #41

lilactown opened this issue Mar 19, 2019 · 16 comments

Comments

@lilactown
Copy link
Collaborator

lilactown commented Mar 19, 2019

I'm creating this issue to capture the discussion that has been happening across other issues and PRs around the <-state hook.

To summarize:

Currently, <-state returns an object that implements the IDeref / IReset / ISwap interfaces, allowing it to be used similar to how Clojure(Script)atoms are used.

This feels very familiar to me and many others because this is the primary way that Reagent provided state management.

However, there are several conflicts when trying to apply atom semantics to React's state management which all revolve around the fact that React doesn't apply state updates synchronously.

Issues that this brings in:

  1. Currently, we have a custom type that provides a thin implementation of the protocols on top of React's useState directly. This has been brought up (add an example and a possible solution for atomified state #38, Implement additional protocols for Atomified react-state #11) that it violates the expectations one has for calling swap! and then dereferencing in the same tick.

  2. If we adopt atom semantics whole-hog (e.g. actually use an atom and use add-watch), then we create circumstances where our atom state and React's state can diverge.

A simple example of this divergence I've taken from Dan Abramov's blog post The Complete Guide to useEffect:

(hx/defnc StateWithEffect [{:keys [receive]}]
  (let [count (hooks/<-state 0)]
    (hooks/<-effect
     (fn []
       (js/setTimeout
        (fn []
          (prn @count))
        3000)
       js/undefined))
    [:div {:on-click #(swap! count inc)}
     @count]))

Clicking this 5 times in a row, will give different results depending on if <-state return value is updated synchronously vs. asynchronously.

This problem gets worse when Concurrent React lands.

I think that this is due to a fundamental mismatch between the way we have handled state in the past, and how React wants to handle managing our state and doing computations. I am still in the process of grokking how React Hooks work and the concepts behind them, but this is what I understand so far:

React wants to own the scheduling of our application. This means that we tell React the what and the how, and React takes care of the when.

This is especially true for renders, but React wants to schedule computing state as well. This is something that I have only just understood. An example:

(def lifecycle (atom 0))

(hx/defnc WhenApplied [_]
  (reset! lifecycle :rendering)
  (let [count (hx.hooks/<-state 0)]
    (reset! lifecycle nil)
    [:div {:on-click #(swap! count (fn update [n]
                                     (prn "update:" @lifecycle)
                                     (inc n)))}
     @count]))

We notice that after the first click (not sure why this doesn't apply to the first one), the update fn is run during the :rendering phase; specifically when the corresponding useState hook is called.

What this means is that when Concurrent React is in full effect, the state computation will be scheduled alongside the accompanying render based on the priority of the dispatch. E.g. the state updates from a user typing in an input field might be applied before the state updates of a websocket connection, even though the typing events fired after the websocket events.

I tried to setup a simple example. It's not super powerful, but if you try it can illustrate the point:

(def schedule (atom :high))

(hx/defnc Scheduler [_]
  (let [updates (hooks/<-state [])]
    [:div
     [:div "Updates: " (prn-str @updates)]
     [:div
      [:input
       {:on-change (fn update []
                     (case @schedule
                       :low (do
                              (reset! schedule :high)
                              (scheduler/unstable_scheduleCallback
                               #(swap! updates conj :low)))
                       :high (do
                               (reset! schedule :low)
                               (swap! updates conj :high))))}]]
     [:div [:button {:on-click #(reset! updates [])} "reset"]]]))

(react-dom/render
  (hx/f [react/unstable_ConcurrentMode
         [Scheduler]]))

This will alternate between high and low scheduling priorities. If you type REALLY REALLY fast in order to get the page to start dropping frames, you can get multiple :high updates in a row. This is because React is prioritizing those updates over the :low ones when performance starts to degrade.

This means that we have to do one of two things if we want to adopt atom semantics for <-state hook:

  1. Short circuit React's scheduler so that our updates are always the same priority.

  2. Risk that our atom state and React's state get out of sync

If we do number 1, then we can never rise above Reagent/Rum's async-always behavior. If we do number 2... we can't do number 2 😝that's just a bug.

This is why I believe the best course of action is to... not use atoms or the IDeref / IReset / ISwap protocols. At this point I think that React has hit a pretty good sweet spot by just returning a tuple [state set-state] that can be re-named by the consumer.

Anyway, those are my thoughts. Let's continue the discussion here!

@lilactown
Copy link
Collaborator Author

cc @orestis @mhuebert @roman01la

Also I have created some devcards of the above examples in the master branch of this repo.

@lilactown
Copy link
Collaborator Author

lilactown commented Mar 19, 2019

Relevant article written by Andrew Clark: https://github.com/acdlite/rfcs/blob/context-write/text/0000-context-write.md#concurrent-invalidation

In it he talks about how maintaining state in a synchronous external store with concurrent renders can cause inconsistencies.

@DjebbZ
Copy link

DjebbZ commented Mar 20, 2019

Thanks for layout the issues so clearly. I would conclude the same thing as you, don't try to do non React thing, just embrace it.

But this raises a question for us Clojure(Script) developers: what could be the equivalent of global store in Clojure(Script) then? Should we just have a top level atom and add a watcher that re-render the top level component with the content of the atom passed as props (or React context) ? Use the useReducer hook and pass the dispatch function as props (or again pass it through context)? Just abandon Atoms and lose the observability they provide (this hurts somehow but may be ok)?

I'd love your thoughts on these questions, I don't have an answer.

@DjebbZ
Copy link

DjebbZ commented Mar 20, 2019

I've just read the proposal you linked from Andrew Clarke. Maybe we need to wait for a solution like this to allow using atoms and React together. What I mean is that we could reset! the atom in the contextDidUpdate callback, so that the UI and the state stay consistent. But not sure if it's doable or a good idea, would need to see what to come up with.

In the meantime, given React don't have stable APIs for scheduling state transitions... I don't know 😅

@lilactown
Copy link
Collaborator Author

Yeah. The global state management problem is one of the most interesting at the moment w.r.t. Concurrent React, which is probably one of the reasons they aren’t willing to do a stable release with it yet 😄 .

FWIW I have the very strange (in CLJS) opinion that maintaining state outside of the render tree is an anti pattern, but even trying to maintain “global” state inside the render tree is waiting on improvements to context.

I have one approach that I want to test before I call it completely infeasible to do without the React team improving context. It involves maintaining global state inside of a top-level useRef kept in context, and maintaining subscriptions inside of the components using local state. It could specifically take advantage of ClojureScript’s immutable data to be much more memory and CPU efficient.

I’ll see if I can come up with a POC in the next week or two - got to make sure I don’t spend too much of my limited brain power on this 😂

@DjebbZ
Copy link

DjebbZ commented Mar 20, 2019

I'm interested to see what you come up with. I also had the idea that keeping the global state inside the top-level component could be a solution. Looking forward to seeing it!

@lilactown
Copy link
Collaborator Author

lilactown commented Mar 21, 2019

I've created a PR that removes the Atom-like wrapper around useState, as well as optimizes certain cases where the state update fn might return a structurally equivalent but referentially different collection.

#42

This aligns us with the semantics of React and users can expect similar/API behavior as the React docs say.

Looking for feedback!

@DjebbZ
Copy link

DjebbZ commented Mar 22, 2019

React-redux seems to have found a solution, since they've released a beta of version 7, which uses hooks and seems to just store the "global" content inside the UI tree itself, to let React handle it. Admittedly I've glanced quickly only, so you may want to read it carefully https://www.reddit.com/r/reactjs/comments/b40y7u/reactredux_v700beta0_faster_performance_and_built/

@lilactown
Copy link
Collaborator Author

As far as I can tell, they’ve punted on trying to align Redux with Concurrent React’s scheduler. They’re essentially moving back to a similar architecture as v5, where they keep a synchronous store that dispatches renders to each component that subscribes to it.

AFAICT this is due to the fact that previous versions of Redux gave the guarantee that dispatch is synchronous, so that you can do something like:

dispatch(FOO_EVENT);

store.getState();

This is exactly the same expectation that is broken by representing React useState as a Clojure atom.

If we do not carry forward this expectation, I think we might be able to align ourselves with React’s scheduler and leverage its benefits when it finally lands in stable.

@lilactown
Copy link
Collaborator Author

Some discussions on the react-redux repo making for good reading:

reduxjs/react-redux#890

reduxjs/react-redux#1177

@orestis
Copy link
Member

orestis commented Mar 25, 2019

Something that becomes really obvious the moment you start storing state in React -- hot reloading becomes a little bit more annoying, as React will re-run all your hooks and re-fetch all data from the backend.

So the defonce trick for state that survives hot reloads doesn't really work.

My hacky workaround is to have a top-level defonce and then use that until the fresh data comes back from the backend, so at least then I don't "loading state" flashes when I work on presentational stuff.

@danielneal
Copy link
Contributor

I really value the approach in this project of providing a minimal wrapper, and respecting the semantics of React. The atom interface did look like a nice idea at first, but I think you're correct in identifying that there's a mismatch. On balance, I think I'd personally prefer the clarity of using the [state setState] tuple as proposed in your merge request. Thanks for all the think-effort on this :)

@lilactown
Copy link
Collaborator Author

@orestis check out #44

@caleb
Copy link

caleb commented Apr 3, 2019

I don't use hx yet (I'm using Rum right now), but I'm keeping my eye on it because of the new stuff coming down the React pipeline (hooks, concurrent, etc), but I think this would be a good change to make it more consistent with React.

Are there synchronization issues with the component's state and an atom when using the <-deref hook? I'm trying to wrap my head around how I would do state management with hooks in hx. Right now each line of my form subscribes to updates on the global atom when that line changes. This is for performance reasons so I'm not creating a react virtual Dom tree for my whole app on every keystroke.

@lilactown
Copy link
Collaborator Author

Yes, <-deref is a foot-gun in concurrent React. Either you skip out on using scheduling with React, or you risk getting out or sync with your state.

@frankiesardo
Copy link

This is a very interesting discussion, thank you all for sharing it here. Shame I only become aware of it today!

I've been tinkering a lot about using something like citrus together with the React hooks. I like the colocation of state and side effect and the ability of supplying different effect handlers for e.g. testing that citrus gives you.

This is my best shot at recreating reducers with side effects in pure React, no extra atoms and observers.

https://gist.github.com/frankiesardo/d135c73d74695d83277c4294a987006f

I would love to hear the thoughts of people in this conversation since you've analysed the problem quite deeply

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants