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.

Sign up for GitHub

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

Jump to bottom

Persist the ref across all transactions in ptransact! #98

Merged
merged 2 commits into from Dec 14, 2017
Merged

Persist the ref across all transactions in ptransact! #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6652981
Persist the ref across all transactions in ptransact!
wilkerlucio Dec 13, 2017
f4c38b4
Fix ptransact! when mutation doesn't have a remote
wilkerlucio Dec 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/demos/cards/server_return_values_as_data_driven_mutation_joins.cljc
View file
Open in desktop
Expand Up @@ -87,7 +87,8 @@
(dom/button #js {:onClick #(prim/ptransact! this `[(add-item {:list-id ~id
:id ~(prim/tempid)
:value "A New Value"})
(set-overlay {:visible? false})])} "Add item")))
(set-overlay {:visible? false})
:overlay])} "Add item")))

(def ui-list (prim/factory ItemList {:keyfn :db/id}))

Expand Down
9 changes: 5 additions & 4 deletions src/main/fulcro/client/data_fetch.cljc
View file
Open in desktop
Expand Up @@ -359,13 +359,13 @@
(defmethod mutate 'fulcro/load [env _ params] (load* env params))
(defmethod mutate `load [env _ params] (load* env params))

(defmutation run-deferred-transaction [{:keys [tx reconciler]}]
(defmutation run-deferred-transaction [{:keys [tx ref reconciler]}]
(action [env]
(let [reconciler (-> reconciler meta :reconciler)]
#?(:clj (prim/transact! reconciler tx)
:cljs (js/setTimeout (fn [] (prim/transact! reconciler tx)) 1)))))
#?(:clj (prim/transact! reconciler ref tx)
:cljs (js/setTimeout (fn [] (prim/transact! reconciler ref tx)) 1)))))

(defmutation deferred-transaction [{:keys [tx remote]}]
(defmutation deferred-transaction [{:keys [tx remote ref]}]
(action [env]
(let [{:keys [reconciler component] :as env} env
reconciler (cond
Expand All @@ -377,6 +377,7 @@
:remote remote
:marker false
:post-mutation-params {:tx tx
:ref ref
:reconciler (with-meta {} {:reconciler reconciler})}})
(log/error (str "Cannot defer transaction. Reconciler was not available. Tx = " tx)))))
(remote [env] (remote-load env)))
Expand Down
12 changes: 7 additions & 5 deletions src/main/fulcro/client/primitives.cljc
View file
Open in desktop
Expand Up @@ -2816,20 +2816,21 @@
"Converts a sequence of calls as if each call should run in sequence (deferring even the optimistic side until
the prior calls have completed in a full-stack manner), and returns a tx that can be submitted via the normal
`transact!`."
[tx]
[ref tx]
(let [ast-nodes (:children (query->ast tx))
{calls true reads false} (group-by #(= :call (:type %)) ast-nodes)
first-call (first calls)
dispatch-key (:dispatch-key first-call)
get-remote (or (some-> (resolve 'fulcro.client.data-fetch/get-remote) deref) (fn [sym]
(log/error "FAILED TO FIND MUTATE. CANNOT DERIVE REMOTES FOR ptransact!")
:remote))
remote (get-remote dispatch-key)
remote (or (get-remote dispatch-key) :remote)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fixes the issue when the mutation doesn't have a remote

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

um...why would we want to add a remote if there is not a remote?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because otherwise, the whole process stops, the chaining depends on a remote call to happen so the post-mutation is triggered to call the next mutation. Without this change, let's say you have 2 mutations on the list, the first one doesn't have a remote, what happens is that second never gets called, and an error is fired because of an attempt to call something without a remote (don't call exactly what now). This remote is not the real one from the mutation, this is for the one triggered by load-action that triggers the next mutation, makes sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thanks for the reminder...I'll look at it later tonight or tomorrow.

tx-to-run-now (into [(ast->query first-call)] (ast->query {:type :root :children reads}))]
(if (seq (rest calls))
(let [remaining-tx (ast->query {:type :root :children (into (vec (rest calls)) reads)})]
(into tx-to-run-now `[(fulcro.client.data-fetch/deferred-transaction {:remote ~remote
:tx ~(pessimistic-transaction->transaction remaining-tx)})]))
:ref ~ref
:tx ~(pessimistic-transaction->transaction ref remaining-tx)})]))
tx-to-run-now)))

(defn ptransact!
Expand All @@ -2841,8 +2842,9 @@

NOTE: `ptransact!` *is* safe to use from within mutations (e.g. for retry behavior)."
[comp-or-reconciler tx]
#?(:clj (transact! comp-or-reconciler (pessimistic-transaction->transaction tx))
:cljs (js/setTimeout (fn [] (transact! comp-or-reconciler (pessimistic-transaction->transaction tx))) 0)))
(let [ref (if (component? comp-or-reconciler) (get-ident comp-or-reconciler))]
#?(:clj (transact! comp-or-reconciler (pessimistic-transaction->transaction ref tx))
:cljs (js/setTimeout (fn [] (transact! comp-or-reconciler (pessimistic-transaction->transaction ref tx))) 0))))

#?(:clj
(defn- is-link? [query-element] (and (vector? query-element)
Expand Down
27 changes: 19 additions & 8 deletions src/test/fulcro/client/primitives_spec.cljc
View file
Open in desktop
Expand Up @@ -879,16 +879,27 @@
(specification "pessimistic-transaction->transaction"
(assertions
"Returns the transaction if it only contains a single call"
(prim/pessimistic-transaction->transaction `[(f {:x 1})]) => `[(f {:x 1})]
(prim/pessimistic-transaction->transaction nil `[(f {:x 1})]) => `[(f {:x 1})]
"Includes the follow-on reads in a single-call"
(prim/pessimistic-transaction->transaction `[(f {:y 2}) :read]) => `[(f {:y 2}) :read]
(prim/pessimistic-transaction->transaction nil `[(f {:y 2}) :read]) => `[(f {:y 2}) :read]
"Converts a sequence of calls to the proper nested structure, deferring against the correct remotes"
(prim/pessimistic-transaction->transaction `[(f) (g) (h)]) => `[(f)
(df/deferred-transaction {:remote :remote
:tx [(g) (df/deferred-transaction
{:remote :rest-remote
:tx [(h)]})]})]))

(prim/pessimistic-transaction->transaction nil `[(f) (g) (h)])
=> `[(f)
(df/deferred-transaction {:remote :remote
:ref nil
:tx [(g) (df/deferred-transaction
{:remote :rest-remote
:ref nil
:tx [(h)]})]})]
"Spread the ref across the calls"
(prim/pessimistic-transaction->transaction [:id 123] `[(f) (g) (h)])
=> `[(f)
(df/deferred-transaction {:remote :remote
:ref [:id 123]
:tx [(g) (df/deferred-transaction
{:remote :rest-remote
:ref [:id 123]
:tx [(h)]})]})]))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; query spec, with generators for property-based tests
Expand Down