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

Add Server#addListener and Server#removeListener #1961

Closed
wants to merge 1 commit into from

Conversation

elierotenberg
Copy link

Now we can remove a previously attached main namespace listener directly, eg. removing a connection event handler.

@ghost
Copy link

ghost commented Feb 12, 2015

Bump

@ghost
Copy link

ghost commented Feb 12, 2015

I set RemoveListener function in the very first line of an event. After a test, I got 2/4 success. It's probably because Node.JS is async. removeListener is therefore not suitable for its usage.

I did not tested addListener as I don't know what it is for (there is already .on)

@nkzawa
Copy link
Contributor

nkzawa commented Feb 12, 2015

I'm not sure whether those shortcut methods are necessary.
I feel we should find a better way if intend to proxy all methods and flags.
Actually there are some others like once and removeAllListeners as well.

@ghost
Copy link

ghost commented Feb 12, 2015

@nkzawa Once ? Is there any documentation about that please ?

@nkzawa
Copy link
Contributor

nkzawa commented Feb 12, 2015

http://nodejs.org/api/events.html

Since Namespace is actually an EventEmitter.

@ghost
Copy link

ghost commented Feb 12, 2015

Oh, thanks I didn’t know :D

Sabri

On 12 Feb 2015, at 17:24, Naoyuki Kanezawa notifications@github.com wrote:

http://nodejs.org/api/events.html http://nodejs.org/api/events.html
Since Namespace is actually an EventEmitter.


Reply to this email directly or view it on GitHub #1961 (comment).

@darrachequesne
Copy link
Member

Closed by #2601. Thanks!

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

3 participants