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

Bug-fix: set this.eventToCallbackMap property values to undefined instead of null on destroy() #644

Merged
merged 1 commit into from
Oct 30, 2016

Conversation

rovolution
Copy link
Collaborator

Fixes #640

@seiyria
Copy link
Owner

seiyria commented Oct 30, 2016

Looks good to me. Maybe a semantics thing, but would it make more sense to delete it instead of waiting for it to get gc'd?

@rovolution
Copy link
Collaborator Author

delete just removes the reference and marks it for GC on the subsequent GC pass. Won't make much of a difference other than removing the eventName key from the this. eventToCallbackMap object

@rovolution
Copy link
Collaborator Author

rovolution commented Oct 30, 2016

@seiyria went ahead and refactored to use delete. feels great to have a good unit test that validates the implementation change still achieves the desired functionality :)

@rovolution rovolution merged commit 1e763b6 into master Oct 30, 2016
@rovolution rovolution deleted the bugfix-60 branch October 30, 2016 04:15
@rovolution
Copy link
Collaborator Author

@snurby7 merged and published to 9.4.0! thanks again!

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.

2 participants