-
Notifications
You must be signed in to change notification settings - Fork 413
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
cursor watch not triggering when base atom is swapped. #244
Comments
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, 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 Reagent atoms/cursors take have some properties which are only apparent when they are part of a component. |
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, 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. |
@retrogradeorbit, you may want to use |
Seems like the same/related issue as in #116 (see my comments there) |
@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). |
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 I faced the same issue recently. (ent/cursor my-ratom [:my :path]) All watches will trigger automatically when 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. |
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:
demonstrates the watches only firing on direct swaps.
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.
The text was updated successfully, but these errors were encountered: