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

SIP.EventEmitter inherits from Node's EventEmitter #66

Closed
wants to merge 4 commits into from

Conversation

josephfrazier
Copy link
Contributor

An alternative to #64

resolves #64

josephfrazier referenced this pull request in josephfrazier/SIP.js Jul 17, 2014
This avoid sharing state between multiple instances.
@josephfrazier josephfrazier reopened this Jul 17, 2014
@josephfrazier josephfrazier force-pushed the eventemitter2 branch 8 times, most recently from b77e5ee to f0d3b2c Compare September 26, 2014 16:45
@josephfrazier josephfrazier added this to the 0.7.0 milestone Sep 26, 2014
@josephfrazier josephfrazier force-pushed the eventemitter2 branch 2 times, most recently from 8447dcf to a160b80 Compare October 5, 2014 04:27
Repository owner locked and limited conversation to collaborators Nov 24, 2014
@wakamoleguy
Copy link
Contributor

(I don't really know what locking this does.)

I just wanted to note that this has been accepted for the 0.7.0 version, rather than #64.

@josephfrazier
Copy link
Contributor Author

Excellent, I rebased onto master and added 59129bd to keep the bundle size down. Of course, I can squash the commits down before this gets merged, if desired.

This reduces code duplication by using a well known/tested module:
  http://nodejs.org/api/events.html

This also preserves most of the public API:
  http://sipjs.com/api/0.6.0/eventEmitter/

See below for more details:

* #on, #once and #off do NOT take third arguments (bindTarget)

  listeners should be explicitly bound to their objects before passing to
  EventEmitter methods

* #setMaxListeners works like Node's

  - does NOT prevent extra listeners
  - passing 0 allows unlimited listeners

* #emit no longer returns `this`, works like Node's

* #checkListener has been removed

  The rest of SIP.js uses:

    emitter.listeners(eventName).length

  instead of:

    emitter.checkListener(eventName)

* remove all other methods and their tests

Supporting changes:

* test/spec/SpecSession.js: define .logger on the constructed Session

  This prevents tests from failing
  due to an artificially constructed Session's logger being undefined
  since the Session no longer has a logger by virtue of being an EventEmitter

* src/Session.js: fix the following methods to correctly return `this`:

  - failed
  - rejected
  - canceled
  - accepted
  - terminated
  - connecting
(elsewhere): don't call #off except for in its tests
This avoids bundling the module into the browser build.
@wakamoleguy
Copy link
Contributor

Rebased and merged.

josephfrazier pushed a commit to josephfrazier/SIP.js that referenced this pull request Apr 27, 2015
@josephfrazier josephfrazier deleted the eventemitter2 branch May 2, 2016 14:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants