naja-js / naja

Modern AJAX library for Nette Framework
https://naja.js.org
MIT License
109 stars 15 forks source link

UIHandler: Event delegation, introducing DOM handleEvent *) #400

Closed filiphlm closed 1 month ago

filiphlm commented 2 months ago

I have no ambition to "fix" anything, I am giving purely for consideration/discussion a modification that has worked well for me in an AJX-heavy / custom elements riddled application.

BC Break: The bindUI function has been removed as there is no further use for it.

*) Since 2000; see: https://dom.spec.whatwg.org/#interface-eventtarget

jiripudil commented 1 month ago

Hi,

first of all, sorry for the delayed response, I was taking a week off and then a week of catching up 😬

Thanks for opening the topic!

I prefer Naja's UI binding the way it is because it feels more transparent and straight-forward. I always believed performance implications to be negligible for the majority of use cases, and honestly, I still do.

That said, I understand the impact on performance may grow when your application gets complex enough to contain many interaction targets. I was in fact considering event delegation before, but decided to leave this for when somebody actually comes up with the issue. And here we are :)

Re your PR: I think it tries to do two unrelated things in one sweep:

filiphlm commented 1 month ago

And here we are :)

And I sincerely apologize for that.

About that article: The bottom line is, "You have to know exactly what you're doing and why you're doing it." That said, event delegation is still better/safer option in most cases, IMO. Yes, he's right about modern browsers. But still, every time you add a listener and don't reference it properly (which is unfortunately the "industry standard" approach), it's like creating a setInterval with no way to stop it. The only thing that changed over time is that we went from setInterval(()=>{}, 200) to setInterval(()={},10) 😄

handleEvent - to be honest, I don't see the value in this change.

I get it. I admit that there is a certain degree of convenience behind it. (Why use extra memory for a context when you have a native behavior that does it for you? And If someone accidentally adds same instance as listener twice, I'm still safe. Etc.) When I found out about this years ago, it was one of the "holy f***" moments, and I rarely use bind or arrow functions in listeners since then. But you're absolutely right, it is not necessary in the given context.

Would you be interested in implementing that?

Yes, sure. Although I have only a vague idea of the exact solution at this point.