Bug in aggregate subscriptions based on rum derived atoms ?

[DjebbZ]

Hi,

I'm following the README to the letter and I think I found a bug. See the following minimal reproduction case in the REPL :

(require '[scrum.core :as scrum])
=> nil
(require '[rum.core :as rum])
=> nil
(let [rec (scrum/reconciler {:state (atom {})
                             :resolvers {[:a] (constantly :a)}})
      sub (fn [reconciler]
            (scrum/subscription reconciler [:a]))
      der (fn [reconciler]
            (rum/derived-atom [(sub reconciler)] ::key (constantly :derived)))]
  @(der rec))
=> ClassCastException scrum.resolver.Resolver cannot be cast to clojure.lang.IRef  clojure.core/add-watch (core.clj:2134)

Whereas plain derived-atoms in rum work fine :

(let [a (atom :a)
      b (atom :b)
      der (rum/derived-atom [a b] ::key
                            (fn [a b] 
                              (println "a" a "b" b)))]
  (reset! a :aaa)
  (reset! b :bbb))
a :a b :b
a :aaa b :b
a :aaa b :bbb
=> :bbb

The problem : rum calls add-watch on each ref, and in clojure.core add-watch has a type hint on the ref of clojure.lang.IRef.

(I think) I was able to track down the root cause : scrum's Resolver only implements clojure.lang.IDeref protocol, but clojure.lang.IRef extends IDeref and implements more methods. I think the Resolver should implement IRef as well to work just fine. Not sure, I'm not very familiar with Clojure's protocols.

In the meantime, not sure what I should do to work around the problem.

[DjebbZ]

As a temporary workaround I'll do the following. What do you think ?

(let [rec   (scrum/reconciler {:state     (atom {})
                               :resolvers {[:a] (constantly :a)
                                           [:b] (constantly :b)}})
      sub-a (fn [reconciler]
              (scrum/subscription reconciler [:a]))
      sub-b (fn [reconciler]
              (scrum/subscription reconciler [:b]))
      derived (fn [reconciler]
                (let [a @(sub-a reconciler)
                      b @(sub-b reconciler)]
                  [a b]))]
  (derived rec))

The example is dumb, but I hope you get the point.

[DjebbZ]

FYI, not working, since I'm using it as a subscription (with (rum/react XXX))... I think I'm blocked...

[roman01la]

Hey. You are right. I'm going to update the code later today. Which version of Scrum is it?

[DjebbZ]

2.1.0-SNAPSHOT. Sorry I should have mentionned it.

Going to upgrade to 2.2.0 to ease the transition to the bugfix version.

[roman01la]

No worries, I'll update both.

[roman01la]

Actually I think that's inconsistency in Rum. When used in Clojure derived-atom shouldn't add watchers.

[DjebbZ]

Indeed, I think you're right : you render in one pass server-side, watches are useless. At least so it seems. Are you opening an issue in Rum ?

[roman01la]

Yeah. Going to talk to Nikita about this.

[roman01la]

Fixed in 86d9adf for 2.1.0 and 2.2.9 in master. Please check if it works for you.

[DjebbZ]

Looks like it works fine in 2.1.0. Thanks a lot !