den1k / re-frame-utils

Re-frame extensions
MIT License
61 stars 19 forks source link

Watched subscriptions are not disposed #9

Open dmitryn opened 3 years ago

dmitryn commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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