Skip to content
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

Confusing behaviour when inheriting from classes with bound methods #4239

Closed
jameswilddev opened this issue Mar 31, 2016 · 6 comments
Closed
Milestone

Comments

@jameswilddev
Copy link

I understand why this is happening (methods are rebound in the constructor) but this is somewhat confusing; if you inherit from a class and don't call super in the constructor, or call it too late, the bound functions against the class are left unbound, which can be hard to notice as most of the time this has no ill effects, until you try to use one as a callback:

class abc
  x: => 4

class def extends abc
 constructor: ->
    # Uncomment this to make a.x != b.x
    # super
    return

a = new def()
b = new def()

alert "#{a.x is b.x}"

This could be fixed by moving the rebindings out of the constructor and into a generated function called automatically before any constructor anywhere down the inheritance tree is called.

@akre54
Copy link

akre54 commented Mar 31, 2016

Yeah that's just the way bound methods work. They are bound to the instance in the constructor, there's not a way to fix this in JS.

If you remove the subclass from your example, you'll see the same behavior.

class abc
  x: => 4

a = new abc()
b = new abc()

alert "#{a.x is b.x}"

The principle is the same in plain js:

function foo() {}
var a = foo.bind(null);
var b = foo.bind(null);

alert(a === b);

@jameswilddev
Copy link
Author

I think you may have misunderstood; a.x and b.x are not bound methods in my example, though they are supposed to be; the alert shown is "true". In your example, the alert shown is "false".

I'm saying this isn't necessarily wrong, but is very confusing; you've defined them as bound methods, and from a CoffeeScript user's perspective it's a declaration as part of the class, not the constructor.

Thinking about it some more, I think it would be reasonable not to add bound methods to the prototype. (as instances override them anyway) I'd rather it fail than execute the method unbound as it's more noticeable.

@akre54
Copy link

akre54 commented Mar 31, 2016

That's because by not calling super you're not actually calling the binding code, so they still point to the same method (abc.prototype.x). All kinds of weird stuff happens when you don't call super, not all of which can be guarded against. What would you have the desired behavior be? Every subclass is responsible for method binding?

@jameswilddev
Copy link
Author

Yeah. I'm saying that's quite confusing as a user; it's not wrong, but very confusing and an easy mistake to make without any obvious error showing up. You're declaring something as bound but a mistake elsewhere makes it unbound.

Putting the methods on the prototype when they are always overridden in properly constructed instances doesn't make much sense. If the methods were kept in the constructor's scope, improperly constructed objects would be missing the methods entirely rather than missing one subtle aspect of them.

Thanks for your feedback on this!

@vendethiel
Copy link
Collaborator

related: #2773

@GeoffreyBooth
Copy link
Collaborator

CS2 is banning bound methods in classes, as they’re not supported in ES classes and every implementation we’ve tried introduces unexpected behavior or subtle bugs. See #4530.

@GeoffreyBooth GeoffreyBooth modified the milestone: 2.0.0 Aug 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants