Skip to content
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

connectedCallback super #32

Open
LarsDenBakker opened this issue Mar 8, 2019 · 8 comments
Open

connectedCallback super #32

LarsDenBakker opened this issue Mar 8, 2019 · 8 comments

Comments

@LarsDenBakker
Copy link

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 43081j/eslint-plugin-lit#36, opened on request.

@43081j
Copy link
Owner

43081j commented Mar 9, 2019

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).

@stramel
Copy link
Collaborator

stramel commented Mar 9, 2019

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.

@tpluscode
Copy link

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?

@43081j
Copy link
Owner

43081j commented May 20, 2020

@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.

@AmirhosseinAzimyzadeh
Copy link

@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?

@43081j
Copy link
Owner

43081j commented Dec 15, 2021

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

@joekukish
Copy link

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

@43081j
Copy link
Owner

43081j commented May 1, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants