From b528e497ee593b12d01a8f23253f6183133947e6 Mon Sep 17 00:00:00 2001 From: Christian Paul Dehli Date: Fri, 14 Jun 2024 09:39:39 -0400 Subject: [PATCH 1/7] group listener notifications --- core/src/refx/subs.cljc | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/core/src/refx/subs.cljc b/core/src/refx/subs.cljc index 5e5d53b..fa335c5 100644 --- a/core/src/refx/subs.cljc +++ b/core/src/refx/subs.cljc @@ -76,6 +76,38 @@ (doseq [[_ sub] @sub-cache] (dispose! sub))) +(defonce ^:private listeners-state + (atom {:counter 0 :pending (sorted-map)})) + +(defn- invoke-listener + "This function is responsible for ensuring that signal listeners + (from DynamicSubs) are called before triggering regular listeners + (eg: added via use-sub hook). Listeners are triggered in the order they + were registered. This function ensures that one db update will only trigger + a single render." + [listener-key listener-fn] + (swap! listeners-state (fn [state] + (cond-> (update state :counter inc) + (not (signal? listener-key)) + (update :pending assoc listener-key listener-fn)))) + + (when (signal? listener-key) + (listener-fn)) + + (interop/next-tick + (fn [] + (let [listener-fns (atom nil)] + (swap! listeners-state (fn [state] + (let [{:keys [counter pending] :as new-state} + (update state :counter dec)] + (if (zero? counter) + (do + (reset! listener-fns pending) + (assoc new-state :pending (sorted-map))) + new-state)))) + (doseq [[_ f] @listener-fns] + (f)))))) + (deftype Listeners [^:mutable listeners] Object (empty? [_] (empty? listeners)) @@ -84,8 +116,8 @@ (remove [_ k] (set! listeners (dissoc listeners k))) (notify [_] - (doseq [[_ f] listeners] - (f)))) + (doseq [[k f] listeners] + (invoke-listener k f)))) (defn- make-listeners [] (Listeners. nil)) From fad37c773647c5ba3b8b360605fedef87c53d2a9 Mon Sep 17 00:00:00 2001 From: Christian Paul Dehli Date: Fri, 14 Jun 2024 13:17:47 -0400 Subject: [PATCH 2/7] properly sort listeners --- core/src/refx/hooks.cljs | 2 +- core/src/refx/subs.cljc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/refx/hooks.cljs b/core/src/refx/hooks.cljs index 2b540af..5db5a23 100644 --- a/core/src/refx/hooks.cljs +++ b/core/src/refx/hooks.cljs @@ -28,7 +28,7 @@ (react/useMemo (fn [] [(fn [callback] - (let [key (str "use-sub-" (swap! use-sub-counter inc))] + (let [key {:index (swap! use-sub-counter inc)}] (subs/-add-listener sub key callback) #(subs/-remove-listener sub key))) (fn [] diff --git a/core/src/refx/subs.cljc b/core/src/refx/subs.cljc index fa335c5..850a031 100644 --- a/core/src/refx/subs.cljc +++ b/core/src/refx/subs.cljc @@ -77,7 +77,7 @@ (dispose! sub))) (defonce ^:private listeners-state - (atom {:counter 0 :pending (sorted-map)})) + (atom {:counter 0 :pending (sorted-map-by :index)})) (defn- invoke-listener "This function is responsible for ensuring that signal listeners @@ -103,7 +103,7 @@ (if (zero? counter) (do (reset! listener-fns pending) - (assoc new-state :pending (sorted-map))) + (assoc new-state :pending (sorted-map-by :index))) new-state)))) (doseq [[_ f] @listener-fns] (f)))))) From a7e49ff93bc03ebdc48d43d947defb276d603e1f Mon Sep 17 00:00:00 2001 From: Christian Paul Dehli Date: Fri, 14 Jun 2024 15:26:36 -0400 Subject: [PATCH 3/7] use empty for sorted-map --- core/src/refx/subs.cljc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/refx/subs.cljc b/core/src/refx/subs.cljc index 850a031..30b9dea 100644 --- a/core/src/refx/subs.cljc +++ b/core/src/refx/subs.cljc @@ -103,7 +103,7 @@ (if (zero? counter) (do (reset! listener-fns pending) - (assoc new-state :pending (sorted-map-by :index))) + (update new-state :pending empty)) new-state)))) (doseq [[_ f] @listener-fns] (f)))))) From 8c0aa1128e0f921673f43a01ae1858877a223b6f Mon Sep 17 00:00:00 2001 From: Christian Paul Dehli Date: Sun, 16 Jun 2024 20:43:52 -0400 Subject: [PATCH 4/7] add some comments --- core/src/refx/subs.cljc | 46 ++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/core/src/refx/subs.cljc b/core/src/refx/subs.cljc index 30b9dea..ca68b34 100644 --- a/core/src/refx/subs.cljc +++ b/core/src/refx/subs.cljc @@ -86,27 +86,31 @@ were registered. This function ensures that one db update will only trigger a single render." [listener-key listener-fn] - (swap! listeners-state (fn [state] - (cond-> (update state :counter inc) - (not (signal? listener-key)) - (update :pending assoc listener-key listener-fn)))) - - (when (signal? listener-key) - (listener-fn)) - - (interop/next-tick - (fn [] - (let [listener-fns (atom nil)] - (swap! listeners-state (fn [state] - (let [{:keys [counter pending] :as new-state} - (update state :counter dec)] - (if (zero? counter) - (do - (reset! listener-fns pending) - (update new-state :pending empty)) - new-state)))) - (doseq [[_ f] @listener-fns] - (f)))))) + (let [listener-fn-this-tick (atom nil)] + (swap! listeners-state (fn [state] + (let [new-state (update state :counter inc)] + (if (signal? listener-key) + ;; For the case of DynamicSub, we need to call its + ;; listener this tick to trigger dependent subs + (do (reset! listener-fn-this-tick listener-fn) + new-state) + (update new-state :pending assoc listener-key listener-fn))))) + + (when-let [f @listener-fn-this-tick] + (f)) + + (interop/next-tick + (fn [] + (let [listener-fns (atom nil)] + (swap! listeners-state (fn [state] + (let [{:keys [counter pending] :as new-state} + (update state :counter dec)] + (if (zero? counter) + (do (reset! listener-fns pending) + (update new-state :pending empty)) + new-state)))) + (doseq [[_ f] @listener-fns] + (f))))))) (deftype Listeners [^:mutable listeners] Object From 392ec270c6e743be06d9593825b679f389c8fbd5 Mon Sep 17 00:00:00 2001 From: Christian Paul Dehli Date: Tue, 18 Jun 2024 08:21:40 -0400 Subject: [PATCH 5/7] add listener test --- core/src/refx/subs.cljc | 4 +++- core/test/refx/subs_test.cljs | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/core/src/refx/subs.cljc b/core/src/refx/subs.cljc index ca68b34..0d0fccc 100644 --- a/core/src/refx/subs.cljc +++ b/core/src/refx/subs.cljc @@ -77,7 +77,9 @@ (dispose! sub))) (defonce ^:private listeners-state - (atom {:counter 0 :pending (sorted-map-by :index)})) + (letfn [(comparator [a b] + (compare (:index b) (:index a)))] + (atom {:counter 0 :pending (sorted-map-by comparator)}))) (defn- invoke-listener "This function is responsible for ensuring that signal listeners diff --git a/core/test/refx/subs_test.cljs b/core/test/refx/subs_test.cljs index a7f6fa8..c0c6d7b 100644 --- a/core/test/refx/subs_test.cljs +++ b/core/test/refx/subs_test.cljs @@ -104,3 +104,39 @@ (is (empty? @subs/sub-cache)) (done)) 10))))) + +(deftest listener-ordering + (let [source (atom 0)] + (subs/register :a (constantly source) identity) + (subs/register :b (constantly source) identity) + (subs/register :c (constantly (subs/sub [:b (subs/sub [:a])])) identity) + (subs/register :d (constantly [(subs/sub [:b]) (subs/sub [:c])]) identity) + (let [sub-a (subs/sub [:a]) + sub-b (subs/sub [:b]) + sub-c (subs/sub [:c]) + sub-d (subs/sub [:d]) + listener-count (atom 0) + listener-calls (atom '()) + remove-listener-fns (atom '()) + add-listener! (fn [sub] + (let [key {:index (swap! listener-count inc)}] + (subs/-add-listener sub key #(swap! listener-calls conj key)) + (swap! remove-listener-fns conj #(subs/-remove-listener sub key)))) + remove-listeners! (fn [] + (doseq [f @remove-listener-fns] + (f)))] + (add-listener! sub-a) + (add-listener! sub-b) + (add-listener! sub-c) + (add-listener! sub-d) + (reset! source 1) + (remove-listeners!) + (async done + (js/setTimeout (fn [] + (is (= @listener-calls + '({:index 1} + {:index 2} + {:index 3} + {:index 4}))) + (done)) + 10))))) From ab2a826c4255e4aadeb02a6a45e4f349a633be24 Mon Sep 17 00:00:00 2001 From: Christian Paul Dehli Date: Tue, 18 Jun 2024 12:47:18 -0400 Subject: [PATCH 6/7] fix comparator bug --- core/src/refx/subs.cljc | 2 +- core/test/refx/subs_test.cljs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/refx/subs.cljc b/core/src/refx/subs.cljc index 0d0fccc..0f6264f 100644 --- a/core/src/refx/subs.cljc +++ b/core/src/refx/subs.cljc @@ -78,7 +78,7 @@ (defonce ^:private listeners-state (letfn [(comparator [a b] - (compare (:index b) (:index a)))] + (compare (:index a) (:index b)))] (atom {:counter 0 :pending (sorted-map-by comparator)}))) (defn- invoke-listener diff --git a/core/test/refx/subs_test.cljs b/core/test/refx/subs_test.cljs index c0c6d7b..83c2371 100644 --- a/core/test/refx/subs_test.cljs +++ b/core/test/refx/subs_test.cljs @@ -116,7 +116,7 @@ sub-c (subs/sub [:c]) sub-d (subs/sub [:d]) listener-count (atom 0) - listener-calls (atom '()) + listener-calls (atom []) remove-listener-fns (atom '()) add-listener! (fn [sub] (let [key {:index (swap! listener-count inc)}] @@ -134,9 +134,9 @@ (async done (js/setTimeout (fn [] (is (= @listener-calls - '({:index 1} - {:index 2} - {:index 3} - {:index 4}))) + [{:index 1} + {:index 2} + {:index 3} + {:index 4}])) (done)) 10))))) From 90e02dd48c71206832d8542524fb32c7f0714666 Mon Sep 17 00:00:00 2001 From: Christian Paul Dehli Date: Thu, 11 Jul 2024 15:58:48 -0400 Subject: [PATCH 7/7] fix unmount bug --- core/src/refx/hooks.cljs | 2 +- core/src/refx/subs.cljc | 30 +++++++++++++++++++----------- core/test/refx/subs_test.cljs | 12 ++++++------ 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/core/src/refx/hooks.cljs b/core/src/refx/hooks.cljs index 5db5a23..cf0eab9 100644 --- a/core/src/refx/hooks.cljs +++ b/core/src/refx/hooks.cljs @@ -28,7 +28,7 @@ (react/useMemo (fn [] [(fn [callback] - (let [key {:index (swap! use-sub-counter inc)}] + (let [key {::subs/index (swap! use-sub-counter inc)}] (subs/-add-listener sub key callback) #(subs/-remove-listener sub key))) (fn [] diff --git a/core/src/refx/subs.cljc b/core/src/refx/subs.cljc index 0f6264f..9d83a01 100644 --- a/core/src/refx/subs.cljc +++ b/core/src/refx/subs.cljc @@ -78,7 +78,7 @@ (defonce ^:private listeners-state (letfn [(comparator [a b] - (compare (:index a) (:index b)))] + (compare (::index a) (::index b)))] (atom {:counter 0 :pending (sorted-map-by comparator)}))) (defn- invoke-listener @@ -103,16 +103,20 @@ (interop/next-tick (fn [] - (let [listener-fns (atom nil)] - (swap! listeners-state (fn [state] - (let [{:keys [counter pending] :as new-state} - (update state :counter dec)] - (if (zero? counter) - (do (reset! listener-fns pending) - (update new-state :pending empty)) - new-state)))) - (doseq [[_ f] @listener-fns] - (f))))))) + (let [{:keys [counter pending]} + (swap! listeners-state update :counter dec)] + + (when (zero? counter) + (doseq [[listener-key _] pending] + ;; Triggering a listener-fn can result in a subsequent sub's + ;; remove-listener to be called (which will remove it from pending). + ;; This check ensure it's still pending. + (let [listener-fn (atom nil)] + (swap! listeners-state (fn [state] + (reset! listener-fn (get-in state [:pending listener-key])) + (update state :pending dissoc listener-key))) + (when-let [f @listener-fn] + (f)))))))))) (deftype Listeners [^:mutable listeners] Object @@ -160,6 +164,8 @@ (-add-listener [_ k f] (.add listeners k f)) (-remove-listener [this k] + (when-not (signal? k) + (swap! listeners-state update :pending dissoc k)) (.remove listeners k) (when (.empty? listeners) (sub-orphaned this))) @@ -238,6 +244,8 @@ (-add-listener [_ k f] (.add listeners k f)) (-remove-listener [this k] + (when-not (signal? k) + (swap! listeners-state update :pending dissoc k)) (.remove listeners k) (when (.empty? listeners) (sub-orphaned this))) diff --git a/core/test/refx/subs_test.cljs b/core/test/refx/subs_test.cljs index 83c2371..ca6cc20 100644 --- a/core/test/refx/subs_test.cljs +++ b/core/test/refx/subs_test.cljs @@ -119,7 +119,7 @@ listener-calls (atom []) remove-listener-fns (atom '()) add-listener! (fn [sub] - (let [key {:index (swap! listener-count inc)}] + (let [key {::subs/index (swap! listener-count inc)}] (subs/-add-listener sub key #(swap! listener-calls conj key)) (swap! remove-listener-fns conj #(subs/-remove-listener sub key)))) remove-listeners! (fn [] @@ -130,13 +130,13 @@ (add-listener! sub-c) (add-listener! sub-d) (reset! source 1) - (remove-listeners!) (async done (js/setTimeout (fn [] + (remove-listeners!) (is (= @listener-calls - [{:index 1} - {:index 2} - {:index 3} - {:index 4}])) + [{::subs/index 1} + {::subs/index 2} + {::subs/index 3} + {::subs/index 4}])) (done)) 10)))))