omcljs / om

ClojureScript interface to Facebook's React
6.66k stars 363 forks source link

Normalization on link lookup #824

Open den1k opened 7 years ago

den1k commented 7 years ago

Why does normalization not work on link lookups?

(let [Sib    (ui
               static om/Ident
               (ident [_ {:keys [db/id]}]
                      [:sibling/by-id id])
               static om/IQuery
               (query [_]
                      [:db/id]))
      Child  (ui
               static om/Ident
               (ident [_ {:keys [db/id]}]
                      [:child/by-id id])
               static om/IQuery
               (query [_]
                      [:db/id
                       ;; this fails
                       {[:sibling '_] (om/get-query Sib)}]))
      Parent (ui
               static om/IQuery
               (query [_]
                      [{:child (om/get-query Child)}
                       ;; this works
                       #_{:sibling (om/get-query Sib)}]))]
  ;; false
  (is (= (om/tree->db Parent {:child   {:db/id 1}
                              :sibling {:db/id 2}} true)
         {:child          [:child/by-id 1],
          :sibling        [:sibling/by-id 2],
          :child/by-id    {1 {:db/id 1}}
          :sibling/by-id  {2 {:db/id 2}}
          :om.next/tables #{:child/by-id :sibling/by-id}}))

  (comment
    ;; data from failing case
    {:child          [:child/by-id 1],
     :sibling        {:db/id 2},
     :child/by-id    {1 {:db/id 1}}
     :om.next/tables #{:child/by-id}}))
anmonteiro commented 7 years ago

I think your expectations are wrong here. :sibling is supposed to be a key that lives at the root of the app state, which supposely doesn't need to be normalized because there is only ever supposed to be one.

den1k commented 7 years ago

This forces us to describe a component tree (of idents & queries only) for our state that is different from our ui component tree for the mere purpose of normalizing our app-state on init.

What's the reason for excluding linked queries from normalization?

swannodette commented 7 years ago

@den1k I think you are probably using a this feature in a way that was just not intended. The primary purpose for this feature was for client only application state (no reification in the backend). Doing anything more here is in enhancement territory.

den1k commented 7 years ago

@swannodette that's not the case. We have multiple components on the frontend that query the same top level state using links. This state need not be queried at the root, since there is no immediate child that uses it. Because queries with links as keys do not get normalized we have to add a bunch of unused queries at the root to normalize top level state. To @anmonteiro's point, we want the values to normalize, here's why:

  ;; normalized app-state
  {:app/current-doc nil
   :docs/by-id      {1 {:db/id    1
                        :doc/text "Foo"}}}

  ;; today
  ;; create new doc and assoc to app/current-doc
  ;; does not get normalized unless queried at root + root must rerender to normalize
  ;; or normalize manually using App query and om/tree->db
  {:app/current-doc {:db/id    2
                     :doc/text "Bar"}
   :docs/by-id      {1 {:db/id    1
                        :doc/text "Foo"}}}

  ;; desired
  {:app/current-doc [:docs/by-id 2]
   :docs/by-id      {1 {:db/id    1
                        :doc/text "Foo"}
                     2 {:db/id    2
                        :doc/text "Bar"}}}
swannodette commented 7 years ago

@den1k I know what you want. But I'm telling you that's not how it was ever intended to work. Let's please refrain from adding redundant information to issues. Thanks.

swannodette commented 7 years ago

Please leave this issue alone. It just needs more thought.