Use of rf/defn macro to register re-frame event handlers

[ulisesmac]

Hi all!

I've seen the code is using a defn macro in utils.re-frame to register re-frame event handlers.
I'd say it's a bit confusing to understand what it does and why it's necessary, also my IDE is showing me code errors since I tried to resolve it as clojure.core/defn and the macro generates a function taking the number of delcared args - 1.

I oppened this discussion to:

1. Share my thoughts about it.
2. Know what mobile devs think about the macro. If you support the use of this macro, please share your thoughts on why it is good/better/more confortable :)

Here's my list:

• Since this project is open source, creating these kind of custom macros could make harder for new people to contribute
• As far as I understand, this macro is not able to create events generated by re-frame.core/reg-event-db, it always creates them using re-frame.core/reg-event-fx, so we're always receiving cofx, which is not required by all events. When possible, generating events receiving only db is more simple and we're losing this abstraction.
• It's structure is not easily resolved by editors/IDEs:
  • We are linting this defn macro not as clojure.core/defn but clj-kondo.lint-as/def-catch-all which is a def ignoring its body and being lenient with linting.
  • I'm using IntelliJ, and I configured the macro to be resolved as clojure.core/defn but the function generated takes less arguments than the args declared, e.g.

    (rf/defn save-multiaccount-and-login-with-keycard
      [_ args] ;;<- Receives two args
      {:keycard/save-multiaccount-and-login args})
     ;; but it should be called with only one arg, causing some warning/errors in the editor
    (save-multiaccount-and-login-with-keycard params)

• IMO, most of the time re-frame's event handler creation macros are enough and simple to use.
• We are working on status_im2, particularly, we are implementing new onboarding UIs (which is part of the initialization of the app) and migrating/enhancing the old code, it'd be a good moment to slowly stop using it if we wish.

I must say I'm a big fan of using/writing custom macros, but I try to be very careful when making the decision of where and when to use/create them, also I try not to get too inventive with the API as I use the my IDE's symbol resolution very often.

Thanks for reading,
What do you guys think about it?

[flexsurfer]

Hey @ulisesmac thanks for opening the discussion and sharing your thoughts

So why this macros exist:
When we started working on status, we found that we use events a lot, and we had cases like this

function
-dispatch event1
-dispatch event2
-- dispatch event21
-- dispatch event22
-dispatch event3
-dispatch event4
etc

so lots of events and even events which dispatch lots of events itself

as you might know there is a state machine in re-frame, so these events will be dispatched in next js task, one by one, and its not so effective, also one event could rewrite results from previous events, but each event will trigger reagent render , yes there is optimizations but it works differently in RN, (reagent doesn't consider that it will be used in other env different from web browser), we had cases there js thread was busy and UI was slow
but the main reason we wanted to remove this event graph hell, we wanted events to work predictably, so we needed a merge functionality that just run all events in one go and merge the results and update app-db only once triggering reagent atoms reactions
we discussed this with other devs from other companies and they complain about having these issues as well, so our solution proved that it's pretty good

yes in IDEA it highlights macro with a warning, but at some point, I gave up and ignore these warnings

• As far as I understand, this macro is not able to create events generated by re-frame.core/reg-event-db, it always creates them using re-frame.core/reg-event-fx, so we're always receiving cofx, which is not required by all events. When possible, generating events receiving only db is more simple and we're losing this abstraction.

i would remove this sugar from re-frame, it adds only confusion, reg-event-fx should be used only, because there are tons of mistakes when you expect db but its cofx and vise versa

• I'm using IntelliJ, and I configured the macro to be resolved as clojure.core/defn but the function generated takes less arguments than the args declared, e.g.

yes thats a PITA, i've been living with this long time :( also Cursive doesn't work well with js interop :(

• IMO, most of the time re-frame's event handler creation macros are enough and simple to use.

it's not a macros it's just a regular function

btw we also have a rule that we shouldn't use :dispatch, :dispatch-n we should use fx/merge but there are exceptions for sure , some edge cases

hope this shed some light

[flexsurfer]

just to summarize

we have lots of small events, we want to combine them in one handler, order matters, re-frame doesn't guarantee it, so the example above might be executed like

-event2
-event1
-event4
-event3
-event21
-event22

so as you can see its completely unpredictable but developer might expect that event2 will be handled on top of the result of event1 in app-db

with fx/merge it will be

-event1
-event2
-event21
-event22
-event3
-event4

[ulisesmac]

just to summarize

we have lots of small events, we want to combine them in one handler, order matters, re-frame doesn't guarantee it, so the example above might be executed like

-event2 -event1 -event4 -event3 -event21 -event22

so as you can see its completely unpredictable but developer might expect that event2 will be handled on top of the result of event1 in app-db

with fx/merge it will be

-event1 -event2 -event21 -event22 -event3 -event4

Hey @flexsurfer!
I'm waiting for more people joining this discussion. I'll completely answer this later. BTW, thanks for your comments!

Just a quick note on:

we have lots of small events, we want to combine them in one handler, order matters, re-frame doesn't guarantee it, so the example above might be executed like

I guess using re-frame's built-in :fx effect is enough to guarantee the execution order 🤔

Do we have docs that I could check about this behaviour/decision?

I would like to have a small reproducible example showing re-frame's built-in limitations/problems vs the advantages you mentioned about this macro, but I know it would take a significant amount of time to create it, so, just in case you know a place in the code where this is exposed or a document explaining this situation, please share and I'll be happy to look at it!

Thanks again! 😃

[flexsurfer]

I guess using re-frame's built-in :fx effect is enough to guarantee the execution order

we want to order events not effects

Do we have docs that I could check about this behaviour/decision?

i believe we don't, the decision was made in slack , here is the PR #5972

just in case you know a place in the code where this is exposed

it's just everywhere in the code, any handler which uses rf/merge

for example on-return-from-background