Skip to content

Commit

Permalink
Warn user when calling subscribe outside of a reactive context
Browse files Browse the repository at this point in the history
Calling subscribe outside of a reactive context will cache the
subscription, but there will be no ratoms around to remove the
subscription from the cache when the ratom is disposed. This can lead
to memory leaks where subscriptions stay in the cache indefinately.

This should fix issue #740
  • Loading branch information
dannyfreeman authored and superstructor committed Mar 4, 2022
1 parent c7c8b27 commit 6cd9341
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
4 changes: 4 additions & 0 deletions src/re_frame/interop.clj
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,7 @@
"Doesn't make sense in a Clojure context currently."
[reactive-val]
"rx-clj")

(defn reactive?
[]
true)
4 changes: 4 additions & 0 deletions src/re_frame/interop.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,7 @@
reagent.ratom/Track "tr"
"other")
(hash reactive-val))))

(defn reactive?
[]
(reagent.ratom/reactive?))
13 changes: 11 additions & 2 deletions src/re_frame/subs.cljc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(ns re-frame.subs
(:require
[re-frame.db :refer [app-db]]
[re-frame.interop :refer [add-on-dispose! debug-enabled? make-reaction ratom? deref? dispose! reagent-id]]
[re-frame.interop :refer [add-on-dispose! debug-enabled? make-reaction ratom? deref? dispose! reagent-id reactive?]]
[re-frame.loggers :refer [console]]
[re-frame.utils :refer [first-in-vector]]
[re-frame.registrar :refer [get-handler clear-handlers register-handler]]
Expand Down Expand Up @@ -61,11 +61,19 @@
([query-v dyn-v]
(get @query->reaction [query-v dyn-v])))


;; -- subscribe ---------------------------------------------------------------

(defn warn-when-not-reactive
[]
(when (and debug-enabled? (not (reactive?)))
(console :warn
"re-frame: Subscribe was called outside of a reactive context.\n"
"See: https://day8.github.io/re-frame/FAQs/UseASubscriptionInAJsEvent/\n"
"https://day8.github.io/re-frame/FAQs/UseASubscriptionInAnEventHandler/")))

(defn subscribe
([query]
(warn-when-not-reactive)
(trace/with-trace {:operation (first-in-vector query)
:op-type :sub/create
:tags {:query-v query}}
Expand All @@ -84,6 +92,7 @@
(cache-and-return query [] (handler-fn app-db query)))))))

([query dynv]
(warn-when-not-reactive)
(trace/with-trace {:operation (first-in-vector query)
:op-type :sub/create
:tags {:query-v query
Expand Down

0 comments on commit 6cd9341

Please sign in to comment.