rum icon indicating copy to clipboard operation
rum copied to clipboard

Pass "old state" to `will-update` and `did-update` hooks

Open mmavko opened this issue 9 years ago • 8 comments

I can compare old and new state in transfer-state hook, but it's not called on forceUpdate (and that's what's done in rum/request-render called by rum/local mixin among other possible cases).

Here's jsFiddle showing how componentWillReceiveProps works: http://jsfiddle.net/kb3gN/10756/

mmavko avatar Apr 10 '15 12:04 mmavko

Why should state change if forceUpdate was called? What changes are you trying to catch? Can you give me a real example?

On Fri, Apr 10, 2015 at 6:57 PM Myron Mavko [email protected] wrote:

I can compare old and new state in transfer-state hook, but it's not called on forceUpdate (and that's what's done in rum/request-render called by rum/local mixin among other possible cases).

Here's jsFiddle showing how componentWillReceiveProps works: http://jsfiddle.net/kb3gN/10756/

— Reply to this email directly or view it on GitHub https://github.com/tonsky/rum/issues/26.

tonsky avatar Apr 12 '15 18:04 tonsky

Consider you have a mixin mixin1 that marks some state flag flag under specific conditions. Another mixin mixin2 performs some side effect upon flag change.

;; NOT USABLE - CONCEPT CODE

(def ^:dynamic *flagged* false)

(def mixin1
  {:will-mount (fn [state]
                 (assoc state :flag *flagged*))
   :did-mount (fn [{cmp :rum/react-component :as state}]
                (go
                  (<! (timeout 1000))
                  (set! *flagged* true)
                  (rum/request-render cmp)))
   :will-update (fn [old-state state]
                  (assoc state :flag *flagged*))})

(def mixin2
  {:will-update (fn [old-state state]
                  (if (and (not (:flag old-state)) (:flag state))
                    (println "at last!")))})

(defc C < mixin1 mixin2 [:div "a component"])

In order to handle this case component state should be recreated (through :transfer-state) on each render (on React's componentWillUpdate), not only on componentWillReceiveProps.

mmavko avatar Apr 14 '15 16:04 mmavko

Ok, I still don’t understand neither the problem nor the solution. Why :transfer-state is a bad place to compare states?

tonsky avatar Apr 15 '15 08:04 tonsky

Because :transfer-state won't be called on (rum/request-render cmp).

Ok, probably the issue can be suspended till more obvious practical application found :)

mmavko avatar Apr 15 '15 09:04 mmavko

It won’t. But state won’t change as well?

tonsky avatar Apr 15 '15 09:04 tonsky

:will-update of mixin1 will change it (eventually) in my example. And :will-update of mixin2 is willing to catch that change.

mmavko avatar Apr 15 '15 09:04 mmavko

Right. Thanks for the explanation. I see it now (probably :) ) Rum definitely need simplification of state model. It’s too hard to walk all these possible state transitions in a head. Your problem cannot be easily fixed, because Rum has no notion of old-state. As component goes through many mixins, there’s miriad of intermediate states and they always might be considered “old” or “new” at some point. I will see what I can do about it

tonsky avatar Apr 15 '15 09:04 tonsky

I can weigh in on another example here:

When I tried to port the componentDidUpdate from here: https://github.com/tastejs/todomvc/blob/gh-pages/examples/react/js/todoItem.jsx, I couldn't access the old local state - I know that would be difficult, since the actual local state is in its own atom, and that will change before components are re-rendered..

The way I worked around it was to write a mixin for :did-update:

(defn cursor-to-end [local-key trigger-edit-key editref]
     {:did-update
      (fn [state]
        (when (get @(get state local-key) trigger-edit-key)
          (let [node (rum/ref-node state editref)]
            (.focus node)
            (.setSelectionRange node (.-length (.-value node)) (.-length (.-value node)))
            (swap! (get state local-key) dissoc trigger-edit-key)))
        state)})

(rum/defcs todo-item
  < (rum/local {:edit-text ""} ::local) (cursor-to-end ::local :start-edit "editField")
 ... )

And inside the component have an on-click that sets the :start-edit which the mixin unsets as part of did-update

A bit painful to figure out, but not unworkable.

CmdrDats avatar Nov 08 '16 20:11 CmdrDats