Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid running after/enrich without a db effect #511

Merged
merged 1 commit into from
Jun 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## Unchanged

#### Fixed

- `after` and `enrich` interceptors now no longer run if there is no `db` effect, rather than running against the `db` coffect. [#453](https://github.com/Day8/re-frame/issues/453)


## 0.10.6 (2018-09-03)
Expand Down
27 changes: 12 additions & 15 deletions src/re_frame/std_interceptors.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@
return a modified `db`.

Unlike the `after` interceptor which is only about side effects, `enrich`
expects `f` to process and alter the given `db` coeffect in some useful way,
expects `f` to process and alter the given `db` effect in some useful way,
contributing to the derived data, flowing vibe.

Example Use:
Expand Down Expand Up @@ -283,21 +283,19 @@
:id :enrich
:after (fn enrich-after
[context]
(let [event (get-coeffect context :event)
db (if (contains? (:effects context) :db)
(get-effect context :db) ;; If no db effect is returned, we provide the original coeffect.
(get-coeffect context :db))]
(->> (f db event)
(assoc-effect context :db))))))
(let [event (get-coeffect context :event)
db-fx? (contains? (:effects context) :db)
db (get-effect context :db)]
(cond-> context
db-fx? (assoc-effect :db (f db event)))))))



(defn after
"returns an interceptor which runs a given function `f` in the `:after`
position, presumably for side effects.

`f` is called with two arguments: the `:effects` value for `:db`
(or the `coeffect` value of db if no db effect is returned) and the event.
`f` is called with two arguments: the `:effects` value for `:db` and the event.
Its return value is ignored, so `f` can only side-effect.

Examples use can be seen in the /examples/todomvc:
Expand All @@ -308,12 +306,11 @@
:id :after
:after (fn after-after
[context]
(let [db (if (contains? (:effects context) :db)
(get-in context [:effects :db])
(get-in context [:coeffects :db]))
event (get-in context [:coeffects :event])]
(f db event) ;; call f for side effects
context)))) ;; context is unchanged
(let [event (get-coeffect context :event)
db-fx? (contains? (:effects context) :db)
db (get-effect context :db)]
(when db-fx? (f db event))) ;; call f for side effects
context))) ;; context is unchanged


(defn on-changes
Expand Down
72 changes: 39 additions & 33 deletions test/re_frame/interceptor_test.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -136,42 +136,48 @@
((:after change-i))
(get-effect :db ::not-found))))))

(defn- intercept-with-db-effect
"Used to test if interceptors run given a db effect."
[interceptor-f db-fx add?]
(cond-> (context [:a :b] [(interceptor-f)] {:a 1})
add? (assoc-effect :db db-fx)
true (interceptor/invoke-interceptors :before)
true interceptor/change-direction
true (interceptor/invoke-interceptors :after)))

(deftest test-after
(testing "when no db effect is returned"
(let [after-db-val (atom nil)]
(-> (context [:a :b]
[(after (fn [db] (reset! after-db-val db)))]
{:a 1})
(interceptor/invoke-interceptors :before)
interceptor/change-direction
(interceptor/invoke-interceptors :after))
(is (= @after-db-val {:a 1}))))
(testing "when a false db effect is returned"
(let [after-db-val (atom :not-reset)]
(-> (context [:a :b]
[(after (fn [db] (reset! after-db-val db)))]
{:a 2})
(assoc-effect :db nil)
(interceptor/invoke-interceptors :before)
interceptor/change-direction
(interceptor/invoke-interceptors :after))
(is (= @after-db-val nil))))
(testing "when a nil db effect is returned"
(let [after-db-val (atom :not-reset)]
(-> (context [:a :b]
[(after (fn [db] (reset! after-db-val db)))]
{:a 3})
(assoc-effect :db false)
(interceptor/invoke-interceptors :before)
interceptor/change-direction
(interceptor/invoke-interceptors :after))
(is (= @after-db-val false)))))
(let [after-db-val (atom nil)
run-after (partial intercept-with-db-effect
#(after (fn [db] (reset! after-db-val db))))]
(testing "when no db effect is returned"
(reset! after-db-val :not-reset)
(run-after nil false)
(is (= @after-db-val :not-reset)))
(testing "when a nil db effect is returned"
(reset! after-db-val :not-reset)
(run-after nil true)
(is (= @after-db-val nil)))
(testing "when a false db effect is returned"
(reset! after-db-val :not-reset)
(run-after false true)
(is (= @after-db-val false)))
(testing "when a truthy db effect is returned"
(reset! after-db-val :not-reset)
(run-after :reset true)
(is (= @after-db-val :reset)))))

(deftest test-enrich
(testing "when no db effect is returned"
(let [ctx (context [] [] {:a 1})]
(is (= ::not-found (get-effect ctx :db ::not-found)))
(-> ctx (:after (enrich (fn [db] (is (= db {:a 1})))))))))
(let [run-enrich (partial intercept-with-db-effect
#(enrich (fn [db] (assoc db :enriched :added))))]
(testing "when no db effect is returned"
(let [ctx (run-enrich nil false)]
(is (= (-> ctx (get-effect :db) :enriched) nil))))
(testing "when nil db effect is returned"
(let [ctx (run-enrich nil true)]
(is (= (-> ctx (get-effect :db) :enriched) :added))))
(testing "when truthy db effect is returned"
(let [ctx (run-enrich {} true)]
(is (= (-> ctx (get-effect :db) :enriched) :added))))))

(deftest test-update-coeffect
(let [context {:effects {:db {:a 1}}
Expand Down