omcljs / om

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

Different idents for the same entity cause duplicate state #827

Open den1k opened 7 years ago

den1k commented 7 years ago

Also discovered another use-case I find dangerous. It's possible to end up with duplicate state by normalizing with two different components, see here:

(let [st          {:app/users        [{:id        1
                                           :user/name "Eva"
                                           :user/type :type/foo}
                                          {:id        2
                                           :user/name "Fred"
                                           :user/type :type/foo}
                                          {:id        3
                                           :user/name "Elli"
                                           :user/type :type/bar}]
                       :app/current-user {:id 1}}
          User        (ui
                        static om/Ident
                        (ident [_ {:keys [id]}]
                               [:user/by-id id])
                        static om/IQuery
                        (query [_]
                               [:id
                                :user/name
                                :user/type]))
          UnionFooBar (ui
                        static om/Ident
                        (ident [_ {:keys [user/type id]}]
                               [type id])
                        static om/IQuery
                        (query [_]
                               {:type/foo (om/get-query User)
                                :type/bar (om/get-query User)
                                }))]

      (pprint
        (om/tree->db [{:app/users (om/get-query UnionFooBar)}
                      {:app/current-user (om/get-query User)}] st true))

;; duplicate state=>
      {:app/users        [[:type/foo 1] [:type/foo 2] [:type/bar 3]],
       :app/current-user [:user/by-id 1],
       :type/foo
                         {1 {:id 1, :user/name "Eva", :user/type :type/foo},
                          2 {:id 2, :user/name "Fred", :user/type :type/foo}},
       :type/bar         {3 {:id 3, :user/name "Elli", :user/type :type/bar}},
       :user/by-id       {1 {:id 1, :user/name "Eva", :user/type :type/foo}},
       :om.next/tables   #{:type/foo :type/bar :user/by-id}})

The problem is a) that Om does not know that the same state is normalized twice and b) that even if it knew that both idents normalize the same entity, it would still not know which one to prefer, i.e. which should point at the other. In this case type/foo looks like the more transient one, that can easily be replace by passing different props. If the state ends up there it will be overwritten.

swannodette commented 7 years ago

When opening issues it's preferred that we just focus on describing the perceived problem. We should defer from proposing any solutions as it's just more stuff to wade through on the way to actually understanding the problem. So far I do not understand the problem at all. So let's work on that first.

My guess is that you have two components with different queries on the same logical entity and these are not getting merged correctly since there's no notion of precedence?

den1k commented 7 years ago

@swannodette I agree these two should be separate initially. It would be beneficial, however, to have a place to share and discuss ideas, even if they're throwaways. (Wiped my proposed ideas)

Yes, your guess is spot on.

anmonteiro commented 7 years ago

I fail to see what the problem here is. The concept of identity in Om Next is specified by the Ident protocol. If your Ident implementations are different, Om Next will never know that they refer to the same logical entity.