Update defeffect to improve navigability

[phronmophobic]

Based on a suggestion from @holyjak, https://clojurians-log.clojureverse.org/membrane/2021-03-01

defeffect registers names of effects based off keywords. Registering keywords is also used by libraries like spec and re-frame. IDEs are starting to support navigation by registered keyword, but it's unclear whether it's implemented by adding specific support for specific re-frame and spec functions rather than a more general mechanism that membrane could tap into.

calva: https://github.com/BetterThanTomorrow/calva/issues/655 emacs/cider: https://github.com/oliyh/re-jump.el/blob/master/re-jump.el

One way to improve navigability is to take a cue from fulcro's mutations: https://book.fulcrologic.com/#_mutations In Fulcro, a defmutation expression will also create a function with the same name as the mutation that will return itself as a list when called.

For example:

;; Assume in ns.of.mutation you have:
(defmutation add-person ...)

;; Then:
user=> (require '[ns.of.mutation :refer [add-person]])
user=> (def name "Tony")
user=> (add-person {:name name})
;;=> (ns.of.mutation/add-person {:name "Tony"})

;;This is an important point. Thus you can write transactions without the need to quote:

(comp/transact! this [(add-person {:name name})])

Current example of a defeffect definition:

(defeffect ::toggle [$bool]
  (dispatch! :update $bool not))

Some other relevant considerations:

• The current syntax looks like it's registering an effect based off of the keyword (which it is), but it does not signal that it's also defining a function with the same name as the keyword
• Currently, any keyword with any namespace qualifier can be registered with defeffect, if defeffect were to define a function to match the keyword, it would be defining a var in another namespace! Alternatively, defeffect could require that all effects are namespaced and that effects are only defined for the current namespace. Since all defeffects are registered globally, this may be a good practice to enforce regardless.
• Allowing effects to be "called" to simply return a list can be confusing to newcomers since calling the effect "doesn't do anything".
• If we were to adopt fulcro's solution, it might also make sense to have intents return lists rather than vectors.
• In clojure, vectors are used for data while lists are used for code. Changing the way effects are written communicates a slightly different perspective. Compare the two following examples:

(on
 :mouse-down
 (fn [pos]
   [[::request-focus!]
    [::move-mouse pos]]
   (ui/label "Foobar")))

;; vs
(on
 :mouse-down
 (fn [pos]
   [(request-focus!)
    (move-mouse pos)]
   (ui/label "Foobar")))

The first is focuses on the intents as data while the second example emphasizes that effects are really a mini DSL for your application. While both are true, I prefer the data orientation.

• The paren version is slightly easier to read than the square bracket version
• One of the design goals of membrane is to separate the description of the intents from the implementation of the effect handlers that carry them out. The fulcro version encourages some amount of coupling (eg. importing the namespace that implements the effect from the namespace that returns the corresponding intent).
• Having an implementation of the effect also produce the data version of the intent makes it more awkward to define multiple implementations for an effect. Maybe intents should allow something similar to defprotocol so that each intent can idiomatically have multiple implementations. Maybe intents should be protocols?

There might also be other options besides fulcro's solution to make effects more easily navigable.

[holyjak]

I guess you could use symbols instead of keywords with the data version and tell your IDE that defeffect is a Def-like thing:

(defeffect fire! [..] (..)) .. [fire!...]

Cons: The ide will likely complain about the body of the defeffect referring to "Unknown" symbols such as dispatch! and the arguments.

[phronmophobic]

I think the biggest drawback is:

One of the design goals of membrane is to separate the description of the intents from the implementation of the effect handlers that carry them out. The fulcro version encourages some amount of coupling (eg. importing the namespace that implements the effect from the namespace that returns the corresponding intent).

Ultimately, I think the implementation should make it easy to maintain this separation.

[holyjak]

I see, so that seems to be contrary to the desire for navigability.

I understand that it is most desirable to separate issuing an intent and actually executing the effect. But I do not see (and that is because of my limitations) why is it so important that the intent is fully separated from the implementation. Is it so that you can replace it with a different implementation if you want to?

My experience as a developer is that those two are in practice very much linked. After all, I issue an intent because I want an effect. The ability to navigate to the effect implementation has been very valuable to me in the past. For example if I add some more data to the intent, I also want to change the effect to actually use the data. To understand the system, I want to know what a ::add-todo actually does and what is all the info it needs. So decoupling them so that finding that out will be difficult seems to be a pain...

[phronmophobic]

I see, so that seems to be contrary to the desire for navigability.

I don't think they're antithetical. I agree with you that one implementation per intent type is a common use case and I think supporting navigation would improve the developer experience noticeably. I also think it can be accomplished without introducing unnecessary coupling.

For instance, emacs/cider can support it with something like https://github.com/oliyh/re-jump.el/blob/master/re-jump.el. I know clojure-lsp has recently add support for "registered keywords", but I'm not sure if the support is a general solution that would work for membrane or if it's only adding support for specific library functions in re-frame and spec.

@holyjak , Which IDE are you using? I can look into if/how that IDE handles registered keywords for re-frame and spec.