43081j / eslint-plugin-lit

lit-html support for ESLint
120 stars 22 forks source link

option: list subclass names #148

Open bennypowers opened 1 year ago

bennypowers commented 1 year ago

We should provide an option that lists (string) subclass names, so that

export class IsReallyALitElement extends FooElement {
  /*...*/
}

lints up with our plugin

43081j commented 1 year ago

probably can re-use the same logic eslint-plugin-wc uses:

"settings": {
  "wc": {
    "elementBaseClasses": ["MyElement"]
  }
}

one idea is that we could use the same detection as eslint-plugin-wc: look for a customElement decorator or JSDoc on the class we're in. then if we see it has a render method and makes use of html, maybe we assume its a lit element?

bit of an awkward one though, lot of guesswork 😬

may be better to just do the base class stuff

freshp86 commented 5 months ago

Has there been any resolution/workaround here? Looking to possibly use this plugin, but all our Lit elements extend from a custom LitElement subclass, so it fails to find any errors when specifying lit/lifecycle-super checks.

maxpatiiuk commented 5 months ago

My workaround, for now, has been to name my custom Lit class LitElement, and to use https://typescript-eslint.io/rules/no-restricted-imports/ to forbit importing LitElement directly from lit

freshp86 commented 5 months ago

Thanks for the suggestion. While the above would work, it would require a lot of renaming across the codebase so not ideal. An easier approach would be to directly patch the relevant rules locally to use our custom subclass name (see below result for git grep --no-index 'LitElement'

lib/rules/lifecycle-super.js:                node.superClass.name !== 'LitElement') {
lib/rules/no-property-change-update.js:                node.superClass.name !== 'LitElement') {
lib/rules/no-this-assign-in-render.js:                node.superClass.name !== 'LitElement') {

Having said that, this would also not work well for our case since we often use Mixins using the following pattern.

const MyElementBase = SomeMixin(MyLitElement);

class MyElement extends MyElementBase {...}

So we would need to supply a list of classes.

43081j commented 5 months ago

ill take a look at this at the weekend if i can 👍

pretty busy next few days but ill pick it up soon as i can