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

Revisit .off() and .clear() #25

Merged
merged 5 commits into from Jan 8, 2018
Merged

Conversation

novemberborn
Copy link
Collaborator

Fixes #22. on(), onAny(), off() and offAny() now must be called with a listener function. You can no longer remove all listeners for a particular event. Renamed clear() to clearListeners() which is, well, clearer if Emittery is subclassed.

I discussed more advanced "disposal" behaviors in #22 but I think this is sufficient.


PR against #24 as to avoid having to resolve the inevitable merge conflicts.

This removes the previous behavior of removing all listeners for the event.
This method name is less likely to collide in subclasses.
@novemberborn
Copy link
Collaborator Author

I realized clearListeners(eventName) would still work quite nicely, so I've returned that behavior. It's now similar to off(eventName), but without the "oops my listener variable was undefined" footgun.

Note that you can't clear just the onAny() events. We could have another clearAnyListeners() function but I doubt it'll be necessary.

@sindresorhus sindresorhus changed the title Revisit off and clear Revisit .off() and .clear() Jan 8, 2018
@sindresorhus sindresorhus changed the base branch from hide-privates to master January 8, 2018 09:55
@sindresorhus sindresorhus changed the base branch from master to hide-privates January 8, 2018 09:55
@sindresorhus sindresorhus merged commit 95cfd0f into hide-privates Jan 8, 2018
@sindresorhus sindresorhus deleted the revisit-off-and-clear branch January 8, 2018 09:55
@sindresorhus
Copy link
Owner

Great! This is how I should have done it from the start :)

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