Add the default eslint react hooks config to the wordpress eslint package#14995
Add the default eslint react hooks config to the wordpress eslint package#14995youknowriad merged 2 commits intomasterfrom
Conversation
|
@ntwb I already have a CHANGELOG entry |
| import { Notice } from '@wordpress/components'; | ||
| import { useEffect } from '@wordpress/element'; | ||
|
|
||
| function ContrastCheckerMessage( { tinyBackgroundColor, tinyTextColor, backgroundColor, textColor } ) { |
There was a problem hiding this comment.
Out of curiosity, what ESLint rule was triggered which enforced this refactor? I'm wondering what is the lesson learned.
There was a problem hiding this comment.
The reason is that you can't use hooks after "early returns"
| 'react/no-children-prop': 'off', | ||
| 'react/prop-types': 'off', | ||
| 'react/react-in-jsx-scope': 'off', | ||
| 'react-hooks/rules-of-hooks': 'error', |
There was a problem hiding this comment.
Interesting, they don't provide the recommended preset.
| 'react/prop-types': 'off', | ||
| 'react/react-in-jsx-scope': 'off', | ||
| 'react-hooks/rules-of-hooks': 'error', | ||
| 'react-hooks/exhaustive-deps': 'warn', |
There was a problem hiding this comment.
I'm hesitant on the need of this one as we already have a valid use-case for extra deps, but I'm merging and we can try and see if it feels constraining.
| "_originalInput": "#666", | ||
| "_r": 102, | ||
| "_roundA": 1, | ||
| "_tc_id": 2, |
There was a problem hiding this comment.
These IDs increment with each use of tinycolor2, thus can be affected externally and fail the tests.
See: 4c6a9a4 / #18681
Do we have to pass around the TinyColor object? Should we avoid snapshots?
There was a problem hiding this comment.
yes, these snapshots are not that useful I think.
closes #14676
This PR adds the default eslint config for react hooks (See https://reactjs.org/docs/hooks-rules.html) to the WordPress eslint config.
URLPopoverAtLinkcomponent. It's our use-case is legitimate though so I was wondering if we should remove the second rule (the warning) added here. cc @aduth