diff --git a/CHANGES.md b/CHANGES.md index 630012a..1b38104 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,6 +16,7 @@ - Add `re-frame.loggers/get-loggers` function to well, you know. - Added `clear-subscription-cache!` function. This should be used when hot reloading code to ensure that any bad subscriptions that cause rendering exceptions are removed. See [reagent-project/reagent#272](https://github.com/reagent-project/reagent/issues/272) for more details. - Added experimental tracing features. These are subject to change and remain undocumented at the moment. By default they are disabled, and will be completely compiled out by advanced optimisations. To enable them, set a [`:closure-defines`](https://www.martinklepsch.org/posts/parameterizing-clojurescript-builds.html) key to `{"re_frame.trace.trace_enabled_QMARK_" true}` +- [#223](https://github.com/Day8/re-frame/issues/223) When using `make-restore-fn`, dispose of any subscriptions that were created after the restore function was created. #### Fixes diff --git a/src/re_frame/core.cljc b/src/re_frame/core.cljc index 8510754..da13510 100644 --- a/src/re_frame/core.cljc +++ b/src/re_frame/core.cljc @@ -12,7 +12,8 @@ [re-frame.interceptor :as interceptor] [re-frame.std-interceptors :as std-interceptors :refer [db-handler->interceptor fx-handler->interceptor - ctx-handler->interceptor]])) + ctx-handler->interceptor]] + [clojure.set :as set])) ;; -- dispatch @@ -134,14 +135,14 @@ (fn [] ;; call `dispose!` on all current subscriptions which ;; didn't originally exist. - #_(->> subs/query->reaction - vals - (remove (set (vals subs-cache))) ;; - (map interop/dispose!) - (doall)) + (let [original-subs (set (vals subs-cache)) + current-subs (set (vals @subs/query->reaction))] + (doseq [sub (set/difference current-subs original-subs)] + (interop/dispose! sub))) - ;; reset the atoms - (reset! subs/query->reaction subs-cache) + ;; Reset the atoms + ;; We don't need to reset subs/query->reaction, as + ;; disposing of the subs removes them from the cache anyway (reset! registrar/kind->id->handler handlers) (reset! db/app-db app-db) nil))) diff --git a/test/re-frame/restore_test.cljs b/test/re-frame/restore_test.cljs new file mode 100644 index 0000000..5052f06 --- /dev/null +++ b/test/re-frame/restore_test.cljs @@ -0,0 +1,74 @@ +(ns re-frame.restore-test + (:require [cljs.test :refer-macros [is deftest async use-fixtures testing]] + [re-frame.core :refer [make-restore-fn reg-sub subscribe]] + [re-frame.subs :as subs])) + +;; TODO: future tests in this area could check DB state and registrations are being correctly restored. + +(use-fixtures :each {:before subs/clear-all-handlers!}) + +(defn one? [x] (= 1 x)) +(defn two? [x] (= 2 x)) + +(defn register-test-subs [] + (reg-sub + :test-sub + (fn [db ev] + (:test-sub db))) + + (reg-sub + :test-sub2 + (fn [db ev] + (:test-sub2 db)))) + +(deftest make-restore-fn-test + (testing "no existing subs, then making one subscription" + (register-test-subs) + (let [original-subs @subs/query->reaction + restore-fn (make-restore-fn)] + (is (zero? (count original-subs))) + @(subscribe [:test-sub]) + (is (one? (count @subs/query->reaction))) + (is (contains? @subs/query->reaction [[:test-sub] []])) + (restore-fn) + (is (zero? (count @subs/query->reaction)))))) + +(deftest make-restore-fn-test2 + (testing "existing subs, making more subscriptions" + (register-test-subs) + @(subscribe [:test-sub]) + (let [original-subs @subs/query->reaction + restore-fn (make-restore-fn)] + (is (one? (count original-subs))) + @(subscribe [:test-sub2]) + (is (contains? @subs/query->reaction [[:test-sub2] []])) + (is (two? (count @subs/query->reaction))) + (restore-fn) + (is (not (contains? @subs/query->reaction [[:test-sub2] []]))) + (is (one? (count @subs/query->reaction)))))) + +(deftest make-restore-fn-test3 + (testing "existing subs, making more subscriptions with different params on same subscriptions" + (register-test-subs) + @(subscribe [:test-sub]) + (let [original-subs @subs/query->reaction + restore-fn (make-restore-fn)] + (is (one? (count original-subs))) + @(subscribe [:test-sub :extra :params]) + (is (two? (count @subs/query->reaction))) + (restore-fn) + (is (one? (count @subs/query->reaction)))))) + +(deftest nested-restores + (testing "running nested restores" + (register-test-subs) + (let [restore-fn-1 (make-restore-fn) + _ @(subscribe [:test-sub]) + _ (is (one? (count @subs/query->reaction))) + restore-fn-2 (make-restore-fn)] + @(subscribe [:test-sub2]) + (is (two? (count @subs/query->reaction))) + (restore-fn-2) + (is (one? (count @subs/query->reaction))) + (restore-fn-1) + (is (zero? (count @subs/query->reaction)))))) diff --git a/test/re-frame/test_runner.cljs b/test/re-frame/test_runner.cljs index 7083d34..379bbef 100644 --- a/test/re-frame/test_runner.cljs +++ b/test/re-frame/test_runner.cljs @@ -7,7 +7,8 @@ [re-frame.interceptor-test] [re-frame.subs-test] [re-frame.fx-test] - [re-frame.trace-test])) + [re-frame.trace-test] + [re-frame.restore-test])) (enable-console-print!) @@ -21,7 +22,8 @@ 're-frame.interceptor-test 're-frame.subs-test 're-frame.fx-test - 're-frame.trace-test)) + 're-frame.trace-test + 're-frame.restore-test)) ;; ---- KARMA ----------------------------------------------------------------- @@ -31,4 +33,5 @@ 're-frame.interceptor-test 're-frame.subs-test 're-frame.fx-test - 're-frame.trace-test)) + 're-frame.trace-test + 're-frame.restore-test))