Allow subscribe to be safely called from any context

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 #753

This change probably requires some documentation cleanup

Author: dannyfreeman

src/re_frame/subs.cljc

@@ -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]
@@ -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}}
@@ -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

test/re_frame/subs_test.cljs

@@ -3,6 +3,7 @@
             [reagent.ratom     :as r :refer-macros [reaction]]
             [re-frame.subs     :as subs]
             [re-frame.db       :as db]
+            [re-frame.interop]
             [re-frame.core     :as re-frame]))
 
 (test/use-fixtures :each {:before (fn [] (subs/clear-all-handlers!))})
@@ -87,16 +88,37 @@
       (swap! side-effect-atom inc)
       (reaction @db)))
 
- (let [test-sub (subs/subscribe [:side-effecting-handler])]
-    (reset! db/app-db :test)
-    (is (= :test @test-sub))
-    (is (= @side-effect-atom 1))
-    (subs/subscribe [:side-effecting-handler])  ;; this should be handled by cache
-    (is (= @side-effect-atom 1))
-    (subs/subscribe [:side-effecting-handler :a]) ;; should fire again because of the param
-    (is (= @side-effect-atom 2))
-    (subs/subscribe [:side-effecting-handler :a]) ;; this should be handled by cache
-    (is (= @side-effect-atom 2))))
+  (testing "Inside of a reactive context"
+    (with-redefs [re-frame.interop/reactive? (constantly true)]
+      (let [test-sub (subs/subscribe [:side-effecting-handler])]
+         (reset! db/app-db :test)
+         (is (= :test @test-sub))
+         (is (= @side-effect-atom 1))
+         (subs/subscribe [:side-effecting-handler])  ;; this should be handled by cache
+         (is (= @side-effect-atom 1))
+         (subs/subscribe [:side-effecting-handler :a]) ;; should fire again because of the param
+         (is (= @side-effect-atom 2))
+         (subs/subscribe [:side-effecting-handler :a]) ;; this should be handled by cache
+         (is (= @side-effect-atom 2)))))
+
+  (subs/clear-subscription-cache!)
+  (reset! side-effect-atom 0)
+
+  (testing "Outside a reactive context"
+    (let [test-sub (subs/subscribe [:side-effecting-handler])]
+      (is (= :test @test-sub))
+      (is (= @side-effect-atom 1)
+          "Subscribed once")
+      (subs/subscribe [:side-effecting-handler])
+      (is (= @side-effect-atom 2)
+          "Subscribed twice and the cache was never populated.")
+      (with-redefs [re-frame.interop/reactive? (constantly true)]
+        (subs/subscribe [:side-effecting-handler :a])
+        (is (= @side-effect-atom 3)
+            "Fires again because of the param, but this time the cache is populated"))
+      (subs/subscribe [:side-effecting-handler :a])
+      (is (= @side-effect-atom 3)
+          "The cache was already populated, so the cached reaction is used."))))
 
 ;============== test clear-subscription-cache! ================
 
@@ -105,14 +127,15 @@
    :clear-subscription-cache!
    (fn clear-subs-cache [db _] 1))
 
-  (testing "cold cache"
-    (is (nil? (subs/cache-lookup [:clear-subscription-cache!]))))
-  (testing "cache miss"
-    (is (= 1 @(subs/subscribe [:clear-subscription-cache!])))
-    (is (some? (subs/cache-lookup [:clear-subscription-cache!]))))
-  (testing "clearing"
-    (subs/clear-subscription-cache!)
-    (is (nil? (subs/cache-lookup [:clear-subscription-cache!])))))
+  (with-redefs [re-frame.interop/reactive? (constantly true)]
+    (testing "cold cache"
+      (is (nil? (subs/cache-lookup [:clear-subscription-cache!]))))
+    (testing "cache miss"
+      (is (= 1 @(subs/subscribe [:clear-subscription-cache!])))
+      (is (some? (subs/cache-lookup [:clear-subscription-cache!]))))
+    (testing "clearing"
+      (subs/clear-subscription-cache!)
+      (is (nil? (subs/cache-lookup [:clear-subscription-cache!]))))))
 
 ;============== test register-pure macros ================