-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add Nav block files to those triggering error for exhaustive deps #48821
Add Nav block files to those triggering error for exhaustive deps #48821
Conversation
Size Change: +20 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
Flaky tests detected in fcfcff8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4352425804
|
Seeking a confidence check here that this only enables for Nav block files. I think this is a pattern we could employ until we have eliminated all |
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.
Makes sense to me 🚀
// Components package. | ||
'packages/components/src/**/*.[tj]s?(x)', | ||
// Navigaiton block. | ||
'packages/block-library/src/navigation/**/*.[tj]s?(x)', |
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.
👍 I think it's a good call to expand this list as we work through addressing the existing warnings and aim to not introduce new ones.
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
What?
With the merge of #24914 and #48680 this PR ups the level of the exhaustive deps rule to
error
for the Navigation block.Why?
There have been far too many bugs caused by humans missing deps. This rule will force avoiding any future regressions.
How?
Add the Nav block files to the exceptions setting exhaustive deps eslint rule to
error
as opposed to the defaultwarning
.Testing Instructions
useEffect
in the Nav blockindex.js
file and remove some of the dependencies from the deps array.error
status (not justwarning
).Testing Instructions for Keyboard
Screenshots or screencast