lilactown / helix

A simple, easy to use library for React development in ClojureScript.
Eclipse Public License 2.0
624 stars 52 forks source link

Any good way to make multimethods work as component functions? #116

Closed simonacca closed 1 year ago

simonacca commented 1 year ago

Hi, I realized that it doesn't seem to be possible to define components with multimethods, even with a custom macro such as

(defmacro defmethod
     [multifn dispatch-val & form-body]
     (let [props-bindings (first form-body)
            body (rest form-body)]
       `(cljs.core/defmethod ~multifn ~dispatch-val
          [props# maybe-ref#]
          (let [~props-bindings [(helix.core/extract-cljs-props props#) maybe-ref#]]
            ~@body))))

Upon usage of such a component with:

(defmulti animals (fn [props] (.-animal props)))

(my-custom-helix-macros/defmethod animals :dog
  []
  (helix/$ :div "rendering dog"))

(my-custom-helix-macros/defmethod animals :sheep
  []
  (helix/$ :div "rendering sheep"))

(helix/defnc using-multimethod
  []
  (helix/$ animals {:animal :dog})
)

When rendering such a component, one gets this error from React:

Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object

This seems to be because a multimethod is realized in Javascript as an object that is not callable, hence react doesn't know what to do with it.

(This is what a multimethod looks like in js: {name: {...}, default_dispatch_val: {...}, hierarchy: {...}, method_table: {...}, ...}.)

One could get around this without breaking the contract with React too much by defining (defn animals-wrapper [& args] (apply animals args)) and calling animal-wrappers from using-multimethod instead.

Is there a more elegant way to solve this problem? (not necessarily in helix core, also in userspace would be fine)

Thanks!

lilactown commented 1 year ago

React components must be either a literal JavaScript function, or an object that inherits from the React.Component class. Unfortunately, multi methods do not fall into those categories.

I would recommend structuring your code in a way that the business logic is handled by multi method, but the component itself is a function. Similar to your examples:

(defmulti animals :animal)

(defmethod animals :dog
  []
  "rendering dog")

(defmethod animals :sheep
  []
  "rendering sheep")

(helix/defnc using-multimethod
  []
  (helix/$ :div (animals {:animal :dog})))

While less powerful than using the multi method as a component itself, this protects you from a potential footgun: if your multicomponent used hooks, and the :animal passed in changed, it could render new hooks or need to re-initialize them based on the method used and React wouldn't know how to do that, because the identity of methods are the same no matter the branch it takes. It would be akin to:

(defnc my-multi-component
  [{:keys [animal]}]
  (case animal
    :dog (let [[barked? set-barked] (hooks/use-state false)]
            ,,,)
    :sheep ,,,))

This is breaking the rule where hooks should never be conditionally used inside of a component.

simonacca commented 1 year ago

Wow, thank you, I didn't consider the hook problem at all!!

I still belive it would be great to get multimethods working in this particular case as I'm using them to implement lazy-loaded plugins (so both the business logic and the components are loaded just in time when the application calls for a particular plugin).

As a last attempt, what are your thoughts on using the multimethod's dispatch-val (that is, :dog, :sheep and the like) as a React key to "force" React to realize that they are indeed different components?

Proof of concept:

(defmulti animal
  (fn [props] (.-animap props)))

(helix/defnc multimethod-wrapper [{:keys [mm] :as props}]
  (helix/$ (fn [inner-props] (mm inner-props))
          {:key ((dispatch-fn mm) (helix.impl.props/-props props))
           :& props}))

(helix/defnc using-multimethod
  []
  (helix/$ multimethod-wrapper {:mm animal :animal :dog}))

p.s. I suppose the anonymous function in multimethod-wrapper should be memoized...

lilactown commented 1 year ago

How exactly do you implement lazy-loading this way? I'm interested to learn.

simonacca commented 1 year ago

Using shadow-cljs's built-in loader. In practice:

  1. Define multimethod in one namespace
  2. Define plugins one per namespace, as methods of the above multimethod
  3. Define each plugin as a separate module in the config
  4. At runtime, make sure to load each module using shadow.loader before it is rendered
lilactown commented 1 year ago

Have you looked at React.lazy and [shadow.lazy], as described in thheller's blog post on code splitting?

With a bit of macro magic you can lazily load individual components really easily.

;; a lazy-loaded about page
(def about (lazy town.lilac.app.about/page)

(defnc routes
  []
  (case (use-route)
    :home ($ home)
    :about ($ about)))

(defnc app
  []
  (d/div
    ($ navigation)
    (helix/suspense
     {:fallback ($ spinner)}
     ($ routes))))

This way, you can render a fallback spinner if the module hasn't been loaded yet and is taking awhile. I'm not sure how you would do this with the multi method approach.

simonacca commented 1 year ago

Very interesting, didn't know about either shadow.lazy nor React.lazy, thank you!

Youst to continue the theoretical exercise, if one still wanted to use multimethods, they could for example:

In any case, it seems clear (in my view at least) that there is nothing to change in helix per se stemming from this conversation, therefore am closing the issue. Should you have a different opinion, please feel more than welcome to reopen.

Thanks again! Simon

lilactown commented 1 year ago

I appreciate the good discussion @simonacca :)

simonacca commented 1 year ago

Likewise!