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

Watched subscriptions are not disposed #9

Open
dmitryn opened this issue Mar 16, 2021 · 2 comments
Open

Watched subscriptions are not disposed #9

dmitryn opened this issue Mar 16, 2021 · 2 comments

Comments

@dmitryn
Copy link

dmitryn commented Mar 16, 2021

Hello 👋

I think there is unexpected behavior regarding subscriptions disposal.

If you deref reagent atom outside of reagent rendering context, dependent subscription are not updated inside a watching list of parent subscription. So when you dispose! it, dependent subs are not getting disposed.
Instead of using deref, should reagent.ratom/run be used which properly updates ratom watching list.

Here is the code snippet which demonstrates the issue:

(ns inject-sub-cofx
  (:require [re-frame.core :refer [reg-sub subscribe]))


(reg-sub :bar
  (fn [db]
    (count db)))

(reg-sub :foo
  (fn [_]
    (subscribe [:bar]))
  (fn [a _] a))
             
             
(comment
 (def sub (subscribe [:foo]))
 (deref sub)
 (.-watching sub) ;; => nil
 (reagent.ratom/dispose! sub)
 (filter #(= :bar (get-in % [0 0 0])) @re-frame.subs/query->reaction) ; => ([[[:bar] []] #object[reagent.ratom.Reaction {:val 67}]]))
 (reagent.ratom/run sub)
 (.-watching sub) ;; => #js[#object[reagent.ratom.Reaction {:val 67}] #object[reagent.ratom.Reaction {:val 67}]]
 (reagent.ratom/dispose! sub)
 (filter #(= :bar (get-in % [0 0 0])) @re-frame.subs/query->reaction)) ;; => ()

I could make a PR if you think this is a good idea.

I also raised this question in clojurians slack, but didn't get any response.

@green-coder
Copy link

Hi @dmitryn, would you have a fix for that issue? Even if the project does not appear to be maintained, its users would be very happy to get access to a PR.

@dmitryn
Copy link
Author

dmitryn commented May 26, 2021

Hello @green-coder yes, I have used the same fix you applied in https://github.com/green-coder/re-frame-utils/commit/c2840ce8cbe3e7900cc5a8ae893950044f852dbb

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