Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(lint): update eslint rules #645

Merged
merged 3 commits into from
Feb 21, 2020
Merged

Conversation

austingreendev
Copy link
Contributor

@austingreendev austingreendev commented Feb 20, 2020

Description

This PR is an audit of our existing ESLint rules. The only change is that we now require displayNames for all React components. This was disabled previously, but should be best practice for us going forward.

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 🌐 Styleguidist demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • ♿ analyzed via axe and evaluated using VoiceOver
  • 💂‍♂️ includes new unit tests
  • 📝 tested in Chrome, Firefox, Safari, Edge, and IE11

@coveralls
Copy link

coveralls commented Feb 20, 2020

Coverage Status

Coverage increased (+0.08%) to 93.977% when pulling add6771 on agreen/eslint-alignment into f8f7068 on master.

Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

Looks like there should be a new require-display-name custom rule added for eslint under /utils.

Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

Please add displayName to the .template package.

@austingreendev
Copy link
Contributor Author

Looks like there should be a new require-display-name custom rule added for eslint under /utils.

There is a lot of complexity in determining whether a displayName is valid. https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/display-name.md only missed 2 components due to our unique createPortal usage. We might want to stick with the existing solution to avoid complications as React introduces new APIs as well.

Please add displayName to the .template package.

Updated! 👍

@austingreendev austingreendev merged commit 03122a3 into master Feb 21, 2020
@austingreendev austingreendev deleted the agreen/eslint-alignment branch February 21, 2020 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants