понедельник, 12 июня 2017 г.

The Simplest Way to Mess Up om/build

I played with ClojureScript Om recently and, while working on a stupidly simple task, stumbled into a strange problem when my components losed state and got re-rendered whenver a user attempted any interactions that caused the underlying data to change.

I had a component that was bound to a path in app state holding a vector of maps. It consisted of several identical components, whose purpose was to edit values in these maps. Each of these editors updated the corresponding value during the onChange event - whenever a user would edit the text in them. An important detail is also that these editors had state - they could either be in "view" mode (rendered as <span>) or in the "edit" mode (rendered as <input>). The thing transitions to the "edit" mode when it is clicked and jumps back after user hits Enter in the input or moves focus somewhere else. It all worked pretty fine until I actually started editing stuff - whenever I pressed a key all the components would get re-rendered and, more importantly, lose the editing state.

I attempted some debugging and code tweaking, but nothing helped or even shed the light on what was going on - I saw that the controls were re-rendering, saw that their state was lost immediately after edits and later even noticed that they are being mounted as new ones after edits. This later observation explained the problem with lost state, but didn't give me much clue onto why that was happenning. However the thing got clear after I read the documentation for om's build and build-all functions a couple times:

build
(defn build
  ([f x] ...)
  ([f x m] ...))
Constructs an Om component. f must be a function that returns an instance of om.core/IRender or om.core/IRenderState. f must take two arguments - a value and the backing Om component usually referred to as the owner. f can take a third argument if :opts is specified in m. The component is identified by the function f. Changing f to a different function will construct a new component, while changing the return value will not change component. x can be any value. m is an optional map of options.

At some point I noticed my dumb mistake - I was calling build-all like this:
(defn component [data owner]
   (reify
     om/IRender
     (render [_]
       (html
        [:div.container
         (om/build-all (fn [data owner opts]
                        (if some-condition
                          (edit-component1 data owner opts)
                          (edit-component2 data owner opts)))
                       (:attributes data)
                       {:key :attribute-id
                        :opts {}})]))))

Which means that on each render of the parent component I would pass a new function to build-all. Because components "are identified by their functions", that meant that on every change of the underlying state the parent component will start re-rendering the child ones, see a new function created by the (fn []) form and believe that it is rendering new components. Thus it won't even want to connect the old state to them and they will be rendered in the default "view" state.

Simply changing my code to the following dirty thing immediately resolved the issue:
(defn edit-component [data owner opts]
 (if some-condition
   (edit-component1 data owner opts)
   (edit-component2 data owner opts)))

(defn component [data owner]
   (reify
     om/IRender
     (render [_]
       (html
        [:div.container
         (om/build-all edit-component
                       (:attributes data)
                       {:key :attribute-id
                        :opts {}})]))))

The conclusion is simple: never ever pass to build or build-all a function that you create in the render method of the parent component or in any other bit of code that is called multiple times over the lifetime of your components. If you do this you will get new components built everytime and thus lose whatever information you associate with them (plus some time for their creation). I hope that you never commit this mistake, because that's one of the dumb things that require time to spot, but if you are unfortunate to run into it, this note may help.