-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
lit/no-template-bind is not correct about auto-binding? #125
Comments
In your example, you don't use lit element, but lit directly. Lit 2.x does automatically bind if using With LitElement: class MyElement extends LitElement {
// ...
render() {
return html`
<div @click=${this._onClick}></div> <!-- THIS IS AUTOMATICALLY BOUND ->
`;
}
} With lit directly: class MyElement extends HTMLElement {
connectedCallback() {
render(html`
<div @click=${this._onClick}></div>
`, this, {host: this}); // <- `host` means all event handlers are bound to `this`
}
} |
Ah my bad, I see! This is why I was confused when seeing different results when trying to reproduce 😅 class MyElement extends LitElement {
logThis() {
console.log(this);
if (!(this instanceof MyElement)) {
alert("this not instanceof MyElement");
}
}
indirectLogThis() {
this.shadowRoot.querySelector("some-element").logThis();
}
render() {
return html`
<some-element .logThis=${this.logThis}></some-element>
<button @click=${this.indirectLogThis}>.logThis=logThis</button>
`;
}
} Am I still missing something? I noticed this being an issue when using 3rd-party components that require passing a function via properties... |
Lit doesn't know or care what you're passing in that case as its just a data binding. If you pass a function, it'll get a function, etc. In those cases its upto you to create a bound version of it, which you should do once in the constructor: class MyElement extends LitElement {
// ...
constructor() {
super();
this._logThis = this._logThis.bind(this);
// or
this._logThisBound = this._logThis.bind(this);
} then if you pass |
Okay, that's what I thought. This does mean that the |
it isn't showing false positives as you should bind those functions once in the constructor, not in your render method. otherwise, you'll be creating a function every time you render when i get time i will add more examples to the docs |
Oh, I see now! This rule is not really about the auto-binding, but about repeatedly binding a function in the LitElement render functions! |
ah thats a fair point. we should probably point out that its still valid to use |
First of all: Thank you so much for all your work and effort!
I've noticed that this plugin considers
.bind
-calls inside template-expressions an error, claiming that lit would auto-bind those. Trying to fix my projects unnecessary binding, I noticed that auto-binding seems to no longer be performed by lit (at least in version 2 it isn't).I've created a small project to replicate the error and a working use of the directive in a property-binding: https://github.com/lucaelin/lit-lint-issues/blob/main/issue-template-bind.js (please ignore the async-directive stuff, as it is related to a different issue in a different project)
This change seems to also affect the no-template-arrow rule, as it is build around the same assumption.
Kind regards!
The text was updated successfully, but these errors were encountered: