omcljs / om

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

merge! behavior when add-root! called with nil target #825

Open calvis opened 7 years ago

calvis commented 7 years ago

I'm seeing different results for merge! after an add-root! when the target is a dom element vs. nil. This is not intuitive & is making testing hard, so I'm hoping it's a bug.

(ns frustration.core
  (:require [goog.dom :as gdom]
            [om.next :as om :refer-macros [defui]]
            [om.dom :as dom :include-macros true]))

(enable-console-print!)

(defui ThingWithIdent
  static om/Ident
  (ident [this {:keys [id]}]
    [:id id])
  static om/IQuery
  (query [this]
    '[:id])
  Object
  (render [this] (dom/div {} (str (:id (om/props this))))))

(def thing (om/factory ThingWithIdent {:keyfn id}))

(defui RootView
  static om/IQuery
  (query [this]
    [{:list (om/get-query ThingWithIdent)}])
  Object
  (render [this]
    (let [{:keys [list]} (om/props this)]
      (apply dom/div {} (map thing list)))))

(defmulti read om/dispatch)

(defmethod read :list
  [{:keys [state query] :as env} key params]
  (let [st @state]
    {:value (om/db->tree query (get st key) st)}))

(comment
  (def reconciler1
    (om/reconciler {:state  {:list []}
                    :parser (om/parser {:read read})}))

  (om/add-root! reconciler1
                RootView
                (gdom/getElement "app"))

  (def reconciler2
    (om/reconciler {:state  {:list []}
                    :parser (om/parser {:read read})}))

  (om/add-root! reconciler2
                RootView
                nil)

  (om/merge! reconciler1
             {:list [{:id "an-id"}]})
  ;;=> {:list [[:id "an-id"]], :id {"an-id" {:id "an-id"}}, :om.next/tables #{:id}}
  (om/merge! reconciler2
             {:list [{:id "an-id"}]})
  ;;=> {:list [{:id "an-id"}], :om.next/tables #{}}
  )
swannodette commented 7 years ago

@calvis is targeting nil intentional here?

anmonteiro commented 7 years ago

FWIW, what causes the issue is that a nil target causes :root in the reconciler to be a React element rather than a component instance, which fails the component? check in get-query

calvis commented 7 years ago

@swannodette I thought nil was an acceptable thing to target (e.g. I want to test the behavior of the reconciler after an add-root! to see what my state is after all the remote calls). If I need to get a dom element for testing that's fine too I guess.

anmonteiro commented 7 years ago

@calvis I sometimes use React's TestUtils, which provides a shallow renderer you can render your app to.