How-to contains misleading memo use

[SevereOverfl0w]

(defnc WrappedComponent [{:keys [someValue]}]
  ;; This options map is parsed at compile time to expand to the above
  ;; It's the same way you can pass in `:pre` and `:post` checks for
  ;; `defn` (and likewise, `defnc`).
  {:wrap [(some-lib/withSomeValue)
          (react/memo =)]}
  ;; Return hiccup
  [:div "I was rendered with " someValue])

the memo comparator should not be = but something more like (fn [x y] (= (js->clj x) (js->clj y))). The result of clj->js is what's passed to the comparator, and cljs never compares them as truthy.

[lilactown]

I think that hx should provide a props= helper that would allow people to use it like so:

(defnc WrappedComponent [{:keys [someValue]}]
  {:wrap [(some-lib/withSomeValue)
          (react/memo (props= :someValue))]}
  ;; Return hiccup
  [:div "I was rendered with " someValue])

and it would avoid doing clj->js, which would be very slow to do on every render.

[lilactown]

The docs should also be fixed 😬 thanks for the report

[SevereOverfl0w]

How does hx serialize the props into a Clojure data structure? Does it use clj->js?

[lilactown]

No, it uses custom props->clj and clj->props functions so that we only convert props shallowly, as well as renaming certain props: https://github.com/Lokeh/hx/blob/ff54ea6aa34c4c266025c4e2473bb0d1820d1f84/src/hx/utils.cljs

The eventual goal is to move to cljs-bean for this, and delete most of that code.

To compare props, we don't want to serialize the whole props object back to a map, so it would be better to take a collection of keys as args, turn those into strings on def and then use goog.object/get within the props= function to do the comparison.

Since clj->props only converts the map to an object shallowly, on any individual prop you'll get full Clojure equality semantics.

We could also perhaps enumerate all keys if e.g. nothing is passed into props=, but I hesitate to make that a default since 99% of the time you don't want that.

[SevereOverfl0w]

That's exactly how I was using it, because 99% of my components take immutable data structures.

[lilactown]

@SevereOverfl0w in that case, using plain (react/memo) without passing in a custom equality check would suffice. It does a shallow equality based on identity, which if you're leveraging immutable data structures to their fullest extent should be fast and correct.

The problem I'm trying to dance around is that = does a deep comparison if object identity isn't equal, which should be done with care; with deep structures, it gets quite expensive and can significantly impact performance when you are in fact trying to optimize a hot path under circumstances that might come as surprising.

There are times when this might be necessary or preferred, but I don't really understand those cases yet and I don't want to setup the defaults to be a performance footgun. Reagent at first defaulted to = in determining re-renders / reaction updates, but moved to identical? due to performance reasons later.

I would rather encourage people to leverage immutable data better, where an object identity change is a good way to detect that the data contained within it has changed.

FWIW a function that enumerates each key and does a deep comparison is ~5 lines of code:

(defn all-props= [p p']
  (let [ks (goog.object/getKeys p)
        ks' (goog.object/getKeys p')]
    (and (= (count ks) (count ks'))
         (every #(= (goog.object/get p %) (goog.object/get p' %)) ks))))

But again, this shouldn't be necessary if you're using immutable data.

Memoizing is already full of footguns and gotchas (functions passed in as event handlers, literal data created in render, etc.) that I want to make sure that I don't make it harder to reason about by defaulting to something that can have radically different performance depending on subtle things in the users application.

[mhuebert]

Reagent at first defaulted to = in determining re-renders / reaction updates, but moved to identical? due to performance reasons later.

Minor nit, but Reagent still uses = primarily:

Reaction: https://github.com/reagent-project/reagent/blob/master/src/reagent/ratom.cljs#L439

ShouldComponentUpdate: https://github.com/reagent-project/reagent/blob/master/src/reagent/impl/component.cljs#L199

In the case of Reactions, there is an identical? check in the function that handles external changes (_handle-change, (https://github.com/reagent-project/reagent/blob/master/src/reagent/ratom.cljs#L400)) but this happens after a = comparison has triggered those watches to be fired.

The discussion that led to this: reagent-project/reagent#143

Personally I like the idea of = as default with an escape hatch. Reagent components have this (one can supply shouldComponentUpdate) but Reactions do not.