Binding access when can lead to infinite recursion

[bartosz-m]

When rebinding method onto the same name, livescipt will generate code that leads to infinite recursion. I think it should generate compile error and deserves mention in documentation that binding operator isn't replacement for javascript .bind().

Foo = 
   v: 12
   init: !-> @bar = @~bar
   bar: -> @v

foo = Object.create Foo
    ..init!
foo-bar = foo.bar
foo-bar!

var Foo, x$, foo, fooBar;
Foo = {
  v: 12,
  init: function(){
    this.bar = bind$(this, 'bar');
  },
  bar: function(){
    return this.v;
  }
};
x$ = foo = Object.create(Foo);
x$.init();
fooBar = foo.bar;
fooBar();
function bind$(obj, key, target){
  return function(){ return (target || obj)[key].apply(obj, arguments) };
}

[rhendric]

Maybe this could be a compiler warning if it's a common enough mistake. It seems to me that the fundamental misunderstanding here is that binding access produces a ‘dynamically’ bound function, which uses the value of obj.bar at invocation time instead of at binding time. If you misunderstand that, you're exposing yourself to more pitfalls using binding access than just this one; conversely, I don't know how many people would make this kind of mistake if they do understand it.

[pepkin88]

Yes, I have noticed that too, but I don't remember the context. Moreover, this is not consistent with the behavior of partial application, where the binding is early, not late. I would consider this a bug, rather than a feature. Unless there are some use cases, for which such behavior is expected, but none is coming to my mind.

My proposal:

function bind$(obj, key, target){
  var func = (target || obj)[key];
  return function(){ return func.apply(obj, arguments) };
}

[rhendric]

Hmm. Well, this behavior has been inherited from Coco (see commit 29face5f9) which means it's been around for as long as LiveScript has been LiveScript. Based on the change made in that commit, I'm inclined to think this is a bug as well—at least, it appears not to have been satyr's intent to make the bind dynamic, since the comment on that commit only describes the change as a simplification. It might, however, be a bug that we're stuck with, since it would be a breaking change to revert.

I could be swayed on this one by enough popular support.

[pepkin88]

I don't think many people have noticed it, most of the time the effect is the same. Even for myself, I believe I stumbled upon this just by looking at the compiled code, not running it. Although the use case from OP seams plausible.

Regarding breaking changes, it's not like it hasn't done before. From top of my head, there was unary spread (had to change my code for that), bound parentheses functions (almost had to change my code for that), object rest for destructuring, and many more. They're different from case to case, I admit, but in my opinion changing to early binding would be at least harmless.

But for me, the priority for this decision is low.

[rhendric]

I've added a clarifying note describing the current state of affairs to the gh-pages-next staging branch. It'll go live with the 1.6 release. Does this seem satisfactory for now? I'm happy to revisit this behavior in the future if there is more activity.

[pepkin88]

👍 for temporary fix 👍 for revisiting in the future