Skip to content

Commit

Permalink
fixed condext during construction of bound functions; now using native
Browse files Browse the repository at this point in the history
Function::bind when it is available. related: #1363
  • Loading branch information
michaelficarra committed May 27, 2011
1 parent 085874d commit 8d6e33c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 7 deletions.
14 changes: 12 additions & 2 deletions lib/nodes.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion src/nodes.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -1769,7 +1769,17 @@ UTILITIES =

# Create a function bound to the current value of "this".
bind: '''
function(fn, me){ return function(){ return fn.apply(me, arguments); }; }
(Function.prototype.bind ? Function.prototype.call.bind(Function.prototype.bind) : function(fn, me){

This comment has been minimized.

Copy link
@satyr

satyr May 28, 2011

Collaborator

Function.bind ? Function.call.bind(Function.bind) : function(fn, me) {...}

This comment has been minimized.

Copy link
@michaelficarra

michaelficarra May 28, 2011

Author Collaborator

I considered that, but Function.bind isn't necessarily Function.prototype.bind. Same for call.

This comment has been minimized.

Copy link
@satyr

satyr May 28, 2011

Collaborator

Function.prototype.bind isn't necessarily the native one too.

This comment has been minimized.

Copy link
@michaelficarra

michaelficarra May 28, 2011

Author Collaborator

But if someone changes Function::bind, they are asking for problems with Function::bind. If they add a bind property to Function, that shouldn't necessarily cause issues for things that rely on Function::bind.

return function bound(){
if (!(this instanceof bound))
return fn.apply(me, arguments);
var ctor = function(){};
ctor.prototype = fn.prototype;
var obj = new ctor();
fn.apply(obj, arguments);
return obj;
};
})
'''

# Discover if an item is in an array.
Expand Down
10 changes: 6 additions & 4 deletions test/classes.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,12 @@ test "`new` shouldn't add extra parens", ->


test "`new` works against bare function", ->

eq Date, new ->
eq this, new => this
Date
nonceA = {}
nonceB = null
eq nonceA, new ->
eq (new => nonceB = this), nonceB
notEqual this, new => this
nonceA


test "#1182: a subclass should be able to set its constructor to an external function", ->
Expand Down

6 comments on commit 8d6e33c

@michaelficarra
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crap, commit message is supposed to say "fixed context"

@jashkenas
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh .. what's up with the utility function? Can we leave this off master until we have an uncontroversial implementation of bind?

@michaelficarra
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That test case was broken. new => this should produce the instance of the anonymous method, not the outer context. Now Function::bind and our own __bind behave the same.

@satyr
Copy link
Collaborator

@satyr satyr commented on 8d6e33c May 27, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new => has use cases that deserve 7 lines?

@jashkenas
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three commits reverted on master -- if we want to explore alternative bind functions, we can do it on a branch.

@michaelficarra
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jashkenas: so what do you suggest we do about the errant test case and broken bind function? You said we should use the native bind when it's available (which is pretty much always), but we can't do that if it has different behaviour than the one we provide. It wasn't that big of an addition.

Please sign in to comment.