-
Notifications
You must be signed in to change notification settings - Fork 225
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
no bind to context #1
Comments
That is correct, I also found that out during testing, but the biggest problem is that a lot of libraries are still depending on this context so I can't just remove it as it would make it harder to be a "drop-in" replacement of Node's Emitter. There isn't really that much we can do about this until node's EventEmitter also stop setting the context. On Thursday, September 26, 2013 at 7:04 AM, Julian Gruber wrote:
|
Use of passing the function context is to prevent needing to scope your functions. Say you have: var server = net.createServer(function(c) {
c.on('data', function onData(chunk) { });
}).listen(8000); On every connection the function Also, experience has shown me that real world apps call <= 3 arguments 99.9% of the time. And having function parameters declared but aren't passed by the calling function also incurs a performance hit as well. |
@3rd-Eden I was actually going to remove it, then a PR slipped in under my radar that added official documentation that the callback is called with the context. So... that's not being removed any time soon. :P |
What we could do is stringify the given function and search for the |
Closing this until node also supports no context binds. |
@trevnorris pointed out to me that binding to the eventemitter context when calling listener functions causes V8 to hold back on a lot of its optimisations and is way slower.
You could still pass the context as last argument, but I don't see the big usage for getting the context anyways, as when you start listening for an event - in almost all of the cases - you have the eventemitter instance in your scope anyways.
The text was updated successfully, but these errors were encountered: