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

Improve EventEmitter documentation #4554

Closed
ibc opened this issue Jan 6, 2016 · 8 comments
Closed

Improve EventEmitter documentation #4554

ibc opened this issue Jan 6, 2016 · 8 comments
Labels
doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter.

Comments

@ibc
Copy link

ibc commented Jan 6, 2016

Currently:

emitter.listenerCount(type)
type Value The type of event

emitter.listeners(event)
Returns a copy of the array of listeners for the specified event.

In both cases, type and event arguments refer to the "event name", so I suggest calling both "type".

@targos
Copy link
Member

targos commented Jan 6, 2016

+1 for consistency but I would go with event because it's the name used for the other methods (addListener, emit, ...).
Would you mind creating a PR to address this ?

@targos targos added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. labels Jan 6, 2016
@ibc
Copy link
Author

ibc commented Jan 6, 2016

Wow, I've forked the project and it found that it already uses event instead of type... :)

However I've sent a PR improving it a bit.

@chrisdickinson
Copy link
Contributor

I've been leaning towards the term "topic" since "event" can mean both the name of the event and the actual occurrence of the event.

@mscdex
Copy link
Contributor

mscdex commented Jan 6, 2016

"topic" to me sounds like something that involves a 2+-way conversation or something, when in this case it's a one-way thing. Maybe just use "event name?"

@sam-github
Copy link
Contributor

fwiw, "event name" in text makes sense to me, event in the API signature makes more sense to me:

In order to listen for the exit event, call process.on(event) with the event being the "exit" string.

Seems symmetrical and consistent - we call it the exit event, and we set the event argument to the string 'exit'. "topic" is just a word that doesn't show anywhere else in conversation about the event emitter or docs, so doesn't seem to help a lot (to me).

@ibc
Copy link
Author

ibc commented Jan 6, 2016

The DOM exposes a Event class/object to the user, but Node's EventEmitter does not. Instead EventEmitter just mentions:

  • listener (a function callback)
  • event (the name of the event)

Nothing else. And since there is no "Event" object at all it is 100% safe to use event as a simple string, because there is not anything else in the API.

@ryansobol
Copy link
Contributor

+1 for event. It seems pretty harmless to change it in the future if an Event class/object is ever added to the API.

benjamingr added a commit to benjamingr/io.js that referenced this issue Mar 22, 2016
Implementing the suggestion in
nodejs#4554 this pull request renames
the parameter name in all the places that accept an event name as a parameter.

Previously, the parameter has been called `event` or `type`. Now as suggested
it is consistently called `eventName`.

PR-URL:
Reviewed-By:
Reviewed-By:
benjamingr added a commit that referenced this issue Mar 24, 2016
Implementing the suggestion in
#4554 this pull request renames
the parameter name in all the places that accept an event name as a parameter.

Previously, the parameter has been called `event` or `type`. Now as suggested
it is consistently called `eventName`.

PR-URL: #5850
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
evanlucas pushed a commit that referenced this issue Mar 30, 2016
Implementing the suggestion in
#4554 this pull request renames
the parameter name in all the places that accept an event name as a parameter.

Previously, the parameter has been called `event` or `type`. Now as suggested
it is consistently called `eventName`.

PR-URL: #5850
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 30, 2016
Implementing the suggestion in
#4554 this pull request renames
the parameter name in all the places that accept an event name as a parameter.

Previously, the parameter has been called `event` or `type`. Now as suggested
it is consistently called `eventName`.

PR-URL: #5850
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 30, 2016
Implementing the suggestion in
#4554 this pull request renames
the parameter name in all the places that accept an event name as a parameter.

Previously, the parameter has been called `event` or `type`. Now as suggested
it is consistently called `eventName`.

PR-URL: #5850
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
evanlucas pushed a commit that referenced this issue Mar 31, 2016
Implementing the suggestion in
#4554 this pull request renames
the parameter name in all the places that accept an event name as a parameter.

Previously, the parameter has been called `event` or `type`. Now as suggested
it is consistently called `eventName`.

PR-URL: #5850
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@jasnell
Copy link
Member

jasnell commented Apr 22, 2016

This can be closed. The documentation has been updated to use eventName consistently

@jasnell jasnell closed this as completed Apr 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter.
Projects
None yet
Development

No branches or pull requests

7 participants