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.

Sign up for GitHub

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

Jump to bottom

JavaScript Memory leak in Materialize Dependency #11363

Closed
jvilk opened this issue Feb 19, 2017 · 4 comments
Closed

JavaScript Memory leak in Materialize Dependency #11363

jvilk opened this issue Feb 19, 2017 · 4 comments
Labels
wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.

Comments

@jvilk
Copy link

jvilk commented Feb 19, 2017

I discovered a memory leak in the materialize dependency. Every time you call material_select, the dependency installs a global click handler on window, but never cleans it up.

DataTable calls material_select via handleLimit, which is called every time a new DataTable is created via bindEventsAndApplyStyle. On the default Piwik dashboard, this occurs every time the dashboard is refreshed.

I'm filing this ticket here so you are aware of this leak, if you wish to avoid it at some point. If materialize does not fix this issue, a hackfix could be to remove all jQuery window-level click handlers when the page refreshes
@jvilk
Copy link
Author

jvilk commented Feb 20, 2017

It looks like this is not a bug that materialize is willing to fix. They made a suggestion concerning how you could fix this.

I am unable to help further with this bug, but I hope this information helps you should you decide to look into the leak.

@mattab
Copy link
Member

mattab commented Feb 21, 2017

Thanks @jvilk for all your leaks investigation 👍
We likely won't fix that as it should be a small leak.

@mattab mattab closed this as completed Feb 21, 2017
@mattab mattab added the wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. label Feb 21, 2017
@jvilk
Copy link
Author

jvilk commented Sep 14, 2017

@mattab Materialize fixed this issue in v100.2, if you're willing to upgrade.

Dogfalo/materialize@b6fb319

@mattab
Copy link
Member

mattab commented Sep 14, 2017

@jvilk thanks for the notice. Yes, we'd be willing to upgrade, maybe you'd like to open a pull request with the upgraded library? It would automatically run all our UI tests which would help detect any regression automatically. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mattab @jvilk