markVariableAsUsed for the customElement decorator

[what1s1ove]

Hello! Thank you for such a cool plugin!

There is currently an issue where if you use a custom component declaration through the customElement decorator on a class, ESLint with the no-unused-vars rule will complain that the class is not used, even though it is actually being used. This can be circumvented by declaring the custom component through the customElements.define function, but this is a workaround. Of course, it is preferable to use decorators.

In the typescript-eslint repository, you can find the following comment:

bradzacher commented on Mar 10, 2023
We do not encode semantics for framework-specific side-effects in our lint rules.
We purely work on the code you provide.
If the class is not referenced, but is indirectly used by a side-effect in the generator - then it's unused.
If you'd like to encode information about your specific framework's side-effects into the rule - you can freely do that by creating a new rule that marks the variable as used using ESLint's standard API, context.markVariableAsUsed:
https://eslint.org/docs/latest/extend/custom-rules#:~:text=markVariableAsUsed(name)%20%2D%20marks%20a%20variable%20with%20the%20given%20name%20in%20the%20current%20scope%20as%20used.%20This%20affects%20the%20no%2Dunused%2Dvars%20rule.%20Returns%20true%20if%20a%20variable%20with%20the%20given%20name%20was%20found%20and%20marked%20as%20used%2C%20otherwise%20false.
Here are two example rules that do exactly that:
https://github.com/jsx-eslint/eslint-plugin-react/blob/master/lib/rules/jsx-uses-react.js
https://github.com/jsx-eslint/eslint-plugin-react/blob/master/lib/rules/jsx-uses-vars.js

It seems reasonable to agree with it that frameworks should handle this themselves.

Another example is when I had to work with the ESLint plugin eslint-plugin-jsdoc, which exhibits similar behavior. By default, ESLint complains if you only use a type in comments, as it does not mark such values as markVariableAsUsed. To fix this, the eslint-plugin-jsdoc team created their own rule called no-undefined-types, which marks all values as used that are only used in comments via markVariableAsUsed.

I feel like something similar should emerge in eslint-plugin-lit. Or maybe I haven't found a better way to fix it. I'd be glad to hear any of your thoughts.

[43081j]

you're probably right that we should just mark CEs as used when passed through the customElement decorator

since it is lit-specific knowledge/side-effects, it seems to make sense we tackle it

i'll take a look unless you want to have a stab at it

[what1s1ove]

you're probably right that we should just mark CEs as used when passed through the customElement decorator

since it is lit-specific knowledge/side-effects, it seems to make sense we tackle it

i'll take a look unless you want to have a stab at it

Unfortunately, I won't have time to handle this at the moment. I've temporarily shifted away from the project where I'm using Lit. However, I don't like the fact that I'm currently using the customElements.define function instead of the customElement decorator. When I return to developing with Lit and if this issue hasn't been addressed yet, I'll take it upon myself to resolve it.

[43081j]

i had a think about this, its a bit of an awkward one for us to solve

basically, multiple rules work against lit classes conditionally (i.e. if (isLitClass(cls)) { doStuff(); }). we don't want isLitClass to have side effects (marking the class as used), but it seems strange that each rule would have the same mark-as-used logic, too.

it doesn't seem to belong anywhere sensible. maybe we just need to look at some examples elsewhere in the wild 🤔

im guessing other plugins just published their own version of no-unused-vars that overrides the original or something

[what1s1ove]

i had a think about this, its a bit of an awkward one for us to solve

basically, multiple rules work against lit classes conditionally (i.e. if (isLitClass(cls)) { doStuff(); }). we don't want isLitClass to have side effects (marking the class as used), but it seems strange that each rule would have the same mark-as-used logic, too.

it doesn't seem to belong anywhere sensible. maybe we just need to look at some examples elsewhere in the wild 🤔

im guessing other plugins just published their own version of no-unused-vars that overrides the original or something

I have one example. This is a no-undefined-types rule in the eslint-plugin-jsdoc package. Here is the source code for this rule https://github.com/gajus/eslint-plugin-jsdoc/blob/main/src/rules/noUndefinedTypes.js

[43081j]

that makes sense in their case as they have a rule for detecting those types

we don't really have a rule whose purpose is to detect registered custom elements, so there doesn't seem to be anywhere sensible to mark these variables 🤔

in the jsdoc plugin it works because they explicitly have a rule detecting those types. we don't explicitly have a rule for detecting element definitions

[what1s1ove]

that makes sense in their case as they have a rule for detecting those types

we don't really have a rule whose purpose is to detect registered custom elements, so there doesn't seem to be anywhere sensible to mark these variables 🤔

in the jsdoc plugin it works because they explicitly have a rule detecting those types. we don't explicitly have a rule for detecting element definitions

It seems that eslint-plugin-lit should have a similar rule to mark classes to which the customElement decorator is applied as markVariableAsUsed. Otherwise, there doesn't seem to be another way around it.
Or do you have a different vision on this?

[43081j]

the only reason the jsdoc plugin has a sensible place for it is because they have a rule specifically for detecting type references in JSDoc comments. they then mark them as used, makes sense.

meanwhile, in lit, we don't have a rule for detecting custom element references. so we don't have anywhere sensible to mark them as used

in the plugin right now there is no sensible place to do this logic.

if we had a rule which detected element definitions, what would it be detecting them for? see what i mean? we'd be making a no-op rule for the sole purpose of marking these classes as used (but it actually wouldn't lint anything)

hope im making sense, this isn't easy to explain

[what1s1ove]

Hey @43081j
Sorry for the delayed response!

I understand you, and it seems that indeed, if we add a new rule, it will simply mark classes as used, and nothing more.

The authors of typescript-eslint themselves suggest creating this rule in this comment. But I also agree with you. It's just not entirely clear how to solve this problem then.

[43081j]

Ah interesting, you can see the react plugin went exactly that route: a no-op rule that exists solely to mark some vars as used

I'll have a think about it this week, we may be able to do similar. A new rule like "custom-elements-uses-vars"

[jorg-vr]

Has there been any progress on this issue?

So far I have disabled the no unused vars rule in our eslint config, but this is not ideal

[43081j]

none yet as I have had an extremely busy month of travelling

i'd be happy to help guide someone to contribute this though