43081j / eslint-plugin-lit

lit-html support for ESLint
116 stars 21 forks source link

Feature request: detecting missing call to "super.connectedCallback" #137

Closed eric-burel closed 1 year ago

eric-burel commented 1 year ago

LitElement defines a connectedCallback that is necessary to call the render method.

Therefore, contrary to raw HTMLElement, a call to super.connectedCallback() is necessary if you don't want to break render.

Example:

// good, it's just an HTMLElement
class Foo extends HTMLElement {
  connectedCallback() {
     this.foo = "bar"
  }
}

//  bad => will prevent render
class Foo extends LitElement {
  connectedCallback() {
     this.foo = "bar"
  }
}
// good, super call is necessary for LitElement
class Foo extends LitElement {
  connectedCallback() {
     super.connectedCallback()
     this.foo = "bar"
  }
}

// bad (tricky use case I encounter, as we use an extension to LitElement)
class EvenBetterLitElement extends LitElement {
}
class Foo extends EvenBetterLitElement {
  connectedCallback() {
     this.foo = "bar"
  }
}

If we want to be very precise, the rule is that for every class that is a descendant of "ReactivElement", so "LitElement" and user-defined class that inherits "LitElement" such as my "EvenBetterLitElement", a call to lifecycle methods should start with a call to super.

It seems that eslint webc plugin ask you to list explicitely which classes are concerned, so I might have to manually add my "EvenBetterLitElement" to the list of classes that inherits LitElement.

In eslint-plugin-wc:

{
  "settings": {
    "wc": {
      "elementBaseClasses": ["LitElement"] // Recognize `LitElement` as a Custom Element base class
    }
// we could add a similar configuration as well to implement the "super" rule for lit
"lit": { "elementBaseClasses": ["EvenBetterLitElement"]}
  }
}

What's your opinion on this rule?

43081j commented 1 year ago

sorry its taken me so long to get to this.

i think it could be a useful rule, though would probably make more sense that it applies to all lit's required lifecycle methods (connectedCallback, update, updated, etc).

luchsamapparat commented 1 year ago

I saw that lifecycle-super has been added! This is great :) Are there any plans to release that rule anytime soon?

43081j commented 1 year ago

thanks for the reminder! i thought i'd already published it, my mistake

ill sort a release out soon :+1:

43081j commented 1 year ago

Published in v1.9.0