eslint-plugin-wc icon indicating copy to clipboard operation
eslint-plugin-wc copied to clipboard

connectedCallback super

Open LarsDenBakker opened this issue 5 years ago • 8 comments

I've been doing a lot of workshops where people learn to use lit-html. One of the most common mistakes is to forget calling super.connectedCallback() when using lit-element.

This would be a good linting rule to add.

This is a mirror issue of https://github.com/43081j/eslint-plugin-lit/issues/36, opened on request.

LarsDenBakker avatar Mar 08 '19 20:03 LarsDenBakker

There are two ways to detect a custom element:

  • @customElement JSDoc
  • Direct inheritance from HTMLElement

A lint rule must be able to operate on a single file out of context (i.e. eslint src/foo.ts should be possible without caring what foo imports). This rules out any complex hierarchical detection like walking the inheritance tree.

This is pretty much why the polymer linter made use of the same concept.

One thing we can investigate in future is detecting same-file customElements.define calls to infer the fact that the referenced class is an element.

Anyhow, back to the rule... its a good idea, we should definitely have something to check that lifecycle callbacks always call super (in a guarded way).

43081j avatar Mar 09 '19 11:03 43081j

I can see this covering these 2 cases

Checking for the presence of the @customElement JSDoc and that the baseClass is not HTMLElement, should allow us to assume that it is a standard customElement. That rule could be combined with the guard-super-call to ensure that you don't accidentally call a super method that doesn't exist/wasn't implemented.

The inverse of this rule could check that an element that is directly extending HTMLElement does NOT call super in the lifecycle hooks.

stramel avatar Mar 09 '19 18:03 stramel

I have recently updated this plugin in a TS project and I find this rule conflicted with latest typescript.

I inherit from LitElement, so the connectedCallback is definitely there. If I call it directly I get wc/guard-super-call. If I add the guard compiler complains

TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?

Should the plugin detect that the guard is not necessary in that case?

tpluscode avatar May 20 '20 07:05 tpluscode

@tpluscode its a rather unnecessary rule if you ask me, for typescript projects.

i'd highly recommend it be disabled if using typescript. if its in the default config, i may well remove it from there at some point.

for js projects, its super useful because you don't have any type support to know if there is a super call to make. in typescript projects, you should have strict compilation anyway and know when to call super based on the type system.

43081j avatar May 20 '20 16:05 43081j

@tpluscode its a rather unnecessary rule if you ask me, for typescript projects.

i'd highly recommend it be disabled if using typescript. if its in the default config, i may well remove it from there at some point.

for js projects, its super useful because you don't have any type support to know if there is a super call to make. in typescript projects, you should have strict compilation anyway and know when to call super based on the type system.

Is there any away for this plugin to detect whether its on typescript or not to disable this rule automatically to prevent the rule conflict?

AmirhosseinAzimyzadeh avatar Nov 25 '21 06:11 AmirhosseinAzimyzadeh

we could possibly guess based on the parser being used but that isn't the most reliable either. i think i may just remove it from the default config 🤔 and recommend non-js projects to enable it

43081j avatar Dec 15 '21 13:12 43081j

Is there any update on this? I think specially a missing super.disconnectedCallback() is even more critical as it can prevent memory leaks.

joekukish avatar Apr 20 '23 22:04 joekukish

i've opened 43081j/eslint-plugin-lit#173 for a lit specific solution at least

43081j avatar May 01 '23 11:05 43081j