-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
chore: enable jsx-a11y, react, and react-hooks ESLint plugins internally #4197
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: enable jsx-a11y, react, and react-hooks ESLint plugins internally #4197
Conversation
Thanks for the PR, @JoshuaKGoldberg! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
✔️ Deploy Preview for typescript-eslint ready! 🔨 Explore the source changes: 34a2aee 🔍 Inspect the deploy log: https://app.netlify.com/sites/typescript-eslint/deploys/61a12e403155a70007c5cec5 😎 Browse the preview: https://deploy-preview-4197--typescript-eslint.netlify.app |
Codecov Report
@@ Coverage Diff @@
## main #4197 +/- ##
==========================================
- Coverage 92.76% 92.73% -0.03%
==========================================
Files 332 335 +3
Lines 11524 11541 +17
Branches 3285 3288 +3
==========================================
+ Hits 10690 10703 +13
- Misses 361 364 +3
- Partials 473 474 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Fun, we're starting to run out of memory in CI: $ eslint . --ext .js,.jsx,.ts,.tsx
FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory I can confirm locally errors do get logged for the website with the introduced plugins: C:\Code\win\typescript-eslint\packages\website\src\pages\index.tsx
113:5 error React Hook "useEffect" is called conditionally. React Hooks must be called in the exact same order in every component render
react-hooks/rules-of-hooks
115:9 error Unexpected console statement
no-console |
Fixed in https://github.com/typescript-eslint/typescript-eslint/runs/4279473298?check_suite_focus=true by upping Node's memory allowance. |
q: maybe its a good time to split up linting and updating this command to?
|
@@ -126,8 +126,6 @@ function Sponsors(props: { | |||
title: string; | |||
className?: string; | |||
}): JSX.Element { | |||
// TODO this seems like a ts-eslint problem: JSON types are not resolved | |||
/* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is... interesting. Didn't test it locally, but is that basically just because we used a local ESLint config instead of the global one? Is this actually a valid bug though that we can't resolve JSON types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I haven't dug in too deep. It's better now 🤷
It would be nice to know but I don't quite have the time to poke too closely, heh.
The "proper" fix would be to add a If we had a better CLI - we could split it up automatically #4183 😛 Increasing memory limit is a good way to work around this for now. |
I've got a lot of things in the air this week so I'll just merge this for now & file a followup issue for the lint splitups. |
automerge unfortunately requires all signals pass AND the branch is up to date it's a really finicky feature tbh. |
PR Checklist
[ ] Addresses an existing issue: fixes #000for chore(website): add playground to website #4108Overview
Adds some linting rules that'll help catch things with the more advanced website contents.
Also fixes #4200 by necessity.