-
Notifications
You must be signed in to change notification settings - Fork 836
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
Note: Implementation uses explicit event delegation [sticky] #801
Comments
Any progress on this issue? I’m realizing that assigning DOM Also, does anyone have any microbenchmarks for whether |
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
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 |
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. |
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. |
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:
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.
The text was updated successfully, but these errors were encountered: