airbnb / eslint-plugin-react-with-styles

ESLint plugin for react-with-styles
MIT License
49 stars 22 forks source link

Rule proposal: no dynamic style references #14

Open lencioni opened 7 years ago

lencioni commented 7 years ago

Dynamic style references make code more difficult to understand and interfere with static analycis. This weakens rules such as no-unused-styles. It would be good to have a rule that prevents this kind of coding.

Bad:

styles[foo]
styles['foo' + bar]
styles[`foo${bar}`]

I think it would also be okay to disallow the following:

styles['foo']

Good:

styles.foo
ljharb commented 7 years ago

What about things that aren't valid identifiers?

I'd think that a string literal, or a template literal with no interpolations, should be permitted in bracket notation.

lencioni commented 7 years ago

What about things that aren't valid identifiers?

Can you give me an example?

I'd think that a string literal, or a template literal with no interpolations, should be permitted in bracket notation.

Sure, I don't really care much either way about this.

ljharb commented 7 years ago

':first-child'

lencioni commented 7 years ago

Why is that not a valid identifier?

ljharb commented 7 years ago

Because it starts with a colon and contains a hyphen

lencioni commented 7 years ago

The hyphen should be fine, and the colon is potentially fine--that all depends on the underlying implementation. In our case with Aphrodite, this isn't handled very well and results in a className that contains a colon.

But, it seems like these cases should be better handled by separate rules that dictate the shape of styles properties and not a rule that dictates the type of styles properties.

ljharb commented 7 years ago

I'm not sure I'm being clear - you can't do styles.foo-bar, you have to do styles['foo-bar'] - it seems like an eslint plugin shouldn't try to be too opinionated about dot vs bracket access, nor about the casing of the property names of the styles object.