Why are multi-methods not working as functions for Reagent/Re-frame?

Question

In a small app I'm building that uses Reagent and Re-frame I'm using multi-methods to dispatch which page should be shown based on a value in the app state:

(defmulti pages :name)

(defn main-panel []
  (let [current-route (re-frame/subscribe [:current-route])]
    (fn []
      ;...
      (pages @current-route))))

and then I have methods such as:

(defmethod layout/pages :register [_] [register-page])

where the register-page function would generate the actual view:

(defn register-page []
  (let [registration-form (re-frame/subscribe [:registration-form])]
    (fn []
      [:div
       [:h1 "Register"]
       ;...
       ])))

I tried changing my app so that the methods generated the pages directly, as in:

(defmethod layout/pages :register [_]
  (let [registration-form (re-frame/subscribe [:registration-form])]
    (fn []
      [:div
       [:h1 "Register"]
       ;...
       ])))

and that caused no page to ever be rendered. In my main panel I changed the call to pages to square brackets so that Reagent would have visibility into it:

(defn main-panel []
  (let [current-route (re-frame/subscribe [:current-route])]
    (fn []
      ;...
      [pages @current-route])))

and that caused the first visited page to work, but after that, clicking on links (which causes current-route to change) has no effect.

All the namespaces defining the individual methods are required in the file that is loaded first, that contains the init function, and the fact that I can pick any single page and have it displayed proves the code is loading (then, switching to another page doesn't work):

https://github.com/carouselapps/ninjatools/blob/master/src/cljs/ninjatools/core.cljs#L8-L12

In an effort to debug what's going on, I defined two routes, :about and :about2, one as a function and one as a method:

(defn about-page []
  (fn []
    [:div "This is the About Page."]))

(defmethod layout/pages :about [_]
  [about-page])

(defmethod layout/pages :about2 [_]
  (fn []
    [:div "This is the About 2 Page."]))

and made the layout print the result of calling pages (had to use the explicit call instead of the square brackets of course). The wrapped function, the one that works, returns:

[#object[ninjatools$pages$about_page "function ninjatools$pages$about_page(){
return (function (){
return new cljs.core.PersistentVector(null, 2, 5, cljs.core.PersistentVector.EMPTY_NODE, [new cljs.core.Keyword(null,"div","div",1057191632),"This is the About Page."], null);
});
}"]]

while the method returns:

#object[Function "function (){
return new cljs.core.PersistentVector(null, 2, 5, cljs.core.PersistentVector.EMPTY_NODE, [new cljs.core.Keyword(null,"div","div",1057191632),"This is the About 2 Page."], null);
}"]

If I change the method to be:

(defmethod layout/pages :about2 [_]
  [(fn []
     [:div "This is the About 2 Page."])])

that is, returning the function in a vector, then, it starts to work. And if I make the reverse change to the wrapped function, it starts to fail in the same manner as the method:

(defn about-page []
  (fn []
    [:div "This is the About Page."]))

(defmethod layout/pages :about [_]
  about-page)

Makes a bit of sense as Reagent's syntax is [function] but it was supposed to call the function automatically.

I also started outputting @current-route to the browser, as in:

[:main.container
 [alerts/view]
 [pages @current-route]
 [:div (pr-str @current-route)]]

and I verified @current-route is being modified correctly and the output updated, just not [pages @current-route].

The full source code for my app can be found here: https://github.com/carouselapps/ninjatools/tree/multi-methods

Update: corrected the arity of the methods following Michał Marczyk's answer.

Cafeteria

Are you requiring the namespaces where you use defmethod in some root namespace? Because if you don't require those namespaces explicitly, your methods simply don't get added to the multimethod. – Inundate

@Inundate yes. I'll edit the question to note that. – Cafeteria

To debug this I would 1. Add a default method and 2. print out what the multimethod is returning (probably nil). – Buzzard

@Buzzard I do have a default method: github.com/carouselapps/ninjatools/blob/multi-methods/src/cljs/… and if that was being rendered I would get a blank main content. Instead, the page just doesn't refresh. – Cafeteria

@Buzzard when the method is in square brackets, I'm not calling it myself, so, printing its output is hard. – Cafeteria

Answer 1

I don't have all the details, but apparently, when I was rendering pages like this:

[:main.container
 [alerts/view]
 [pages @current-route]]

Reagent was failing to notice that pages depended on the value of @current-route. The Chrome React plugin helped me figure it out. I tried using a ratom instead of a subscription and that seemed to work fine. Thankfully, telling Reagent/React the key to an element is easy enough:

[:main.container
 [alerts/view]
 ^{:key @current-route} [pages @current-route]]

That works just fine.

Cafeteria

reagent/React will destroy and recreate the pages component each time the key changes. So this method will work, but just be aware of how. – Upandcoming

Just to be clear, this statement: Reagent was failing to notice that pages depended on the value of @current-route is incorrect. Have a look at my answer for an explanation of what's happening. – Upandcoming

Answer 2

So, a component like this: [pages @some-ratom]
will rerender when pages changes or @some-ratom changes.

From reagent's point of view, pages hasn't changed since last time, it is still the same multi-method it was before. But @some-ratom might change, so that could trigger a rerender.

But when this rerender happens it will be done using a cached version of pages. After all, it does not appear to reagent that pages has changed. It is still the same multimethod it was before.

The cached version of pages will, of course, be the first version of pages which was rendered - the first version of the mutlimethod and not the new version we expect to see used.

Reagent does this caching because it must handle Form-2 functions. It has to keep the returned render function.

Bottom line: because of the caching, multimethods won't work very well, unless you find a way to completely blow up the component and start again, which is what the currently-top-voted approach does:
^{:key @current-route} [pages @current-route]
Of course, blowing up the component and starting again might have its own unwelcome implications (depending on what local state is held in that component).

Vaguely Related Background:
https://github.com/Day8/re-frame/wiki/Creating-Reagent-Components#appendix-a---lifting-the-lid-slightly
https://github.com/Day8/re-frame/wiki/When-do-components-update%3F

Upandcoming

what's an elegant alternative to multi-methods that doesn't have unwelcome implications? – Chivy

Answer 3

The first problem that jumps out at me is that your methods take no arguments:

(defmethod layout/pages :register [] [register-page])
                                  ^ arglist

Here you have an empty arglist, but presumably you'll be calling this multimethod with one or two arguments (since its dispatch function is a keyword and keywords can be called with one or two arguments).

If you want to call this multimethod with a single argument and just ignore it inside the body of the :register method, change the above to

(defmethod layout/pages :register [_] [register-page])
                                   ^ argument to be ignored

---

Also, I expect you'll probably want to call pages yourself like you previously did (that is, revert the change to square brackets that you mentioned in the question).

---

This may or may not fix the app – there may be other problems – but it should get you started. (The multimethod will definitely not work with those empty arglists if you pass in any arguments.)

Eby

Good point about the argument list. I think ClojureScript just doesn't check the arity of functions and methods. I made this change but the app still behaves the same way: github.com/carouselapps/ninjatools/commit/… – Cafeteria

Answer 4

How about if you instead have a wrapper pages-component function which is a regular function that can be cached by reagent. It would look like this:

(defn pages-component [state]
  (layout/pages @state))

Diffraction