Skip to content
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: Memory leak for event handlers #155

Open
FlowIT-JIT opened this issue Mar 19, 2022 · 2 comments
Open

Events: Memory leak for event handlers #155

FlowIT-JIT opened this issue Mar 19, 2022 · 2 comments

Comments

@FlowIT-JIT
Copy link
Collaborator

Event handlers registered using Fit.Events.AddHandler(..) remains in memory unless explicitly removed using Fit.Events.RemoveHandler(..). That's because we allow handlers to be removed using an event ID returned from AddHandler(..), so we maintain an index that map IDs to elements and event handler functions. But obviously this comes with the major disadvantage that we must make sure to call Fit.Events.RemoveHandler(..) when an element is no longer needed. That's simply not acceptable as it prevents the browser's garbage collector from automatically cleaning up memory when DOM elements are no longer referenced.

Get rid of support for removing event handlers using an ID so we no longer keep references to elements and their event handler functions.

@FlowIT-JIT
Copy link
Collaborator Author

Check mutation observers as well - make sure they don't leak memory!

@FlowIT-JIT
Copy link
Collaborator Author

FlowIT-JIT commented May 16, 2023

Managing event handlers is a pain. Let's create a simple EventManager that makes it easy to get rid of all handlers registered, without us having to keep references to DOM elements, event handler functions, and event names. This should replace the use of Fit.Events.AddHandler(..) and Fit.Events.RemoveHandler(..).

// Create EventManager
var em = new Fit.EventManager();

// Register a ton of events
em.Add(elm, "click", function(e) {});
em.Add(elm, "keydown", function(e) {});
em.Add(elm, "touchstart", function(e) {});

// Register event that can be removed by an ID
em.Add(elm, "scroll", { Passive: true, Id: "MyScrollHandler" }, function(e) {});

// Remove individual event handler by ID
em.Remove("MyScrollHandler");

// Remove all event handlers
em.RemoveAll();

Replace everything found using this regex search (enable case sensitivity):
Fit\.Events\.(AddHandler|RemoveHandler|AddMutationObserver|RemoveMutationObserver)|\.on\w+ = \w

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant