reagent icon indicating copy to clipboard operation
reagent copied to clipboard

cursor watch not triggering when base atom is swapped.

Open retrogradeorbit opened this issue 8 years ago • 9 comments

When a watch is added to a cursor it only triggers when the cursor is changed directly with a swap!. It doesn't trigger when a change is achieved by swap!ing the base atom.

This code:

(def test-ratom (reagent/atom {:foo {:a 1} :bar {:b 10}}))

(defn watcher-debug [k a o n] (.log js/console "[WATCH]->" k a o n))

(def foo-cursor (reagent/cursor test-ratom [:foo]))
(def bar-cursor (reagent/cursor test-ratom [:bar]))

(add-watch foo-cursor :foo-watcher watcher-debug)
(add-watch bar-cursor :bar-watcher watcher-debug)

(println "swap! :foo via base ratom...")
(swap! test-ratom assoc-in [:foo :a] 2)

(println "swap! :foo via :foo cursor...")
(swap! foo-cursor assoc :a 3)

(println "swap! :bar via base ratom...")
(swap! test-ratom assoc-in [:bar :b] 11)

(println "swap! :bar via :bar cursor...")
(swap! bar-cursor assoc :b 12)

(println "swap! both :foo and :bar via base ratom...")
(swap! test-ratom #(-> %
                       (assoc-in [:foo :a] 4)
                       (assoc-in [:bar :b] 13)))

demonstrates the watches only firing on direct swaps.

swap! :foo via base ratom...
swap! :foo via :foo cursor...
[WATCH]-> foo-watcher 
r…t.r…m.Reaction {state: c…s.c…e.PersistentArrayMap, dirty_QMARK_: true, active_QMARK_: false, watching: null, watches: c…s.c…e.PersistentHashMap…}
 null Object {a: 2}
[WATCH]-> foo-watcher 
r…t.r…m.Reaction {state: c…s.c…e.PersistentArrayMap, dirty_QMARK_: true, active_QMARK_: false, watching: null, watches: c…s.c…e.PersistentHashMap…}
 Object {a: 2} Object {a: 3}
swap! :bar via base ratom...
swap! :bar via :bar cursor...
[WATCH]-> bar-watcher 
r…t.r…m.Reaction {state: c…s.c…e.PersistentArrayMap, dirty_QMARK_: true, active_QMARK_: false, watching: null, watches: c…s.c…e.PersistentHashMap…}
 null Object {b: 11}
[WATCH]-> bar-watcher 
r…t.r…m.Reaction {state: c…s.c…e.PersistentArrayMap, dirty_QMARK_: true, active_QMARK_: false, watching: null, watches: c…s.c…e.PersistentHashMap…}
 Object {b: 11} Object {b: 12}
swap! both :foo and :bar via base ratom...

Also note that on first cursor swap! the watcher is called twice. once from nil to the base value. And then once from the base value to the new value.

The behaviour should be consistent with base atom behaviour.

  1. When the base atom is changed, that should flow through to the cursor and trigger its watch
  2. The first cursor swap should only trigger one watch callback, from the old base value, to the new value.

retrogradeorbit avatar Jun 04 '16 05:06 retrogradeorbit

I can propagate the events manually to the cursors by adding a watcher to the root atom:

(add-watch test-ratom :propagate-child-cursors
           (fn [k a o n]
             (when (not= (:foo o) (:foo n))
               (reset! foo-cursor (:foo n)))
             (when (not= (:bar o) (:bar n))
               (reset! bar-cursor (:bar n)))))

And then things start to work as I expected. Is this by design? The problem of needing this extra code is I cannot just replace atoms with cursors. I need to know that I need to replace an atom with a cursor and a custom watcher to propagate root atom changes.

retrogradeorbit avatar Jun 04 '16 09:06 retrogradeorbit

@retrogradeorbit, could you try what you did, with a scenario where all the changes are happening inside a component.

For example, the following bit of code works perfectly fine for me:

(defonce test-ratom (reagent/atom {:foo {:a 1} :bar {:b 10}}))
(defn watcher-debug [k a o n] (.log js/console "[WATCH]->" k a o n))

(def foo-cursor (reagent/cursor test-ratom [:foo]))
(add-watch foo-cursor :foo-watcher watcher-debug)


(defn page
  []
  (js/console.log (clj->js @foo-cursor))
  [:button {:onClick (fn []
                       (swap! test-ratom assoc-in [:foo :a] (rand-int 1000)))}
   "Hello"])

On eact onClick the component, I see in my dev console that watcher-debug is being called for foo-cursor.

Reagent atoms/cursors take have some properties which are only apparent when they are part of a component.

ducky427 avatar Jun 06 '16 11:06 ducky427

Yes, that works. The difference is your code does a deref on the cursor. When you deref it to log it, magic happens.

Although most access to the cursor comes through a deref, not all does. You can also access the changes of state via a watch function.

Changing the pertinent parts of your example to:

(defn page
  []
  [:button {:onClick (fn []
                       (swap! test-ratom assoc-in [:foo :a] (rand-int 1000)))}
   "Hello"])

(go
  (<! (timeout 10000))
  (js/console.log (clj->js @foo-cursor)))

In the ten seconds before the deref fires, you can click on the Hello button as much as you want, and you will not see a watch fire, even though the state of the atom is changing!

Then the deref fires, and with that the watch function once, even though the atom has changed many times. it seems important work is being done in the state of the cursor on the deref implementation.

The code I am having problems with is a webgl canvas that cannot be mounted/unmounted with each prop change in the elements and reconstructing everything from that state. So it registers a :component-did-mount callback, and that callback adds a watch to the atom. Then, as things change in the atom, the watcher alters the webgl render. There are no derefs. The old and new state are passed into the watcher.

The reason I discovered this is the components originally did use their own atoms. And everything worked. I then wanted to unite a few of the states into a single atom, and I just passed in cursors, expecting the cursors to behave like the atoms. They did not. All the changes to the webgl were ignored. The component became completely static. This was surprising. I had believed that cursors behaves just like sub-atoms, but in this case they do not.

Now seeing it's magic in the deref, I tried to reimplement my root watcher event propagater as:

(add-watch test-ratom :foo-bar-cursor-update (fn [k a o n] @foo-cursor @bar-cursor))

And it worked! I think if cursors are intended as a drop in alternative to atoms, this has to work without the derefs.

EDIT: I'm now using this function to build cursors that behave like this: https://github.com/infinitelives/atelier/blob/master/src/atelier/core.cljs#L67

retrogradeorbit avatar Jun 06 '16 12:06 retrogradeorbit

@retrogradeorbit, ah. I see what you mean. Good catch.

So it looks like cursors are lazy in sense that they only update when deref'ed. Either that is a bug or the documentation is missing/not clear on the behaviour.

ducky427 avatar Jun 06 '16 13:06 ducky427

@retrogradeorbit, you may want to use (not (identical? ... instead of (not=... in your code.

ducky427 avatar Jun 06 '16 13:06 ducky427

Seems like the same/related issue as in #116 (see my comments there)

antishok avatar Jun 06 '16 13:06 antishok

@retrogradeorbit Cursors are lazy: the value doesn't change until the cursor is used. This is by design: for a cursor to be able to update when it is not used, it would have to set up a permanent watch on the atom. That, in turn, would require users of cursors to somehow "destroy" every cursor after use in order to remove that watch – otherwise you'd get a memory leak that would be very hard to find. (Also: lazy cursors will have much better performance).

But you're absolutely right that the documentation is lacking here. I'll add some warning.

As to possible solutions: the cleanest may be to simply deref the cursor in the render method of your component (that way you won't have to worry about removing watches when your component dies).

holmsand avatar Jun 07 '16 14:06 holmsand

I can understand your concern over memory leaks. My app has the cursor only constructed once so it's not as issue for me, but will be in other cases.

I can issue you a PR and add information to describe this effect to the cursor docstring if you haven't done so already?

retrogradeorbit avatar Jun 18 '16 03:06 retrogradeorbit

@retrogradeorbit I faced the same issue recently. If you want to use a non-lazy cursor, you can always make one with Entanglement the same exact way you would with Reagent.

(ent/cursor my-ratom [:my :path])

All watches will trigger automatically when my-ratom is modified.

The downside it that if you deref it inside a reagent component, it will be as if you derefed the entire ratom, instead of a reagent cursor.

Frozenlock avatar Aug 22 '16 23:08 Frozenlock