Skip to content

Commit

Permalink
Allow subscribe to be safely called from any context
Browse files Browse the repository at this point in the history
The `warn-when-not-reactive` warnings are no longer valid if we bypass
the subscription cache when not in a reagent component, so they've been
removed.

This should address issue day8#753

Still needs some testing, and probably some documentation cleanup
  • Loading branch information
dannyfreeman committed Mar 6, 2022
1 parent 69cf395 commit 3c1cffe
Showing 1 changed file with 23 additions and 29 deletions.
52 changes: 23 additions & 29 deletions src/re_frame/subs.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,29 @@
(defn cache-and-return
"cache the reaction r"
[query-v dynv r]
(let [cache-key [query-v dynv]]
;; when this reaction is no longer being used, remove it from the cache
(add-on-dispose! r #(trace/with-trace {:operation (first-in-vector query-v)
:op-type :sub/dispose
:tags {:query-v query-v
:reaction (reagent-id r)}}
(swap! query->reaction
(fn [query-cache]
(if (and (contains? query-cache cache-key) (identical? r (get query-cache cache-key)))
(dissoc query-cache cache-key)
query-cache)))))
;; cache this reaction, so it can be used to deduplicate other, later "=" subscriptions
(swap! query->reaction (fn [query-cache]
(when debug-enabled?
(when (contains? query-cache cache-key)
(console :warn "re-frame: Adding a new subscription to the cache while there is an existing subscription in the cache" cache-key)))
(assoc query-cache cache-key r)))
(trace/merge-trace! {:tags {:reaction (reagent-id r)}})
r)) ;; return the actual reaction
;; Only cache the reaction when subscribe is called from a reactive context
;; where a reagent component will be able to purge the reaction from the subscription cache
;; when the component is dismounted.
(when (reactive?)
(let [cache-key [query-v dynv]]
;; when this reaction is no longer being used, remove it from the cache
(add-on-dispose! r #(trace/with-trace {:operation (first-in-vector query-v)
:op-type :sub/dispose
:tags {:query-v query-v
:reaction (reagent-id r)}}
(swap! query->reaction
(fn [query-cache]
(if (and (contains? query-cache cache-key) (identical? r (get query-cache cache-key)))
(dissoc query-cache cache-key)
query-cache)))))
;; cache this reaction, so it can be used to deduplicate other, later "=" subscriptions
(swap! query->reaction (fn [query-cache]
(when debug-enabled?
(when (contains? query-cache cache-key)
(console :warn "re-frame: Adding a new subscription to the cache while there is an existing subscription in the cache" cache-key)))
(assoc query-cache cache-key r)))
(trace/merge-trace! {:tags {:reaction (reagent-id r)}})))
r) ;; Always return the actual reaction, even if it was not cached.

(defn cache-lookup
([query-v]
Expand All @@ -63,17 +67,8 @@

;; -- 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 @@ -92,7 +87,6 @@
(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 3c1cffe

Please sign in to comment.