43081j / eslint-plugin-lit

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

Enforce property attribute naming convention #187

Closed ajbrun closed 10 months ago

ajbrun commented 11 months ago

I may have missed it, but I've not seen anywhere that will enforce a property attribute's naming convention. I suggest a new rule, which will check properties to ensure that accept only attributes that adhere to the naming convention, which should default to kabab-case.

Examples: :x: - avatarName does not pass rule

  @property()
  avatarName: string = "";

:white_check_mark: - avatar-name passes rule

  @property({ attribute: 'avatar-name' })
  avatarName: string = "";

I could maybe also suggest a further rule to ensure the attribute name is the kebab representative for the camel cased property name perhaps so they don't become disjointed?

Examples: :x: - name does not pass rule

  @property({ attribute: 'name' })
  avatarName: string = "";

:white_check_mark: - avatar-name passes rule

  @property({ attribute: 'avatar-name' })
  avatarName: string = "";

Thoughts? Or is there something that already does this?

43081j commented 10 months ago

sorry im only just getting to this.

this is a great idea for a rule, i've been wanting the former for a while, detecting where people haven't used attribute and have ended up with a camel case name.

i can probably throw it together pretty quickly, but if you want to contribute and get there before me, go for it

ajbrun commented 10 months ago

I meant to have commented back on this myself, and am having second thoughts over if this is a sensible suggestion or not. On the face of it, I think it is. However given that "legacy" html properties don't follow the modern kebab-case standard, I wonder if it becomes too opinionated when there's already an inconsistency in the formats available.

EG, we have maxlength which is acceptably lower cased and unhyphenated. Given google has created lit and Material, it's perhaps best to take inspiration from the material library. I've noticed there, they've used lowercase where it's a "legacy" attribute, but kebab-cased where it's a custom one. If this is to be followed, it's perhaps hard to add a hard and fast rule to be followed...?

@43081j - sounds like you have some interest in this, but I'm happy for it to be closed given the variation described above. IMO, the only real way it could be achieve is if we have a list of all legacy attribute names, or rely on a link in a comment to some documentation in the property tsdoc comment. I feel the former is unlikely and the latter might not be sensible for a distributed ruleset?

43081j commented 10 months ago

you are right, its about convention rather than best practice i suppose

material follows the convention of implementing native attributes where possible, but kebab-case ones where not (i.e. custom attributes).

the rule i've added in the related pr (#191) basically does the minimum here and checks that all attributes are lowercase. that does mean if you have a camelCase property you purposely want a camelcase attribute for, you have to explicitly define it.

that's probably a good middleground, in that it isn't enforcing any conventions really but is ensuring you're aware your non-lowercase props will be lowercase as attributes.

what do you think? we don't need to go as complex as you're suggesting then, as we're simply asserting the attributes are all lowercase, whether you use kebab-case or not.