Changed EventEmitter inheritens to Node0.10 style. #113
Conversation
| @@ -25,6 +26,7 @@ function mixin(target, source) { | |||
| } | |||
|
|
|||
| function Request(uri, options) { | |||
| events.EventEmitter.call(this); | |||
There was a problem hiding this comment.
"call" is not part of EventEmitter in the current 0.10.x version. Not adding this line removes the error and overall this appears to have fixed the multiple result calls
There was a problem hiding this comment.
Yes it is - that's Function.prototype.call
There was a problem hiding this comment.
The removal of "call" is not necessary.
|
This fixes all the multiple result calls!! When is someone going to accept the merge? |
|
Thanks for the fix - it solved my issue and works. |
|
+1 -- is this going to be accepted so we can use the npm version again? |
|
+1 @taxilian |
|
+1 Yeah the npm version generates this error when deplying on Heroku: possible EventEmitter memory leak detected |
|
Eric's patch works for me. It's a tiny change. +1 for accepting the pull |
|
I changed the source with Eric's code and it worked on Heroku. +1 for accepting it. |
|
Also worked for me with nodejs v0.10.3. People who want to try Eric's fix now might enjoy this package.json dependencies line: |
|
Using the dependency line for Eric's repo solves this issue for me as well. Running node v0.10.9. |
|
Thanks @drewp, that will have to do for now (running v0.10.10). +1 to merge. |
|
Seems a pretty important problem on affected versions of node: while my application is running and the listener "hooking" gets reached, I add a listener without the chance to remove it. UP for having it "officially" on NPM. |
|
+1 This library is broken without this. |
|
+1 |
|
+1. I was debugging my own code for hours, thinking I had some kind of logic bug, but it turns out the bug is in restler. This pull request fixed it for me. |
|
I was having some really strange behavior occurring while calling +1ing for merge into master |
|
This issue is still happening, a merge would be nice |
|
Uggg...wasted a lot of time because of this issue. Please fix it. |
Changed EventEmitter inheritens to Node0.10 style.
|
Nice! |
|
Would be nice to have npm registry updated as well. |
|
Yes, it would be great also to know what restler npm version will include this fix (and would be even greater if this will happen shortly ;)). Thanks for your work! |
|
This would have saved me about 8 hours of work if it was in npm and I hadn't had to track it down here.... With that said, SO glad I found this for my sanity's sake. |
|
@danwrong Thanks! Could you please publish a new version to npm? |
|
@danwrong This is fantastic! 👍 Can't wait for the NPM update. 😄 |
|
Noob question - how do I get this fix please guys? :)
Doesn't seem to be working. |
|
Yeah, that link no longer works. This needs to be forked perhaps and a new npm project that updates. It's silly that the npm package isn't getting updated. |
|
"git+http://github.com/danwrong/restler.git" worked for me. Mind you this is scary to use since it's not a point in time. An npm bump is what is really needed here. |
|
@tobinharris: I deleted that fork since it's now been merged into master |
|
Thansks @eric-wieser / @cendrizzi. So, I'll just point at master for now. T |
|
Any word on this? I just have discovered this after 3 hours of debugging. Is there any chance the npm package will be updated? |
|
I'm trying to get owner access on npm. Once I've done that I'll push up a new version and try my best at keeping things updated. |
|
Let us know so that we can change our references. |
|
Will do! |
|
Any news on this? |
|
3.1.0 is out now!!! |
|
For some reason i am still getting the same issue. I will delete the module manually and install it again |
|
Updated to latest module, i have the same issue, the complete events are bubbling up. Can someone help me with a temporary fix? |
|
Its all good. It decide to work for some reason. Thank you. |
|
@easternbloc Great man, all right now on node 0.10.1. You brought back this library to its shine. |
This fixes #110, and fixes #112