-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
No destroy method == memory leak #135
Comments
Closed due to no reduced test case. |
so this plugin does a .on but it's on user to call .off ? |
Yeah, if it's something you need to turn off you can totally make that happen. |
So I would write several Good practice is to expose a destroy method. |
+1 on a single page app with many elements being created and destroyed this does leak memory, and reduce performance. |
Also we can't manually call off because the event function is not exposed to the app scope. |
+1 |
@FezVrasta fork it @davatron5000 is not open to it and not maintaining it anymore... |
I'm not opposed. Just busy. Will try to look at it today after a few @a11yproject things. |
@JSteunou I've already did it, unfortunately I need some more advanced feature. @davatron5000 thanks for the effort |
Not sure why the issue is closed though, as it clearly is an issue. |
@Petah It was closed due no reduced test case per the
I don't have a single line of code from you explaining what you're trying to do. I can't tell if this is super unique to your implementation or not. I don't know if you've rolled out @JSteunou's fork and if that has worked for you. I literally don't know anything at this point other than a few +1's. If I could get concrete feedback from anyone on this thread as opposed to pejorative quips, that would be much appreciated and I'll consider reopening. |
Ok, you bind an event with $(window).on('resize', myFn); you instead have to: var myFnBound = myFn.bind(arg1, arg2, arg3);
$(window).on('resize', myFnBound); and in the $(window).off('resize', myFnBound); |
I don't think its really possible to create a test case for not being able to unbind an event. |
there is a window.on but no off
you should expose a destroy method that unbind event listener.
The text was updated successfully, but these errors were encountered: