-
-
Notifications
You must be signed in to change notification settings - Fork 715
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
Events Can Be Maps. So Too Subscription Queries #644
Comments
I've been thinking about similar things recently regarding my own experimental react-less framework and I'm somewhat at the same point you were at years ago. Currently I'm also choosing the "prettier" approach of vectors but actually would prefer maps and will likely move to them. As far as
Probably with some extra compatibility when |
Hi, we use same convention for dispatch: (dispatch ::event-key {...})
;;or in simple case
(dispatch ::just-key) internally dispatch will assoc event key into args map Or even go father and: (dispatch event-fn {...})
;; or to use defn meta
(dispatch #'event-fn {}) So compiler will check handler exists end skip exra handlers registry |
I personally have some little concern of changing the type of the events, but I guess the world evolves! I hope it will not create many breaking changes. To solve the problem I was using (reg-effect-fx
::some-event
(fn [{db :db} [_ {:keys [args]}]
{:db db})) Same for subscriptions. |
Just to expand a bit on what thheller has already said, because right now I'm in favor of that approach. In that case the context could look something like |
@thheller @p-himik So would result in a |
@mike-thompson-day8 Ah, sorry - I should've been more specific. My intention was to not pass event ID to the handler at all. I've never seen a need to know event ID in a handler, and I cannot think of one. Even if there is one, you can just create N handlers for N events. Regarding There's a problem with implicitly adding the event ID to the args map. You won't be able to pass the map to something that expects a strict set of keys. You'll have to be very careful and to
If someone needs to have the event ID there in the handler, then that need is very specific and explicit. I don't think it should be generalized to all handlers. There can be an interceptor that just injects the event ID into the coeffects. It's simple, it's explicit, there's 0 chance of unknowingly messing something up. |
@p-himik BTW, the intention is that key will be namespaced: |
@mike-thompson-day8 OK, now I've seen such an example. :) And that's a very specific one indeed. |
I do agree that having the Another option however maybe not putting the event-id into the map and instead use metadata ;; in dispatch, instead of assoc
(vary-meta args assoc ::event-id event-id)
;; ending up with
^{:re-frame.core/event-id :my.app/foo!} {:with "args"} I personally don't like using meta too much but it is an option. |
Using What are the problems in removing the event ID completely from the event arguments and moving it to an optional interceptor? Right now, I don't see any. |
tldr; I'm pretty hesitant to break with the precedent set by vectors that the This is only in response to taking event-id out of the map entirely, not an argument against the convenience of The precedent set by event vectors is co-location of the While adding event maps is a new thing, we're also keeping full backwards compatibility with vectors. There is a case for keeping these two mechanisms as similar as possible. In terms of event maps, co-location could be either assoc into the event map like: {re-frame.core/event-id :event-id :foo 1}` Or it could be metadata as suggested by @thheller There are also implementation details that means co-location of the I hear @p-himik that we could move to being more explicit about the We'd presumably change Now how does the new |
I don't understand this reason in the context of us breaking the precedent of using vectors for everything here.
I'm arguing strictly from the perspective of users. As long as they don't notice it at all, it shouldn't be an issue IMO.
I don't see any problem or the need for some complex passing here assuming we pass around a map like |
The main downside I see is that it will become harder to consume events for users. It might be worth it (I'm not debating that here), but if we do go down the road of making events polymorphic, please do make a generic accessor function |
@thheller (dispatch :some-event)
(dispatch :some-event {:with "args"}) That's because there is no equally pleasing way to replicate the sugar in effects: {:db (assoc db :show-twirly? true)
:dispatch ???} <-- what goes here
same when {:db (...)
:fx [ [:dispatch ???]
...more effects here...] So dispatching in views (with a sugary white lie) will be different to dispatching in effects (true map litteral ). That kind of inconsistency is never good. So, I've now swung back to thinkinig I just do map letterals everywhere (no special sugar in views). (dispatch {::rf/event-id :some-event :with "args"}) |
@p-himik Imagine a scenario where a button is pressed to add a customer. The event dispatched is a vector: (dispatch [:add-customer {...}]) where You like the current ability to spec check that Now this example was using a vector for the event. But if a map was used instead, you want a way to separate the event from the bag of other attributes, in those cases? If I have understood (sorry if I haven't), then what would be wrong with this event: (dispatch {:rf/event-id :add-customer :customer {...}}) Such an event would package up a map (shown again as BTW, I strongly suspect that your objection might now disappear because we are now NOT going to use the view sugar |
By this, do you mean harder to read for programmers? Harder to understand? Or some other form of consumption (or user)? |
No, I didn't say anything about spec'ing or checking.
Yes, but it should be generalized -
Nothing is wrong in terms of the data manipulation. As a side note - in your response to thheller you say: "Multiple ways of doing something just multiples cognitive load". The more I think about, the more that "lukewarm coffee" solution grows on me. There's 0 new cognitive load, 0 chance to break any existing library that relies on events being vectors, and a built-in great flexibility. That "white index lie" where the event ID goes first and all the arguments go after it is not sufficiently bad to be able to justify any changes by itself. From my perspective, it's almost the same as a map entry - it's also an indexed collection, always of two elements, where the key always goes first and the value goes second. It's not a |
@p-himik (dispatch [:event-id {:with "args"}]) You still dispatch a vector. But it is a 2-vector consisting of an event-id element, followed by a map of "the extras". This sure has one massive advantage - it requires no technical change at all. Indeed, it is already best practice for some re-frame programmers. (I'm afraid I haven't been as smart as they are). The only change would be that I'd have to modify the docs to describe it as a useful "convention" for reasons X and Y and Z. It does have the problem of "more nesting". That map is inside the vector. And that leads to a more complicated destructuring within the event handler. And it is a little more ugly when I do my "re-dispatch trick" (in the 2-step "weak example" I gave above). |
@mike-thompson-day8 Regarding more destructuring and making updating the event arguments uglier - there can be an interceptor for that. I already use |
@mike-thompson-day8 sorry, to clarify: harder for programmers using re-frame to write generic event-consuming code. For example: I've written effects that are parameterized with 'callback event prefixes': at the end of its execution, the effect will dispatch an event by appending a value at the end of the 'event prefix' vector. This would no longer work in general, as the callback event may now be a map, and the way to fill it would be Not saying this is re-frame's fault, but that's one situation where generic processing of events becomes more complex due to re-frame events becoming polymorphic in their data structure. |
@mike-thompson-day8 funny enough thats the same issue I'm currently mulling over in my framework. I actually want events themselves to be "declarative" when defining the DOM. So instead of [:div.foo {:on-click #(dispatch [::some-event! {:with "args"}])} "hello world"] I just have [:div.foo {:on-click [::some-event! {:with "args"}]} "hello world"]
;; slightly uglier with maps
[:div.foo {:on-click {::sg/event-id ::some-event! :with "args"}} "hello world"] Since I'm not constrained by react I like this much more than having actual functions, especially due to the affordances I get during DOM reconciliation. There is little chance I'm going back to functions after doing this for a while. I've even considered the approach of an uneven length vector being turned into a map (1 for event-id, +N K/V pairs). I don't think it provides much over the "lukewarm coffee" variant but might be something to consider. Harder to provide backwards compatibility though since not all vectors with uneven length will be usable as a map. [:div.foo {:on-click [::some-event! :with "args"]} "hello world"] The last approach is just using a regular function call but that sort of negates the aspect of trying to be declarative. Kinda looks worst overall in the hiccup context. [:div.foo {:on-click (sg/event ::some-event! {:with "args"}} "hello world"] I think however it would be fine in {:db (assoc db :show-twirly? true)
:dispatch (rf/event ::some-event! {:with "args"}} I don't know what to do for re-frame. Luckily I don't have to worry about staying backwards compatible with anything. Still not and easy choice though. I do think that maps are strictly better in every regard on the event handling side. I do remember seeing a lot of |
@thheller Unfortunately, React is an increasingly tangled mess which makes Components focal - what with Suspense and Hooks - and it won't be able to be pulled apart and put into separate workers. Your framework approach, with the various parts teased out into workers, looks the much better approach! I'll follow your progress with interest. :-) |
Yeah, I played with SharedWorkers but given their state I opted not to use them for the UI. IMHO the neo.mjs takes this way too far too. First of all you now have a single thread responsible for multiple tabs, which means it is even more performance constrained than before. Also DOM related things should stay in the main thread, moving VDOM to a worker is not practical and prohibits a large chunk of important stuff ever being performant enough. As seen in the horrid performance of some of their examples. Unfortunately moving stuff into a worker has its own issues and Browsers do very little to optimize them. Did you ever experiment with how re-frame would look when things were async? Thats the main issue that needs to be solved in order for Workers to work for anything. What happens if there is no sync Back on topic I think I'm about to settle on the simplest concept I can think of. [:div.foo {:on-click {:e ::some-event! :with "args"}} "hello world"]
;; respectively
[:div.foo {:on-click #(dispatch {:e ::some-event! :with "args"})} "hello world"] I think this was only bugging me (and you) because of trying to use a namespaced descriptive keyword and it getting too long and repetitive. In this context it doesn't actually matter though. |
@thheller this is getting off topic, but out of curiosity, how will you handle |
@mainej events are always dispatched to the component first. If that component does not handle the event it keeps going up the component tree to the root. The event handler signature is |
I personally like the simplicity of the existing way, as, I tend to use wrapped JS components where I convert vectors into calls to dispatch and I like the simplicity of terseness of the vectors: With that said, on the handler side, I don't really like that I basically always have If using a vector is limiting internally, there is nothing wrong with Alternatively, if you really do want to go with maps, but are retaining backwards compatibility anyway (as has been said), then the If I do need to use maps and type Those are the thoughts I had while reading this thread, at least. 🤷 |
I agree with this. I do like the idea of events being maps. However it concerns me that in large codebase it could become error prone to have vectors and maps mix and match since the handlers now have to be specific to one of the other. I’ve also had a good number of event handlers that dispatch to other event handlers. These will be other points that are easy to overlook. |
Is it important to distinguish between Currently you can't visually tell an event vector from a subscription vector (other than by adopting naming conventions for subs and events, which is what I'm currently doing. All keywords used for events are in ::events and all subs are in ::subs). Using Other variants: (BTW, I also like the idea of reducing keyword usage and go with direct function refs):
Makes it much easier to navigate the code and prevents mis-typing keywords. I've yet to see why keywords are better candidates for indirection compared to symbols. (happy to be enlightened!) |
Throwing my very late 2c in. A configurable dispatch-fn takes the pressure off picking names here. The event could become any type of data and (def default-dispatch-fn [x]
(cond (map? x) (::event-id x)
(vector? x) (first x)))
(def ^:dynamic *dispatch-fn* default-dispatch-fn) Possible complication is libs which need to construct their own dispatch calls - how would they know what key to use? If that's an important use case then perhaps the map key needs to be exposed. The 2-arity dispatch might help too. (def ^:dynamic *dispatch-key* ::event-id)
(defn dispatch-fn [x]
(cond (map? x) (get x *dispatch-key*)
(vector? (first x)))
(defn dispatch
([e] (queue e))
([id e] (dispatch (assoc e *dispatch-key* id))) |
Opening up to supporting anything might open up new audiences. Specifically, JS object support might reduce friction in regular JS components/libs. The dispatch function would be important for this to work. |
I fear this feature just complicates things and brings little value-add. The main rationale here is connasance of name being weaker than connasance of position. That aside, let's note that Re-Frame events look like fn calls (if you squint): [::my/event arg1 arg2] ;; <=> (my/event arg1 arg2) Let's reimagine fn call as maps: {:fn :defn
:name ::make-todo
:args {:title title, :due due}
:body {:fn :hash-map :1 :title :2 title :3 :due :4 due}}
{:fn ::make-todo, :title "Buy milk", :due "today"} ;; => '{:title "Buy milk", :due "today"}
{:fn :+, :1 3, :2 5} ;; => 8 If there's no value-add in the bit above, why would there be in event maps? (rf/reg-event-fx
:do-calc
(fn [{:keys [db]}
[event-id {:keys [flushed?] :or {flushed? false}} :as event]]
(if-not flushed?
;; step 1 -
{:db (assoc db :twirly true)
:dispatch ^:flush-dom (assoc-in event [1 :flushed?] true)} ;; <-- adding to the map
;; step 2 - hog the CPU
{:db (do-some-cpu-intensive-task db)}))) |
re-frame will soon allow:
events
to bemaps
.queries
to bemaps
.Events
Assuming this
ns
:You can still dispatch events which are vectors, as before:
but, now, you can also dispatch a map:
Within the map being dispatched, notice the key
:re-frame.core/eid
via the aliased namespace.eid
meansevent-id
. The value of that key is the event identifier. The other keys in the map are application-specific.Queries
You can still
subscribe
using a vector, as before:but, now, you can also subscribe using a map:
Within the map supplied to
subscribe
, notice the key:re-frame.core/qid
via the aliased namespace. The value of that key is the query-id. The other keys in the map are application-specific.Writing Handlers
You write handlers for map-encoded events as you would other handlers, it is just that the
event
argument is a map not a vector:Open Questions
:re-frame.core/eid
a good choice for map key? Other?:re-frame.core/qid
a good choice for map key? Other?They should be namespaced, yes? Is
eid
too cryptic? Prefer a longer more explicit name?I'm unhappy with
::rf/eid
. That's a horrible mouthful for a newbie. Maybe the tutorials just teach the vectors way and avoid exposing newbies to event maps early on.Timing
@superstructor will check-in a first cut (to
master
) within a day or so. But a release is probably a week away, We'll wait for feedback. And bring a couple of libraries and utilities along.Likely Objections
Question: Won't this approach make dispatching and subscribing more verbose?
Answer: Yes, it will. If you find that offensive, you don't have to use it. You can keep doing exactly what you are doing now. It is just that being able to use maps more easily solves some problems and we wanted to unlock that flexibility.
Question: Why not just use this partial approach
(dispatch [:event-id {:param1 arg1}])
. Ie. make events a 2-vector of[event id a-map]
. That would be backward compatible.Answer: We did consider it. But we felt that design would be lukewarm coffee - we felt it was right to go all-in on maps.
A Breaking Change?
We are "growing the API" by allowing events to be maps. So, in theory, this is not a breaking change.
However, that's cold comfort. If you do start to use these two new features in your application, you might find that
certain re-frame libraries and utilities might break because they expect vectors, and you are giving maps.
These libraries will have to be changed to accommodate the expanded API flexibility. So don't use maps in your application until all dependent libraries and utilities are updated.
We are aware that the following utilities/libraries will need to be changed:
Are we forgetting any?
Why?
Terse Version
For software systems, the arrow of time and the arrow of entropy are aligned. Changes inevitably happen.
When we use vectors in both these situations we are being "artificially placeful". The elements of the vector have names, but we are pretending they have indexes. That little white lie has some benefits (is terse and minimal) but it also makes the use of these structures a bit fragile WRT to change. Names (truth!) are more flexible and robust than indexes (a white lie).
The benefit of using names accumulate on two time scales:
Longer
Back in 2014, I wrestled with how to model events in re-frame. Should I choose to represent events with vectors or maps?
In the end, I choose vectors. If I remember correctly, that was partly because I saw Pedestal App
doing messaging with vectors and it was designed by VIPs (very important programmers). And partly because
I believe aesthetics/ergonomics matter a lot and, at the dispatch point, using a vector is minimal and neat.
I was aware I was giving up something flexibility-wise, but it didn't feel like too much. Tradeoffs.
Mike, did you just try to shift blame to the Pedestal App team?
Oh. It was that obvious, was it? Look, I was a young, foolish and impressionable functional programmer led astray.
Now, in the intervening years, I got wiser slowly. I have become aware that some "falsely positional"
arrangements - arguments to functions, for example - are inherently more fragile WRT to "evolution" (aka the passage of time).
But I thought I had got away with it. I thought that I had managed to get all the benefits of more minimal aesthetics,
without the downside of fragility. Until last week.
I was in a design session with a colleague, Denis, and we were kicking about ideas when I was suddenly struck a bit speechless because I realized that the placefulness of vector events had completely stopped me from considering various design options. If I allowed myself to think about the problem at hand with events being maps, the design fell out kinda easily because that better allowed for evolution (of data over time, within a running app - which was one of the two time scales I identified above).
I hadn't got away with it after all, and this issue, which became all too easy via maps, has been an irritating
pebble in my shoe for waaaaay too long. I'd had blinkers on. And I'd been trying to work around the fake placefulness of using vectors for this purpose.
So, here we are. I'm out of that cage. Change is afoot.
Final thought: vectors really have served us pretty well. There'a only a couple of places where the additional flexibility of maps is going to be useful, but they are important ones.
Weak Example
The following example is a little weak, but I want to keep things simple. Sometimes, you need to
do some very CPU intensive work. Something which will tie up the single browser thread available to you.
You need to update the DOM (show the twirly?) and then hog that thread doing that CPU intensive work.
So, this is a two step process. Update
app-db
to show the twirly, and then wait for the next animation frameto have passed so the UI is drawn. And only then start hogging the CPU. If you hog the CPU before the animation
frame, the twirly never gets shown.
If you have
:flush-dom
metadata on an event you dispatch, re-frame will wait until after the nextanimation frame before handling it.
Okay, so that's the setup. We're going to need a two step process.
But of course, this is all implementation detail. Somewhere within an app, a button will dispatch
an event which says "do calc" but that view knows nothing about the two steps.
Let's model the event emitted by the button click as a map;
(dispatch {::rf/eid :do-calc})
The handler looks like this:
Notice that the event changes over time, in two steps. And that ability to change makes the design an easy one.
And, yes, this is a weak example because it simple. It could be achieved via vector events fairly easily. Maps come into their own with other, more complicated async situations like HTTP operations. More on all that soon, once we get this stage done.
The text was updated successfully, but these errors were encountered: