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

Possible bug: watches are dropped #33

Closed
davesann opened this issue Jan 2, 2015 · 5 comments
Closed

Possible bug: watches are dropped #33

davesann opened this issue Jan 2, 2015 · 5 comments

Comments

@davesann
Copy link

davesann commented Jan 2, 2015

I have been investigating freactive and came across the following - which appears to be a bug.

I was testing out cursors and compound state access in rx.

I created a "complex-state" that included a mouse position and time.
mouse position is updated on mouse move
time is updated by a setInterval.

The bug appears to be that the watch on the time state or cursor is lost when the mouse is moved. i.e the reactions on time-cursor work but then stop when the mouse is moved.

Another thing that I noticed was that repeatedly calling (rdom/mount! root (view)) will keep adding watches. Many more that would seem necessary if mount is replacing the view.

Are these errors on my part - or is there an issue here?

Code below:

(ns test-freactive
  (:require 
    [freactive.core :as rc]
    [freactive.dom  :as rdom]
    )
  (:require-macros
    [freactive.macros :refer [debug-rx rx]]))

(enable-console-print!)

(defn now []
  (.getTime (js/Date.)))


(defonce root (rdom/append-child! (.-body js/document) [:div#root]))


(defonce independent-time-state (rc/atom {:time nil}))

(defonce complex-state (rc/atom {:mouse-pos nil
                                 :time nil}))

(defonce mouse-pos-cursor  (rc/cursor complex-state [:mouse-pos]))
(defonce time-cursor (rc/cursor complex-state [:time]))
(defonce schedule-cursor   (rc/cursor complex-state [:interval]))


(defn update-time! []
  (reset! time-cursor (now))
  (reset! independent-time-state {:time (now)})
  #_(swap! complex-state update-in [:time] (fn [_] (now))))

(defn schedule []
  (when-let [interval @schedule-cursor]
    (js/clearInterval interval))
  (let [interval (js/setInterval update-time! 100)] 
    (reset! schedule-cursor interval)))

(defn view []
  [:div
   {:width "100%" :height "100%" :style {:border "1px solid black"}
    :on-mousemove (fn [e] (reset! mouse-pos-cursor [(.-clientX e) (.-clientY e)]))}
   [:h1 "Hello World!"]
   #_[:p "Your mouse is at: " (rx (str @mouse-pos-cursor (now)))]
   [:p "Basic Time is: "        (rx 
                                 (str @independent-time-state))]

   [:p "cursor Time is: "        (rx 
                                  (str @time-cursor))]

  #_ [:p "lookup time is: "      (rx (str (:time @complex-state)))]



   (rx [:div 
        [:p "full-state  is:" (str @complex-state)]
        [:p "num watches on full state: " (str (count (.-watches complex-state)))]
        [:p "num watches on time update cursor: " (str (count (.-watches time-cursor)))]])

   ])

(defn go-freactive []
  (rdom/mount! root (view)))

(schedule)
(go-freactive)

@aaronc
Copy link
Owner

aaronc commented Jan 2, 2015

Are you using 0.1.0 or 0.2.0-SNAPSHOT? A lot has changed internally since 0.1.0... I will push another snapshot release soon because the current one is already significantly out of date. There were definitely memory leaks in 0.1.0 that are now resolved.

@aaronc
Copy link
Owner

aaronc commented Jan 2, 2015

Latest snapshot is up on Clojars. Dropped in your code quickly and didn't notice anything unusual - maybe you can check it out. Thank you for submitting a full example.

@aaronc
Copy link
Owner

aaronc commented Jan 2, 2015

Btw, you need to change your last rx to:

   (rx [:div 
        [:p "full-state  is:" (str @complex-state)]
        [:p "num watches on full state: " (str (.-watchers complex-state))]
        [:p "num watches on time update cursor: " (str (.-watchers time-cursor))]])

@davesann
Copy link
Author

davesann commented Jan 2, 2015

That has resolved the issue.

I do get compile errors with the snapshot though.

WARNING: Use of undeclared Var freactive.core/this at line 383 ... freactive-0.2.0-SNAPSHOT.jar!/freactive/core.cljs
WARNING: Wrong number of args (4) passed to freactive.dom/bind-attr! at line 422 freactive-0.2.0-SNAPSHOT.jar!/freactive/dom.cljs

I did notice that the number of watches varies - this could be due to when they are being re-rendered. The update cursor is generally 1 but goes from 1 to 0 and back (this seems reasonable). the full state is generally 4 but jumps up and sown to to 5 when moving the mouse - that looked a little suspicious. But there may be a good reason.

I checked the repeated call of mount and that was also resolved.

When do you plan a 0.2.0 release?

Thanks

@aaronc
Copy link
Owner

aaronc commented Jan 2, 2015

Yep, there's still some cleanup to do.

Watches are added and removed dynamically to prevent spurious change notifications so it's expected they will go up and down - what we don't want to see is them going up out of control or disappearing unexpectedly.

Hoping to resolve #22 and #8 before 0.2.0 is released - hopefully soon, but depends on when I get free time.

@aaronc aaronc closed this as completed Jan 2, 2015
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

2 participants