Fork mousetrap to fix listener leak #46
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
scottwittenburg
force-pushed
the
fork-mousetrap-to-fix-listener-leak
branch
from
684a143
to
8554168
Compare
|
Addresses #43 |
scottwittenburg
force-pushed
the
fork-mousetrap-to-fix-listener-leak
branch
from
8554168
to
961eba0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It seems the issue which affected our performance so badly, with over 100K event listeners attached to the DOM after scanning through many images, was not caused by
v-mousetrapfrom thevue-utilitieslibrary, but rather the underlyingmousetraplibrary itself.This issue in
mousetrapwas identified and this PR to fix it were submitted over two years ago, but neither have received any attention from the maintainer since that time. In fact, the maintainer of that project may have moved on to other things, as the last accepted PR was around a year ago.To verify the fix mentioned above, which looks to me like it should solve the problem, I pushed it to my fork of
mousetrapand have tried it out here. It works as advertised, so we could take this approach for now in order to solve our performance problem. However, we may want to look for another approach to key binding, since themousetrapproject may no longer be maintained.