Note: Implementation uses explicit event delegation [sticky]
krausest opened this issue · 6 comments
This note (and it should be regarded as a note, not an issue) marks implementations that use explicit, i.e. manual, event delegation.
The note is somewhat controversial since there are multiple views on it:
- It's natural and best practice to use manual event delegation in vanilla js and in frameworks with a low abstraction level
- It's the fastest approach for all frameworks
- It doesn't show the cost of implicit, i.e. framework provided, event delegation that's typical available for frameworks with a higher level of abstraction. If a framework has such an implicit event delegation this mechanism should be measured in this benchmark. Otherwise we're only comparing vanillajs variations and not the frameworks. (And we already know the performance of vanillajs, so it doesn't add any value.)
Advice: Use whatever fits the idiomatic style of your framework, but please don't over optimize. This note adds a litte pressure to prevent over-optimizations.
Any progress on this issue? I’m realizing that assigning DOM onclick properties is eating a significant chunk of time on tests like rowlots. Is there a sliding scale? Am I allowed to read the row id off the DOM via ev.target? Will I be penalized if I have one event listener on the table vs one for each row? My framework doesn’t implement any sort of framework provided event delegation so I would argue it is idiomatic to use some kind of event delegation mechanism when you have a parent with many children.
Also, does anyone have any microbenchmarks for whether onevent properties are faster or slower than addEventListener calls? It would be a shame to have to switch for performance, I think virtual DOM libraries are actually the perfect use-case for onevent properties.
We don't seem to be marking libraries for this currently. So I don't believe there is any repercussion to doing so? Atleast today. #772 is more about manipulating the selection class directly.
I've argued against this being penalized since it is the defacto way to solve events in a large table. Either the framework does it, or you do it. Some libraries actually have this in the documentation as being the way to do it. Once you get to a certain point on the table everyone is using event delegation. It isn't like it is fundamentally changing the nature of the test.
I think this is sort of gray area in that it makes the implementation look less clean so it's discouraged. It breaks the abstraction wall. But it's hard to penalize someone for something that almost every other library is doing. I do see the argument that if we are critical on libraries using imperative escape hatches to directly manipulate DOM elements in ways the library already has means to but are being avoided purely for performance reasons, that being fine reading from them is a very subtle distinction to make when the library already has a way to attach events. To be fair explicit event delegation still generally uses the libraries event attaching mechanism, it just involves manual traversal of DOM trees.
But I think we need to decide if this is something worth penalizing on. And if not, leave to the implementor. I once made an event delegated version of Svelte but I wasn't comfortable merging it without Svelte community or Rich's blessing. I never got it so I retracted my PR.
It’s odd because it’s one of those optimizations that anyone working with large amounts of DOM nodes will go to first, but I understand the arguments against explicit event delegation. Specifically, you break encapsulation between your table and row components if the row cannot hide its internal DOM structure because the table is reaching into the row during click events. Then again, I don’t think any “row component” should be written in isolation from a table or tbody component, as I don’t think a generic tr component is very useful. Also, there are other cost cutting measures you can use which break encapsulation between the table and row components, which we don’t presumably care about, like assuming the item id passed in will never change for keyed implementations.
I once made an event delegated version of Svelte but I wasn't comfortable merging it without Svelte community or Rich's blessing. I never got it so I retracted my PR.
Is it just me or does Svelte have an even stronger case for event delegation because it doesn’t define a separate component for rows?
@brainkim
Yeah reactive libraries do not need a row components to handle the change detection (we need to build the guards into any loop iteration since it can be just as inefficient with anything component or not). And admittedly in reactive libraries are where the greatest use of explicit event delegation has been seen here. That being said if your framework is about writing less code having the implementation not write less code is equally concerning. I never got a response in either case. But I think Svelte is going for an aesthetic as much as performance.
I wouldn‘t call it „penalize“. It‘s not an error like #694, it‘s just a note that the implementation might take advantage from a user code event delegation.
If that‘s the style of your framework you shouldn‘t hesitate to submit it.
In the end I decided against doing event delegation because it wouldn’t be in good sport. Also finding the row id and making sure the target contains a specific tag seems non-trivial.