Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for using var names in event declarations #14

Open
zeitstein opened this issue Oct 20, 2024 · 2 comments
Open

Add support for using var names in event declarations #14

zeitstein opened this issue Oct 20, 2024 · 2 comments

Comments

@zeitstein
Copy link
Contributor

zeitstein commented Oct 20, 2024

Motivation & Goals

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

  • editor conveniences for working with vars, e.g. jump to definition
  • vars need to be defined, :required, etc.
  • lint/compile checks: warning when attempting to use undefined handlers, lint marks for unused handlers, etc.
  • enforces (the familiar code org) discipline (might be a bad thing depending on your perspective)

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

  • It might be useful to allow dispatching of multiple events: :on-click [ev-1 ev-2 ...]. Not yet sure this is a good idea?
  • Would be nice for process-event and event-handling in general to be pluggable. In my experience with complex apps, it's common to customise the event handling system and introduce mini-DSLs to make writing handlers more ergonomic.
@thheller
Copy link
Owner

thheller commented Oct 20, 2024

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
Copy link
Contributor Author

zeitstein commented Oct 20, 2024

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.

  • Same signature as the defn macro.
  • Perhaps introduce (shadow.grove/set-handler-id! handler id) and (shadow.grove/get-handler-id handler) helpers. The former can be used in the macro.
  • Perhaps could be nice to hide that set! part behind a reader conditional, allowing handlers to be defined in .cljc files (they are pure fns after all).

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]?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants