-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Ignore self binding in prefer-prototype-methods
#1343
Comments
prefer-prototype-methods
self bindingprefer-prototype-methods
Agreed. We can add an option if someone asks. |
Could you clarify why does |
Because
I think first one should be prefered, but if your class don't have a name to use, you can use the second one , not much difference. |
Hmm, See this example: class Parent {
constructor() {
const foo = this.foo.bind(this);
foo();
}
foo() {
console.log("Parent foo");
}
}
class Child extends Parent {
foo() {
console.log("Child foo");
}
}
const child = new Child(); // logs "Child foo" If you change it to But if you change it to That makes me think that But |
You are right about super classes. I'm thinking maybe we should only check |
People are disabling this rule https://github.com/search?q=prefer-prototype-method&type=commits |
It produces dubious warning to change `this.foo.bind(this)` to `ClassName.prototype.foo.bind(this)` or `this.constructor.prototype.foo.bind(this)` See sindresorhus/eslint-plugin-unicorn#1343
Was actually going to open an issue and then saw this. I'm also disabling this rule. At the very least, an option for self binding would be nice |
this.foo.bind(this)
andfoo.bar.bind(foo)
is very common.Even though
ClassName.prototype.method.bind(this)
make more sense, many people still prefer the old way as they are shorter and not really "borrowing" method, should we ignore it or check under an option?The text was updated successfully, but these errors were encountered: