Watched subscriptions are not disposed in inject-sub-cofx interceptor

[dmitryn]

Hi 👋

Issue is not directly related to re-frame, but to the re-frame-utils library which uses approach described in re-frame docs and in the comments of this issue

I also created an issue in re-frame-utils but it seems like library is not actively maintained so i'm writing here.

Here is the summary of an issue:

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)) ;; => ()

We actively using re-frame-utils library in our project (forked version) and i think we need to apply proposed patch (using reagent.ratom/run instead of deref).

Please confirm or tell my assumption is wrong. Thanks.

[kimo-k]

This behavior is a consequence of creating both subscriptions outside of a reactive context. Reagent doesn't do any reference-counting in that case.

I agree that re-frame-utils should probably sync a reaction with the reactions it watches before disposing it.

I don't think we will merge re-frame-utils, so I can't directly do much. Calling run might come in handy to implement our future api, but that alone doesn't solve the broader issue.

We discuss the concern of subscription lifecycle in detail on #680. I hope to get a solution out soon. It should be more friendly & feature-complete than re-frame-utils.