omcljs / om

ClojureScript interface to Facebook's React
6.65k stars 364 forks source link

parsing reads returned transform-reads can discard results #869

Open petterik opened 7 years ago

petterik commented 7 years ago

Did: Ran (om/transact! this [(do/this) :read/that]) Expected: Components with read :read/that to have their query to be sent remotely and get an answer back. Happened: Only one of the components with :read/that got all of its data.

Narrowed it down to transform-reads returning reads where more than one read had the same root join. The stock om/parser returns a map for read values and will only return one result for each key, in this case one result for each join.

We used the om/parser both client and server side, resulting in us not getting all the data we needed back from the server.

Minimal case with reads, render and reconciler code omitted:

;; Example:
;; (om/transform-reads r [:foo])
;; returns: [{:join/parent [{:join/child1 [:foo]}]} 
;;           {:join/parent [{:join/child2 [:foo]}]}]
;; Parsing the query will return :foo only for the second path:
;; {:join/parent {:join/child2 {:foo 1}}}

;; Components
(defui Child1
  static om/IQuery
  (query [this] [:foo]))

(defui Child2
  static om/IQuery
  (query [this] [:foo]))

(defui Parent
  static om/IQuery
  (query [this] 
    [{:join/child1 (om/get-query Child1)}
     {:join/child2 (om/get-query Child2)}]))

(defui Grandparent
  static om/IQuery
  (query [this] [{:join/parent (om/get-query Parent)}]))

(om/get-query Grandparent)
;; => [#:join{:parent [#:join{:child1 [:foo]} #:join{:child2 [:foo]}]}]

(om/transform-reads r [:foo])
;; => [#:join{:parent [#:join{:child1 [:foo]}]} 
       #:join{:parent [#:join{:child2 [:foo]}]}]

(parser env (om/transform-reads r [:foo]))
;; => #:join{:parent #:join{:child2 {:foo <foo-val>}}}

Minimal repro: https://gist.github.com/petterik/0858c903d470be1245314c9b640a00c5

Possible solution: Merge queries by their joins, where:

 [{:join/parent [{:join/child1 [:foo]}]} 
  {:join/parent [{:join/child2 [:foo]}]}]

Would be merged on :join/parent to:

[{:join/parent [{:join/child1 [:foo]} 
                {:join/child2 [:foo]}]}]