imacrayon / alpine-ajax

An Alpine.js plugin for building server-powered frontends.
https://alpine-ajax.js.org
MIT License
561 stars 12 forks source link

No cleanup on window #22

Closed delaneyj closed 1 year ago

delaneyj commented 1 year ago

listenForSubmit(window) && listenForNavigate(window) both return cleanup functions that are never run.

imacrayon commented 1 year ago

Yeah, Alpine has an undocumented destroy hook that these cleanup functions were intended for, but I was waiting to get some more clarity there before implementing them.

Do you have a case where you’re needing those listeners cleaned up?

delaneyj commented 1 year ago

Yes they are creating garbage events that are part of the tab session. They is no guarantee when they'll be cleared, both if you used Alpine directive it has an explicit cleanup function

imacrayon commented 1 year ago

My hesitation with making x-target a directive is this:

It's fairly common to have hundreds of x-target's on the page when building dashboard UIs. If x-target was a directive, there would be at at least one event listener on the page for every x-target. Right now, for performance reasons, Alpine AJAX only registers 2 event listeners on window and all x-target elements delegate to those listeners instead of registering their own.

What if I added a global cleanup function on the Alpine object for those listeners? Something like Alpine.ajax.disable(), calling it would clean up the listeners and disable all x-target behavior on the page.

delaneyj commented 1 year ago

We've talked OOB but I think there is a definite trade-off. Right now you have a global function that is executing a full query on any update. A listener closure will cost more memory but will be only executed on trigger. The directive approach is more inline with normal usage of Alpine such as x-on. Maybe an example with 1000s of divs with addEventListener would prove it out either way. But seems like optimization for the wrong metric (cleanup and GC being already handled by the directive)

imacrayon commented 1 year ago

Yep, I'm warming up to the idea of making x-target a real directive. I think talking through more x-data uses will get me there.