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 #36

Closed
LarsDenBakker opened this issue Mar 8, 2019 · 7 comments
Closed

connectedCallback super #36

LarsDenBakker opened this issue Mar 8, 2019 · 7 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.

@stramel
Copy link
Contributor

stramel commented Mar 8, 2019

@LarsDenBakker, @43081j and I had previously discussed covering this in the eslint-plugin-wc at least for the standard hooks. 43081j/eslint-plugin-wc#15 (comment)

@LarsDenBakker
Copy link
Author

I intentionally didn't put it there, since you can't know which base class does or doesn't implement connectedCallback. If you find a good heuristic for it, then it could go there as well.

@43081j
Copy link
Owner

43081j commented Mar 8, 2019

Would you mind opening an issue in eslint-plugin-wc?

This would be a good rule. It can only really be detected by checking if the base class is HTMLElement or the class has a custom JSDoc annotation of @customElement.

This is the exact same reason why polymer-linter relied on the same JSDoc. A custom element's callbacks don't exist on an interface in the spec, they can exist or not on anything which can be registered as an element. Also, we can't do cross-file linting, which rules out inferring the base class through import traversal (because it goes against eslint conventions etc).

@stramel
Copy link
Contributor

stramel commented Mar 8, 2019

Yup, checking for the presence of the @customElement JSDoc and that the baseCalss 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.

@LarsDenBakker
Copy link
Author

Do you mean that the consumer should be required to add the @customElement annotation?

I'd prefer to either have this option in eslint-plugin-wc if it's possible to detect it in a good and performant way. Or just build it into eslint-plugin-lit as an opinionated option.

Another option is if you could set the base classes in the eslint-plugin-wc option, and eslint-plugin-lit configures it for you.

@43081j
Copy link
Owner

43081j commented Mar 9, 2019

It belongs in the web components plugin rather than here, as the same detection algorithm would have to be used anyway if it was here.

Closing this in place of 43081j/eslint-plugin-wc#32

@WickyNilliams
Copy link

In the lit context, don't you always want to call super.connectedCallback? no detection algorithm needed?

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

4 participants