Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Heads up about .bind() and #14 #16

Closed
yhahn opened this Issue · 2 comments

3 participants

@yhahn

First, thanks for merging #14.

I just ran into an interesting edge case where the callback .length based arg signature detection fails. If you use .bind() on a function, node will generate a new function which appears to be wrapped in such a way that .length does not accurately represent the "inner" function's arguments, e.g.

pool.acquire(function(err, resource) {
    // this works.
});
pool.acquire(function(err, resource) {
    // this fails -- looks like a 1 arg callback to node-pool.
}.bind(this));

I'm not sure what the best solution will be to this problem, or if it will affect most API users at all. It's easy enough to workaround so it might be worth documenting until we have a fix or the 1 arg callback support can be deprecated.

@pixelfreak

I don't fully understand what the problem is, but I just want to clarify that bind is not a Node.js function. It belongs to the V8 engine and what it does is change the scope of the function (basically, the this in the function) to whatever you pass in as the argument, sort of like call or apply (or proxy in jQuery).

@sandfox
Collaborator

Going to close this as it's ancient. Happy to accept a patch that changes this behaviour or re-open if this is still an problem for anyone.

@sandfox sandfox closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.