thheller / shadow-grove

A ClojureScript system to build browser based frontends
Eclipse Public License 1.0
254 stars 7 forks source link

Add support for using var names in event declarations #14

Open zeitstein opened 3 weeks ago

zeitstein commented 3 weeks ago

Motivation & Goals

Some reasons for using vars over keywords in event declarations (e.g. :on-click {:e ::handler}):

This topic has came up before:

However, keywords are good for introspection: logging, devtools, sending over wires, etc.

So, the goals would be:

  1. support declaring events with vars in addition to keywords
  2. retain the "just data" benefits of keywords

Same goes for fx handlers, of course.

Implementation discussion

Goal 1.

To get all the benefits of vars listed above, Fulcro-style declarations `[(some-var ~arg1 ~arg2)] wouldn't work. Anonymous functions (and factory functions producing those) have the downside of causing unnecessary re-renders.

So, the event declaration could look something like this:

(defn handler [env ev])
;; in ui
:on-click {:f handler :foo "foo" :bar bar}

or

(defn handler [env & args])
;; in ui
:on-click [handler "foo" bar]

The second approach has the benefit of letting the users define their handler signatures and is less verbose. I think the trade-off is that working with events downstream (e.g. in interceptors, over wires) becomes a bit more complicated with vectors vs. maps. The second approach is more general, while the first would be a fairly simple change.

One thing lost in the second approach is the :e/* opts like {:e ... :e/stop true}. Those are useful and would be good to figure out a way to incorporate them in the second approach.

In grove, rather than doing re-frame's {:dispatch-n [...]}, we do function composition:

(defn handler1 [env ev]
  (-> env (do-stuff ...) (handler2 ...)))

This was one of the reasons for the macro-driven metadata approach to defining handlers. So, it would be convenient for our event-declaring vars to be directly invokable.

Goal 2.

The difficulty is name-munging under :advanced compilation and the not-quite-reliable metadata in CLJS.

I don't have strong opinions on how to implement handler definition. (I've included a couple of examples in the commit linked below.) A grove-defined macro seems like the best idea, since it can hide the implementation details and leave some room for future extensions/modifications.

I have a sketch implementation (with examples) in this branch.

Out of scope but possibly relevant thoughts

thheller commented 3 weeks ago

One thing I feel you are mixing up or misusing is the term "var names". :on-click {:f handler :foo "foo" :bar bar} is a symbol, that references a local var. What ends up in the map is the actual function, and nothing related to the var.

So, :on-click {:f (fn [env ev] ...)} is effectively the same as far as the code is concerned. Not that this changes much, but as far is the code is concerned there is no way to go from the function to its var, unless using the var in the first place, i.e. #'handler. This however is completely out of the question given how vars work in CLJS and the amount of "bloat" they generate.

Rephrasing all of this to be what it actual is: "Add support for specifying handler functions directly in event declarations", since no actual vars are used anywhere.

So, we have opaque functions that carry no "identifier" and cannot be mapped back to data. But this identifier is useful, for the things you already mentioned. What you are after then is a way to somehow add this identifier to the function directly, so that grove internally (and others) can extract this identifier when needed, without the need for a "registry".

However, what is missing in this issue is how that would look. Your branch has this piece, which I guess does what is desired but isn't really something you'd want to write for every handler?

(set! (.-shadow_grove_ev_id ^js handler1) ::handler1)

I'm assuming you'd expect a macro for this? What would that look like?

This 0-arity fn thing to as a getter for the identifier is not something I'm keen on doing.

All the other things you mentioned are completely separate subjects, and should if anything be their own issues. I can say that event vectors will not happen.

zeitstein commented 3 weeks ago

Thanks for clearing up the confusion with vars. I was trying to be general / not prejudice the implementation. In principle, the symbols in event maps don't need to reference fns to reach the goals, afaict.

Macro, yes. I haven't thought much about it. Seems simple enough to just return a defn and the set! part.

Seems like it could be as simple as that.

In my grove-using project, I've extended the s.g.events/register-events! macro to create more specialised handlers based on handler metadata (e.g. register handlers that take and return (:db env) directly), but this is a user-only concern and it doesn't seem like the new macro will preclude it.

I can say that event vectors will not happen.

Just to be sure: do you mean the vector of events [ev1 ev2] to be dispatched or vector declaration of events [handler "foo" bar]?