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

Fixing a memory leak. #106

Merged
merged 3 commits into from
Jul 13, 2017
Merged

Fixing a memory leak. #106

merged 3 commits into from
Jul 13, 2017

Conversation

ncochard
Copy link

Hello

With this pull request I am fixing the memory leak described in issue 104.

Firstly I had to change the package matchmedia to expose the dispose method so we can remove all listeners attached to the DOM. The author of matchmedia accepted my pull request but he never published the new version of matchmedia to the npm repository. So I published the corrected package under a different name matchmediaquery.

In this pull request:

  1. I make use of matchmediaquery rather than matchmedia.
  2. I call the dispose method to ensure that all listeners are removed.
  3. And I also make use of the utility cross-env so that this package can be compiled on windows.

I hope it all makes sense and that this pull request can be consumed. I've tested it in our project and it works well. If you have any question or comments, feel free to ask.

Regards

@ncochard ncochard mentioned this pull request Jul 13, 2017
@yocontra
Copy link
Owner

Thanks for doing the work on this one!

@yocontra yocontra merged commit f0c9f8d into yocontra:master Jul 13, 2017
@ncochard
Copy link
Author

Thank you very much. Much appreciated.
When you have a moment, would you mind running the following commands?

npm run build
npm publish

Thank you :-)

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

Successfully merging this pull request may close these issues.

None yet

2 participants