developit / decko

:dash: The 3 most useful ES7 decorators: bind, debounce and memoize
https://developit.github.io/decko/
MIT License
1.03k stars 36 forks source link

@bind decorator bug when super method is called #12

Open precious opened 7 years ago

precious commented 7 years ago

This snippet demonstrates that the bind decorator does not work correctly on second method call if that method invokes super method which in turn is also decorated:

class A {
    @bind
    f() {
        console.log('I\'m A.f');
    }
}

class B extends A {
    @bind
    f() {
        console.log('I\'m B.f BEGIN');
        super.f();
        console.log('I\'m B.f END');
    }
}

let b = new B();
console.log('call b.f() 1st time:');
b.f();
console.log('call b.f() 2nd time:');
b.f();

Output:

call b.f() 1st time:
I'm B.f BEGIN
I'm A.f
I'm B.f END
call b.f() 2nd time:
I'm A.f
precious commented 7 years ago

This bug can be reproduced with babel es2015 preset. With .babelrc config

{"presets": ["es2015"], "plugins": ["transform-decorators-legacy"]}

and mocha option --compilers js:babel-core/register.

developit commented 7 years ago

Hmm interesting. I've never used this with prototypal inheritance, I can see how it'd overwrite itself like that. Not sure what the best solution for this would be, seems like any fix would break the lazy-binding behavior, which is sortof the big selling feature of this method.

precious commented 7 years ago

This fix may help. It prevents defineProperty for this if property is accessed from a super class. Let me know if this solution is acceptable -- I'll create a pull request.

wy193777 commented 6 years ago

@memoize also not working when super class' method is called.

Finesse commented 5 years ago

How does the example even work? When I run it, I get an error:

Cannot set property f of #\<B> which has only a getter