Skip to content

🏗 Create eslint rule for preferred Preact props#35722

Merged
caroqliu merged 11 commits intoampproject:mainfrom
caroqliu:preact-eslint
Aug 23, 2021
Merged

🏗 Create eslint rule for preferred Preact props#35722
caroqliu merged 11 commits intoampproject:mainfrom
caroqliu:preact-eslint

Conversation

@caroqliu
Copy link
Copy Markdown
Contributor

@caroqliu caroqliu commented Aug 18, 2021

This PR introduces preact-preferred-props which enforces two things on files written in src/preact, Bento components, and Storybooks:

  • Specific attributes given to JSX nodes should conform to expect styles.
  • Declarations from destructuring in function params (all) and variable declarations (exclusive to objects named props) should access keys of expected styles.
    • Does not fix: props listed in component.type.js

In particular, both of the above expect class over className, tabIndex over tabindex, and kebab-cased SVG attributes. Of these, tabIndex is the only one that is a preferred style in common by both the preact and react libraries.

In the long term we may consider breaking out tabIndex into a common-preferred-props rule that deals with properties both libraries will approve of. This may include aria attributes and others from possibleStandardNames.js.

Partial for #35678

/to @developit

@caroqliu caroqliu marked this pull request as ready for review August 20, 2021 18:03
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Aug 20, 2021

Hey @erwinmombay, @jridgewell! These files were changed:

build-system/eslint-rules/preact-preferred-props.js

@caroqliu caroqliu requested a review from jridgewell August 20, 2021 18:03
@caroqliu caroqliu requested a review from alanorozco August 20, 2021 18:51
Copy link
Copy Markdown

@developit developit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Internally Preact doesn't care about class vs className, the only important thing is that components use them consistently (so that destructured class doesn't get overwritten by rest-collected className, etc). I would guess that class is preferable there though because of the proximity to the Web Component wrappers 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants