clj-commons / citrus Goto Github PK

My leiningen cannot find citrus 3.2.1 in any repo.

error in process sentinel: Could not start nREPL server: Could not find artifact org.roman01la:citrus:jar:3.2.1 in central (https://repo1.maven.org/maven2/)
Could not find artifact org.roman01la:citrus:jar:3.2.1 in clojars (https://repo.clojars.org/)
Could not find artifact org.roman01la:citrus:jar:3.2.1 in sonatype (https://oss.sonatype.org/content/repositories/snapshots)

I suspect it is not published?

We are considering next steps for co-effects implementation. Do we want to keep it, improve it or remove it? Please let me know if you ever used co-effects in Citrus.

Hi,

We had a little discussion last week on reddit about re-frame interceptors and I said I was going to open an issue the day after... Sorry for the delay!

Well I dug up so deep into re-frame interceptors that I ended up making a fork of re-frame that kind of supports rum, and without global state https://github.com/chpill/re-frankenstein.

The https://github.com/chpill/re-frankenstein/blob/master/src/re_frame/frank.cljs namespace was derived from your reconciler design. It shows how to use re-frame interceptors without the global mutable state.

Hello,

I want to start a discussion about using scrum in an isomorphic rum web app. We looked at the code and saw that a really small portion of the code uses javascript interop (mostly calls to js/requestAnimationFrame).

Why didn't you implement isomorphic scrum (with cljc) ? I know that scrum server-side brings almost no interest, but isomorphic does bring a lot.

What we understood from the code and Readme :

• initializing state by dispatching a scrum event with broadcast-sync! already works both server and client side
• subscription just needs a Clojure implementation that does a get-in with the given path in the state. Need to think about multithreading though... Maybe it's a problem.

I hope you engage in this discussion and that we find a way to do it. Thanks in advance !

the committ add dispatch assertions

7819582

means that controlller/control functions will blow up with a
Uncaught Error: No protocol method IMultiFn.-methods defined for type function:
if they are not a multimethod

Is this a bug or a feature?

I have a codebase that was using a simple control function to wrap a multimethod call that broke.
Was this code in violation of the citrus application structure (and therefore deserved to fail)?
Or is it an unintended oversight?

Simple question : for non-trivial application with deep tree of components, how do you pass the reconciler down ? Top down from parents to children ? Require it where necessary ?

I'm interested in your personal experience.

STate manage for Rum?

@escherize was telling me about this library, and I thought Scrum was a confusing name :)

Currently, when a controller returns state that is exactly the same as the previous state, a re-render does not occur, because the state-change callback is not executed. The logic being: why re-render when there are no changes?

However, I currently have a case where re-render should definitely occur, even if new state is the same as the previous state. I will explain the usecase later, but for now a code sample to demonstrate the problem:

(def state {:controller "42"})

(defmulti controller (fn [event] event))

(defmethod controller :set [_ _ current]
  {:state current})

(def reconciler
  (citrus/reconciler
   {:state (atom state)
    :controllers {:controller controller}}))

(def subscription (citrus/subscription reconciler [:controller]))

(rum/defc Editor < rum/reactive [r]
  (let [text (rum/react subscription)]
    [:input {:type "text" :value text 
                          :on-change #(citrus/dispatch-sync! r :controller :set)}]))

(rum/mount (Editor reconciler)
           (. js/document (getElementById "app")))

The code sample contains a simplified example of the issue I am experiencing: I want to prevent the end-user from entering invalid data in an editbox. In case of the sample the behaviour should be that the user can never change the value in the textbox from "42" to something else.

However, if you run the sample, you will see that it's definitely possible to change the value in the textbox from 42 to something else.

The sample seems pretty nonsensical, but in a more reallife example: imagine you want the user to only be allowed to enter certain characters (numerical, or legal email adres characters for instance). Then you want to be able to revert to the old value.

The solution that currently works for me is changing the following lines of code in Citrus:

;; in citrus.reconciler [41 - 47]
  IWatchable
  (-add-watch [this key callback]
    (add-watch state (list this key)
      (fn [_ _ oldv newv]
        (callback key this oldv newv)))
        ;was
        ;(when (not= oldv newv)
        ;  (callback key this oldv newv))))
    this)

;; in citrus.cursor [24 - 32]
  IWatchable
  (-add-watch [this key callback]
    (add-watch ref (list this key)
      (fn [_ _ oldv newv]
        (let [old (reducer (get-in oldv path))
              new (reducer (get-in newv path))]
          (callback key this old new))))
          ;was
          ;(when (not= old new)
          ;  (callback key this old new)))))
    this)

Happy to provide a PR if okay with the solution.

Hi!

I am trying to use namespaced keywords to bring more clarity to subscribing and dispatching and could not make it work.

Here is a modified example of a component:

(ns probe.scrum.counter
  (:require [rum.core :as rum]
            [scrum.core :as scrum]))

(rum/defc counter-ui < rum/reactive [r]
  [:div
   ; :counter here should be in sync with keyword in
   ; reconciler declaration -> =(
   [:button {:on-click #(scrum/dispatch! r ::state :dec)} "-"]
   [:span   (rum/react (scrum/subscription r [::state]))]
   [:button {:on-click #(scrum/dispatch! r ::state :inc)} "+"]])

(def initial-state 0)

(defmulti counter-controller (fn [action] action))
(defmethod counter-controller :init [] initial-state)
(defmethod counter-controller :inc [_ _ counter] (inc counter))
(defmethod counter-controller :dec [_ _ counter] (dec counter))

And here is core.clj

(ns probe.core
  (:require [rum.core :as r]
            [scrum.core :as scrum]
            [probe.scrum.counter :as counter]))

(defonce reconciler
  (scrum/reconciler {:state       (atom {})
                     :controllers {:counter/state counter/counter-controller}}))
(defonce init-ctrl
  (scrum/broadcast-sync! reconciler :init))

(r/mount (counter/counter-ui reconciler)
         (js/document.getElementById "app"))

Could you please help me understand is it a bug or am I doing something wrong here?

Is there any way to use a setTimeout with a citrus reconciler.

When we create a timeout on the form #(citrus/dispatch! reconciler :event args) it seems like it wont touch the reconciler state.

The event handlers can, of course, use the reconciler with dispatches.

Hello!

I see that the reconciler accepts a batched-update function, which default to js/requestAnimationFrame (https://github.com/roman01la/citrus/blob/056eb0ad0c5787fa26d208cf874e0eca95ce68d7/src/citrus/core.cljs#L33).

But in the scheduling code, the js/cancelAnimationFrame function is hardcoded (https://github.com/roman01la/citrus/blob/056eb0ad0c5787fa26d208cf874e0eca95ce68d7/src/citrus/reconciler.cljs#L13), which effectively prevents someone from providing another scheduling function.

Maybe we could provide the :batched-update as a map {:schedule-fn ... :release-fn ...} which would default to {:schedule-fn js/requestAnimationFrame :release-fn js/cancelAnimationFrame} ?

For example, in my current project I'd really like to be able to use {:schedule-fn js/setTimeout :release-fn js/clearTimeout} (which is more or less what reframe does).

Happy to provide a PR if you like the idea!

Hi,

Thinking about our discussion at EuroClojure Berlin, I finally think middlewares could be useful. Indeed watching state changes is nice but doesn't allow logging/watching events and doing more advanced stuff.

So starting a discussion about them.

This is the first draft that describes possible approach to effects handling in Scrum, inspired by re-frame. Code example is available in this gist.

Controllers

A controller returns description of a side-effects as data. :state is a built-in effects handler.

Side-effects description should follow these rules:

• description is a hash map
• a key is an identifier of a particular effect handler (:state, :http, etc.)
• a value is a value which satisfies particular effect handler

(defmethod control :inc [_ _ state]
  {:state (update state :count inc)
   :http {:method :put
          :uri "/api/count"
          :data (inc (:count state))
          :on-success :inc-ready
          :on-fail :inc-fail}})

Reconciler

New key :effects is added. The value is a hash map from effect handler identifier to handler function.

(def r
  (scrum/reconciler
    {:state (atom {})
     :controllers {:counter counter/control}}
     :effects {:http effects/http}))

Effect handlers

Effect handler is a function that receives...

• an instance of Reconciler
• controller identifier
• effect description returned by controller

(defn http [reconciler controller {:keys [method uri data on-success on-fail]}]
  (-> (httpurr/get uri {:method method :data data})
        (p/then #(scrum/dispatch! reconciler controller on-success %))
        (p/catch #(scrum/dispatch! reconciler controller on-fail %))))

Execution flow

• Dispatch an event (scrum/dispatch! r :user :load user-id)
• Triggered controller returns effect description {:http {...}}
• Reconciler executes appropriate effects handler passing in itself, controller name and effect value
• Once effect performed it may dispatch (server response, etc.)

Advantages

• Сonvenient async effects handling (no need to pass reconciler instance into controller to dispatch response later)
• Pluggable & reusable effect handlers

Hi, I brought this up on the slack, but it got lost due to no history.

There is some surprising behaviour when mixing dispatch and dispatch sync.

;; sync inside async gets lost
(defmulti controller (fn [event] event))

(defmethod controller :one []
  (println "one")
  {:state {1 true}
   :effect nil})

(defmethod controller :two [_ _ state]
  (println "two:" state)
  {:state (assoc state 2 true)
   :effect2 nil})

(defmethod controller :three [_ _ state]
  (println "three:" state)
  {:state (assoc state 3 true)
   :effect3 nil})

(defmethod controller :four [_ _ state]
  (println "four:" state)
  {:state (assoc state 4 true)})


(defn effect [r _ _]
  (citrus/dispatch-sync! r :controller :two))

(defn effect2 [r _ _]
  (citrus/dispatch-sync! r :controller :three))

(defn effect3 [r _ _]
  (citrus/dispatch-sync! r :controller :four))


(defonce recon
  (citrus/reconciler {:state (atom {})
                      :controllers {:controller controller}
                      :effect-handlers {:effect effect
                                        :effect2 effect2
                                        :effect3 effect3}}))

(citrus/dispatch! recon :controller :one)

The output of this is

one
two: nil
three: {2 true}
four: {2 true, 3 true}

and then if you print the reconciler once its all run

#object [citrus.reconciler.Reconciler {:val {:controller {1 true}}}]

The apparent behaviour is that first the update from the dispatch! isn't reflected in the subsequent dispatch-sync! method calls. However if you then check the state of the reconciler its only the result of the dispatch! that is reflected.

If you change effect to dispatch! as well, like so

(defn effect [r _ _]
  (citrus/dispatch! r :controller :two))

you get the following output

one
two: {1 true}
three: {1 true}
four: {1 true, 3 true}
#object [citrus.reconciler.Reconciler {:val {:controller {1 true, 2 true}}}]

So which indicates the "bug" is when you switch between dispatch! and dispatch-sync!

If you flip the problem and start from a dispatch-sync! and go to dispatch! it appears to work as expected.

(citrus/dispatch-sync! recon :controller :one)

(defn effect [r _ _]
  (citrus/dispatch! r :controller :two))

(defn effect2 [r _ _]
  (citrus/dispatch! r :controller :three))

(defn effect3 [r _ _]
  (citrus/dispatch! r :controller :four))

Output:

one
two: {1 true}
three: {1 true, 2 true}
four: {1 true, 2 true, 3 true}
#object [citrus.reconciler.Reconciler {:val {:controller {1 true, 2 true, 3 true, 4 true}}}]

Wanted to raise this, even if the answer is simply a warning about mixing dispatch and dispatch-sync.

Currently when I edit some control file, figwheel re-eval that file and reconciler, so the state is set to (atom {}) again and again.

1. Is it possible to keep the state?
2. Is there a way to pull the whole state of reconciler? So I could save the state and load it back after eval

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.

Hi,

I'm not sure if this is a bug or something everyone knows about.
But coming from an new-to-cljs perspective the following confusion happend to me:

1. I had a cljs/figwheel-main/cider/rum setup set up
   1a) including a standard resources/public/index.html which contained some script[src="cljs-out/dev-main.js"]
   2b) the setup worked normally

2. read about citrus, decide to try it out

3. as soon as clj-commons/citrus is added to deps.edn, the build-in-figwheel-dev-server:9500 returns another index.html response with script[src="js/compiled/main.js"] or similar, which is not there.

4. how and why?

Hello @roman01la
Thanks for this project

I tried out scrum and i like it
But for me there is some lack of integration with developer tools (aka redux dev tools)
It would be nice to have the ability to use debugger or logger for events and state changes
or maybe time travel debugger

Maybe i miss something and there is some tools for that?

Hi,

If the reconciler API is to be kept as is, you should update its docstring. The effect-handlers key isn't documented.

@slipset Could you please add @martinklepsch into collaborators here? Trying to make it easier for him to contribute to Citrus since he's using it atm. Thanks!

Ouch, I just stumbled upon what I think is a bug. Not sure.

Say you have this reconciler server-side :

(scrum/reconciler {:state (atom {})
                              :resolvers {[:a] (constantly {:b "b"})
                                                [:a :b] (constantly "b")}}) ;; the weird thing !

and the following subscriptions :

(defn a [reconciler]
  (scrum/subscription reconciler [:a]))

(defn b [reconciler]
  (scrum/subscription reconciler [:a :b] #(some-function %)))

As it is the code works. But if I remove the "weird" resolver (the line with the comment) I get a NullPointerException when loading the page : (relevant stacktrace)

Caused by: java.lang.NullPointerException: null
        at scrum.resolver.Resolver.deref(resolver.clj:8)
        at clojure.core$deref.invokeStatic(core.clj:2310)
        at clojure.core$deref.invoke(core.clj:2296)
        at my.ns.my-component$fn__41563.invokeStatic(my-component.cljc:116)

I can't just remove the first resolver, it's really used somewhere else in my app.

My understanding of the code in resolver.clj is that when there's a subscription, at the moment you rum/react it (which is just clojure.core/deref server-side), it does a simple (get resolvers path). In my case path is [:a :b]. So this path must exist in the resolvers map, even if it means duplicating part of the initial state across several paths.

I know that by design, only rum/reacted subscription hence paths will have their corresponding resolving functions called. But it means that deep subscriptions must either have a top-level subscription executed before to get the child path values or have child path values duplicated in proper deep path in resolvers map.

I hope I was clear.

Edit : after some thinking, I realized I could just have this subscription instead and call it a day :

(defn b [reconciler]
  (scrum/subscription reconciler [:a] #(some-function (:b %)))

That is, reading the nested value in my aggregate function. But I think this make subscriptions mostly useless. Reading your subscriptions examples in the readme, I don't see how using paths like [:users 0 :fname] could work without the corresponding resolving function at this exact path in the reconciler.

Maybe the solution is not to do a simple get in resolver.clj but a smart reduce :

(deftype Resolver [state resolvers path reducer]

  clojure.lang.IDeref
  (deref [_]
    (let [resolve (get resolvers path) ;; use reduce instead
          data (resolve)]
      (when state
        (swap! state assoc-in path data))
      (if reducer
        (reducer data)
        data))))

That is, the reducing code should check for each fragment of path if there's a corresponding resolving function and call if it exists. When no resolving function are found, it should get at this point (or maybe get-in ?).

Long issue. End of work day gotta go. I hope this issue is helpful.

Hi,

I think I spotted a bug:

• in citrus/cursor.cljs, the IDeref implementation is always calling get-in first, then the reducer (with a fallback to identity if not provided).
• in citrus/resolver.clj, the clojure.lang.IDeref does things differently depending on whether a reducer is present or not. If not present, it just calls get-in. But if there's a reducer, it calls reducer first, then get-in, when I think it should be the other way around.

So for a subscription like this:

(defn products-count [reconciler]
  (citrus/subscription reconciler [:products :list] count))

In CLJ it will call (-> data count (get-in [:products :list])), whereas in CLJS it will do (-> data (get-in [:products :list]) count).

Am I right ?

Thanks for sharing this library. I don't see coeffect mentioned anywhere so I ask

EDIT 2020-03-09: This thread discusses the introduction of a :default-handler option – please share your feedback/experiences if you've used it.

Hi all,

We’re currently looking into adapting Citrus so that all controllers methods can access the state of other controllers. In our particular use case we found that we often end up passing central information around in events such as the current users ID. This adds complexity to components because components then need to get this data out of the :users controller first.

We're interested in either something that would allow us to have "shared state" between controllers or a way that would allow us to access individual controllers' state from the methods of a different controller.

There's a few different options I've been mulling over, would be curious what others thing about those.

1. Pass entire reconciler state as 4th argument to multimethods

• Probably a breaking change
• Should be hidden behind a config flag I assume

Handlers would take the fourth argument

(defmethod control :foo
  [_ event controller-state reconciler-state]
  ,,,)

2. Default handler option

This is an option I'm very curious about because I think it makes Citrus very versatile and allows existing codebases to gradually migrate out of the predefined controller design while maintaining backwards compatibility. This is not to say that the controller structure is bad. But codebases grow and knowing that it's possible to break out when needed can be great for peace of mind. I know we are at a stage where we have some long-term concerns around building on top of Citrus but a rewrite is just not something that we'll prioritize over product work.

The basic idea is that whenever dispatch! is called with a controller name that doesn't exist in the reconciler it will call a default handler with the following args:

(default-handler reconciler-instance controller event-key event-args)

This would allow user-land code to basically do anything. For instance we could remove some of our controllers from the :controllers map and implement a default-handler like this:

;; let's assume we only need different behavior for the :user controller
(defn default-handler [reconciler ctrl event-key event-args]
  (if (= :user ctrl)
    ;; we pass the entire reconciler state as a fourth argument
    ;; to make everything pluggable there could also be a function that
    ;; processes effects returned by the function below, e.g.
    ;; (citrus/handle-effects reconciler (user/control ,,,))
    (user/control event-key event-args (:user @reconciler) @reconciler)
    (throw (ex-info "Not implemented." {}))))

This implements the suggestion in 1. but possibilities are endless. We could implement an interceptor chain on top of this for instance, which I believe isn't possible with the current controller/multimethod based approach.

I'm not sure what or if this would break with regards to other Citrus features but I think it would be amazing if Citrus had an escape hatch like this that would allow implementing more complex handlers than what's possible with multimethods.

Other solutions to the general problem of accessing central data

Use broadcast! more

• Use broadcast! to make central pieces of information available to multiple controllers
• Would require each controller to implement an appropriate handler

Custom effect to mirror data to multiple controllers

• Implement a :shared-state effect that writes data to every controller in the reconciler
• Would totally work but feels a little clumsy

Hello.

Funny thing, as I was developing https://github.com/pepe/showrum yesterday, I came to the edge of what is feasible with simple hash and methods and started to think about how to continue. And in the morning I found scrum 👍. Thank you very much for it, I love OpenSource!

As I was looking into the code, I found that DB is global for the library (and makes it a framework?), which brought to my mind this issue day8/re-frame#107 by @darwin in re-frame (which I was using extensively, but abandon it lately). I am still not sure, if it is actually problem or not, I just want to bring it to your attention.

Again, thank you, I will try to move showrum to scrum ASAP.

Tracking migration path as proposed in #16 (comment)

• rename to citrus
• bump to 3.0.0
• rename the repo
• update README
• Put a notice at the top of the repo readme, above the title
• publish an artifact
• Rename Slack channel
• Post on clojurescript mailing list and twitter
• Tell users to s/scrum/citrus/g relevant files
• Migrate example projects

In my current project, I am trying to implement a no-JS fallback to the most basic state controllers (ie. the ones that only emit :state). It would work like this:

1. The user clicks on a button, which is actually the submit of a hidden form
2. The browser calls the current URL as a POST call, with the controller name, the event name and the serialized params in the payload
3. The backend instanciates the reconciler, applies the related state controller (that would modify the related state branch), then renders the HTML page and sends it back

But I have two issues:

• First, I cannot actually manage to modify the state backend-side
• Also, I am not sure how useful the resolvers are

Why I cannot modify the state backend side

The current implementation of the Resolver is written such that it will always resolve the state branch with the initially given resolver function:

;; ...
clojure.lang.IDeref
(deref [_]
  (let [[key & path] path
        resolve (get resolver key)
        data (resolve)]
    (when state
      (swap! state assoc key data))
    (if reducer
      (reducer (get-in data path))
      (get-in data path))))
;; ...

By the way, this is also clearly stated in the doc.

There is no way to bypass that, except by instanciating a new Reconciler, with resolvers that return the modified state branches - which looks definitely smelly.

Can you see any other way to do that, with the current implementation of Citrus?

Also, a solution would be to first check if the related state branch is already in the state, before calling the resolver:

;; ...
clojure.lang.IDeref
(deref [_]
  (let [[key & path] path
        resolve (get resolver key)
        data (if (contains? state key)
               (get state key)
               (resolve))]
    (when state
      (swap! state assoc key data))
    (if reducer
      (reducer (get-in data path))
      (get-in data path))))
;; ...

Which by the way would deal with caching issue at the same time, and allow me to transform the state by just reset!ing the state atom.

Why using resolvers at all

I understand that the purpose of resolvers is to only load data that will actually be used in the UI. But the way I see it, I think it's not the best design:

• I have to deal with cache by myself
• I cannot concurrently load different data branches

So the code can become quite verbose, to have something that is not necessarily done the best possible way.

Meanwhile, if Citrus would simply skip this feature:

• I'd have to only load the data I want (which looks less good on paper, I admit)
• I could load my data the way I want (we already do this here, by the way)
• I would simply give the new reconciler the initial state or a fed atom

The state would no more be lazy, which would make manipulating it way easier. What do you think about it?

Hello, long time no bug report 😄

I noticed a small problem/inconsistency. When I dispatch! an event that's supposed to clear the current state by setting it to nil, the state isn't cleared at all. In fact it isn't changed.

Some code to show the problem :

(defmutli widget-controller (fn [event _ _ ] event))

(defmethod widget-controller :clear [_ _ _]
  {:state nil})

The problem comes from the Reconciler. The line (let [state-effects (filter (comp :state second) effects) ;; other bindings]) will prevent state declared with nil as a target value to be taken into account. Indeed, in the REPL :

(filter :a [{:a 1} {:a 2} {:a nil}])
;; => ({:a 1} {:a 2})

The problem doesn't happen if I dispatch-sync! instead. I think dispatch-sync! has the right behavior. nil is a totally valid value for piece of state.

Here's a solution, directly taken from the code of the binding for other-effects (just below the code linked above). First bad code example, second is solution :

;; Bad, current code
(let [effects [[:ctrl1 {:state 1 :some :effect}]
               [:ctrl2 {:state nil}]
               [:ctrl3 {:some :effect}]]] 
  (filter (comp :state second) effects))
;; => ([:ctrl1 {:state 1, :some :effect}]) ;; :ctrl2 has been filtered out ! Furthermore, the effects are still here (it's ok they're filtered just after though)

;; Good, solution proposed
(let [effects [[:ctrl1 {:state 1 :some :effect}]
               [:ctrl2 {:state nil}]
               [:ctrl3 {:some :effect}]]]
  (->> effects
       (map (fn [[cname effect]]
              [cname (select-keys effect [:state])]))
       (filter (comp seq second))))
;; => ([:ctrl1 {:state 1}] [:ctrl2 {:state nil}]) ;; :ctrl2 still here and only :state effects \o/

Note : I'm still using [org.roman01la/scrum "2.1.0-SNAPSHOT" :exclusions [rum]] but I'm not asking you to fix this old version. I'll move to citrus soon.

In the meantime, I'll just put an empty map when :clearing state.

End of bug report. Have a nice day !

Not sure if I'm missing something obvious, but I am having trouble getting this to work.

When I dispatch 2 events on the same controller, one after the other, they seem to overwrite each other's state.

Looking at the code for Reconciler, it looks like this is because the state for the controller gets extracted when enqueue-ing, and because of this it does not take any changes in state into account when running each event fn.

Is this expected behaviour?

Hi,

I want to talk about an idea I have of citrus that would be a breaking change but would IMHO make citrus-based code more
expressive, coherent and testable.

The problem

There is a strange difference between server-side and browser-side citrus.

• In the server, a resolver returns all the state necessary at once. A resolver is a map with all the necessary keys of the state filled up with some values.
  So one can write a single function that returns all the state, or several functions that return parts of it which can just merge.
• In the browser, controllers/event handlers are restricted to operate on a single part/domain of the state. To be more precise, a handler
  can only read this single part of the state and write to the same single part.

This restriction, while it could look good at first (this is the equivalent of Splitting reducers and combining them in Redux),
is overly restrictive, and means that events which originate from the app, from some user interaction maybe
must have their semantics tied to one specific part/subdomain of the state.

For a small example, imagine a settings form with one input. Every time a user submit the form, you want to do two things with the state:

• save the text from the input,
• increment a global counter of form submissions.

In the current design of citrus, it means one potentially has to dispatch two events:

:on-submit (fn [_]
            (citrus/dispatch! reconciler :settings :save input-value)
            (citrus/dispatch! reconciler :submissions :inc))

or handle the same event in 2 different controllers.

:on-submit (fn [_]
            (citrus/dispatch! reconciler :settings :settings-form-submitted)
            (citrus/dispatch! reconciler :submissions :settings-form-submitted))

Now, what if you wanted to save only when the global counter is less than 10 ? You can't do it in neither the :settings controller nor the :submissions controller!
You have to create an effect just to be able to get a handle on a reconciler so that you can read arbitrary part of the state.

The effect handler would look like this:

(let [new-submissions-counter (inc @(citrus/subscription reconciler :submissions))]
  (citrus/dispatch! reconciler :submissions :inc)
  (if (< 10 new-submissions-counter)
    (citrus/dispatch! reconciler :settings :save input-value)))

Which means an effect handler must be used whereas we do no side-effect, only manipulating state! Or you have to put this code in the UI component itself, which far from optimal.
It also means that in these cases our events look more like "RPC" calls. They don't convey the semantics of what happened, but are often named after the how to manipulate the state part.

Expressed more formally, the problem is that:

• writing to multiple parts of the state for one event is not possible, one has to dispatch one event per state slice or split the logic into different controllers.
• reading from several parts of the state for one event is not possible, one has to go either through dispatch -> effect -> multiple reads or logic in UI component -> multiple reads
• logic is spread everywhere, leading to less cohesive, difficult to maintain and hard to test code
• server-side and client-side citrus operate too differently

A proposed solution

Ideally one would dispatch! a single event like :settings-form-submitted, and do all the logic at once. To do that citrus needs to change in breaking ways. Here's what I think is the least breaking way:

• change nothing server-side
• client-side,
  • create a reconciler with a single controller multimethod that receive the whole state and returns only the part that changes
  • (unsure about it) still declares the top-level keys in the state and use this to check state effects.

API would roughly look like this:

;; single multimethod
(citrus/reconciler {:state (atom {:settings nil :submissions 0}) ; same as before, not empty here for the test below
                    :effect-handlers {} ;same as before
                    :controller control
                    :state-keys #{:settings :submissions}})

(defmulti control (fn [event] event))

;; coherent, cohesive
(defmethod control :settings-form-submitted [event [input-value] {:keys [submissions] :as current-state}]
    {:state {:settings input-value
             :submissions (inc submissions)}})

;; always testable
(deftest settings-form-submitted
   (citrus/dispatch-sync! reconciler :settings-form-submitted "some text")
   (is (= "some text" @(citrus/subscription reconciler [:settings]))
       (= 1 @(citrus/subscription reconciler [:submissions]))))

The top-level keys declaration is not mandatory at all to make it work, but it means without this we lose the information.
That's why we could also before setting the state check that the keys under the :state effect belong to the set of keys declared.

Pros/cons

Obvious cons: breaking change... It could exist in a fork/new version of citrus though.

Pros: more coherent code, events names only convey the semantics, every state-related change can happen in a single event handler. It would also make it easier to integrate a good logger into citrus, that would display both the event-name and the state before/after à la Redux-dev-tools.

Receiving the whole state isn't a problem, immutability got our back. And by the way this how Redux works out of the box, before you split the reducers.

The rest of citrus doesn't change. I think in terms of code size the change could be quite small.

What do you think ?

PS: sorry if there are errors/typos in this (again) long blob of text.
PS 2: pinging @jacomyal, I'd love this input (we already had a similar conversation at work, but I haven't exposed him this precise stuff).

It looks like updates using the :state effect replaces the entire state for the controller whose event returns the :state effect. I'm interested in figuring out how to perform a limited, assoc-in or update-in style update instead of clobbering the entire state for the controller.

One can write something like an update-in as an event handler, by taking the state argument, updating part of it, and emitting a :state effect to set to the update-ind state. However, if two such events are created at the same time, only one will work, because they all see and update the same starting state.

So it would be better to write an :update-in effect. From looking at the source, I'm not sure how to do that. It looks like :state is specially handled, at least in master. In particular, Reconcilers don't implement the swap interface, and I don't seem to be able just grab the (:state reconciler).

Am I overlooking an easy way to write :update-in as an effect?