eslint-plugin-lit icon indicating copy to clipboard operation
eslint-plugin-lit copied to clipboard

feat: add strictSnakeCase option to rule attribute-names

Open jpradelle opened this issue 1 year ago • 5 comments

I would like to force attribute name to be the exact snake-case version of property, like PolymerJS did, I added option strictSnakeCase to attribute-names rule to check that.

With that option, the only possible attributes values are

@property({attribute: 'camel-case-name'})
camelCaseName: string;

// or
@property({attribute: false})
camelCaseName: string;

jpradelle avatar Jul 23 '24 09:07 jpradelle

i feel like if we're to do this, we should provide a more generic option to account for the current behaviour too

e.g. style: "snake" | "none" (none being the default, in that the attr name is the same as the prop name but lowercased)

43081j avatar Jul 29 '24 16:07 43081j

Here it is with style option. By default no style is applied, same behavior a before. I also fixed naming issue between snake_case and dash-case

jpradelle avatar Jul 30 '24 08:07 jpradelle

i understand better now

currently (in this PR), we have two behaviours:

  • default - just check that the attribute is lowercase or there isn't one, and prop is lower case
  • none - check that the attribute is the lowercase of the prop name
  • snake - check that the attribute is the snake_case of the prop name
  • kebab - check that the attribute is the kebab-case of the prop name

the confusion here is that default differs from everything else, including none

maybe we should rename style to convention and have this behaviour:

  • no convention does the current behaviour in main (check the attr is lower case)
  • all others compare the attribute and the prop to enforce a convention (which implicitly will also enforce that it is lower case)
  • none still exists but is the default (i.e. no convention, so just remove the default in the switch because it should be impossible to reach)

43081j avatar Jul 30 '24 16:07 43081j

I updated with convention instead of style

But I don't fully get this part :

maybe we should rename style to convention and have this behaviour:

  • no convention does the current behaviour in main (check the attr is lower case)
  • all others compare the attribute and the prop to enforce a convention (which implicitly will also enforce that it is lower case)
  • none still exists but is the default (i.e. no convention, so just remove the default in the switch because it should be impossible to reach)

It's ok for not changing current rule behavior if convention is not defined.

But I don't see how we can have a default value for convention. Either it's not specified and we keep current rule behavior or we define a convention, and to define it, it must have a value, we can not have default value for it. Check for allowed values is done here : https://github.com/43081j/eslint-plugin-lit/pull/207/files#diff-0eff54e47634b63f2db864b99a12f005fd784612af3d7b64893fbf03ae1f8bd3R25

In that case, none is maybe not very meaningful, what about renaming it something like lowercase or lower

jpradelle avatar Aug 01 '24 15:08 jpradelle

i pushed a few changes, maybe that'll help you understand

having none and a default (null) doesn't make much sense. the default is none, in that we don't enforce a convention

what i was trying to point out was that these two things are separate:

  • enforcing that attributes are lowercase
  • enforcing a naming convention of attributes

all attributes, regardless of convention, need to be lowercase. in the commit i pushed, i've moved the code out of the condition so it always runs.

separately, if the attribute was lowercase, we then enforce a particular naming convention if one is set

43081j avatar Aug 02 '24 09:08 43081j

I pushed a small documentation update, are you requesting more updates on this PR ?

jpradelle avatar Aug 29 '24 11:08 jpradelle

Looks good to me!

Thanks a lot for seeing this through 🙏

43081j avatar Aug 29 '24 14:08 43081j