43081j / eslint-plugin-lit

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

no-useless-template-literals should not be in the recommended default #72

Closed daKmoR closed 3 years ago

daKmoR commented 4 years ago

when having html like

<my-el .propNumber=${10}></my-el>

the rule will say ${10} is not allowed here. However, to my knowledge, there is no other way to set the property as a number 10 with a different syntax.

setting a specific variable for it seems kinda unnecessary.

const forLinting = 10;

<my-el .propNumber=${forLinting}></my-el>

I think as a programmer/team you can decide to follow this rule. However, I would not consider this a recommended rule.

OpportunityLiu commented 3 years ago

no-useless-template-literals should not work in property bindings, but it's ok to be in the default for other positions.

jorenbroekema commented 3 years ago

Agreed with @OpportunityLiu , I think the rule in principle is fine but having it scan property bindings is likely unintended, and imo should be flagged as a bug.

43081j commented 3 years ago

yup that makes sense.

<div attr=${'foo'}> should still be invalid, but <div .prop=${'foo'}> shouldn't (since there's no other way to set it, i.e. you can't do .prop="foo").

jorenbroekema commented 3 years ago

This issue is still there for eslint-plugin-lit 1.4.0

43081j commented 3 years ago

can you possibly give an example?

there is a test here which passes fine in master:

https://github.com/43081j/eslint-plugin-lit/blob/ad4fa4613838698d6f8fa490a749865b9b27de0c/src/test/rules/no-useless-template-literals_test.ts#L28

The change was that the rule is still enabled but will allow properties to be set as literals

jorenbroekema commented 3 years ago
<lion-select
  name="favoriteColor"
  label="Favorite color"
  .modelValue=${'hotpink'}
>
Literals must not be substituted into attributes, set it directly instead (e.g. attr="value")eslintlit/no-useless-template-literals

Interestingly enough I get that error 5 times when I hover the squigly line in VS Code.

npm ls eslint-plugin-lit

lion-tsc@0.0.0 /home/joren/code/lion-tsc
└─┬ @open-wc/eslint-config@4.3.0
  └── eslint-plugin-lit@1.4.0

I scaffolded the project with npm init @open-wc --> new project --> web component --> with typescript + linting only

43081j commented 3 years ago

excellent, ive managed to reproduce it with your example.

ill work on a fix 👍

jorenbroekema commented 1 year ago

@43081j someone at work ran into this issue, was just wondering if it was fixed yet? If not, maybe reopen?

43081j commented 1 year ago

i wish i knew what i managed to reproduce back in 2021 🙈

I scaffolded the project with npm init @open-wc --> new project --> web component --> with typescript + linting only

i did exactly this and added the lion-select example, all worked as expected.

any chance you can help come up with a repro of where you've seen this happen @jorenbroekema ?