ESLint: Stop disabling react-hooks/exhaustive-deps rule (WordPress#…

…66324) * ESLint: Stop disabling react-hooks/exhaustive-deps rule * CHANGELOG * Add ref to dependencies * Retain some context in comments Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
karthick-murugan · Nov 13, 2024 · 9a876cd · 9a876cd
1 parent b2e9eba
commit 9a876cd
Show file tree

Hide file tree

Showing 35 changed files with 55 additions and 146 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -356,18 +356,6 @@ module.exports = {
 				],
 			},
 		},
-		{
-			files: [
-				// Components package.
-				'packages/components/src/**/*.[tj]s?(x)',
-				// Navigation block.
-				'packages/block-library/src/navigation/**/*.[tj]s?(x)',
-			],
-			excludedFiles: [ ...developmentFiles ],
-			rules: {
-				'react-hooks/exhaustive-deps': 'error',
-			},
-		},
 		{
 			files: [ 'packages/jest*/**/*.js', '**/test/**/*.js' ],
 			excludedFiles: [ 'test/e2e/**/*.js', 'test/performance/**/*.js' ],

diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -21,6 +21,10 @@
 -   `Tabs` and `TabPanel`: Fix arrow key navigation in RTL ([#66201](https://github.com/WordPress/gutenberg/pull/66201)).
 -   `Tabs`: override tablist's tabindex only when necessary ([#66209](https://github.com/WordPress/gutenberg/pull/66209)).
 
+### Internal
+
+-   ESLint: Stop disabling `react-hooks/exhaustive-deps` rule ([#66324](https://github.com/WordPress/gutenberg/pull/66324)).
+
 ## 28.10.0 (2024-10-16)
 
 ### Bug Fixes

diff --git a/packages/components/src/autocomplete/autocompleter-ui.native.js b/packages/components/src/autocomplete/autocompleter-ui.native.js
@@ -69,9 +69,8 @@ export function getAutoCompleterUI( autocompleter ) {
 			} else if ( isVisible && text.length === 0 ) {
 				startAnimation( false );
 			}
-			// Temporarily disabling exhaustive-deps to avoid introducing unexpected side effecst.
+			// We want to avoid introducing unexpected side effects.
 			// See https://github.com/WordPress/gutenberg/pull/41820
-			// eslint-disable-next-line react-hooks/exhaustive-deps
 		}, [ items, isVisible, text ] );
 
 		const activeItemStyles = usePreferredColorSchemeStyle(
@@ -112,9 +111,8 @@ export function getAutoCompleterUI( autocompleter ) {
 					}
 				} );
 			},
-			// Temporarily disabling exhaustive-deps to avoid introducing unexpected side effecst.
+			// We want to avoid introducing unexpected side effects.
 			// See https://github.com/WordPress/gutenberg/pull/41820
-			// eslint-disable-next-line react-hooks/exhaustive-deps
 			[ isVisible ]
 		);
 

diff --git a/packages/components/src/autocomplete/autocompleter-ui.tsx b/packages/components/src/autocomplete/autocompleter-ui.tsx
@@ -165,9 +165,8 @@ export function getAutoCompleterUI( autocompleter: WPCompleter ) {
 		useLayoutEffect( () => {
 			onChangeOptions( items );
 			announce( items );
-			// Temporarily disabling exhaustive-deps to avoid introducing unexpected side effecst.
+			// We want to avoid introducing unexpected side effects.
 			// See https://github.com/WordPress/gutenberg/pull/41820
-			// eslint-disable-next-line react-hooks/exhaustive-deps
 		}, [ items ] );
 
 		if ( items.length === 0 ) {
@@ -235,8 +234,5 @@ function useOnClickOutside(
 			document.removeEventListener( 'mousedown', listener );
 			document.removeEventListener( 'touchstart', listener );
 		};
-		// Disable reason: `ref` is a ref object and should not be included in a
-		// hook's dependency list.
-		// eslint-disable-next-line react-hooks/exhaustive-deps
-	}, [ handler ] );
+	}, [ handler, ref ] );
 }
diff --git a/packages/components/src/autocomplete/index.tsx b/packages/components/src/autocomplete/index.tsx
@@ -380,9 +380,8 @@ export function useAutocomplete( {
 				: AutocompleterUI
 		);
 		setFilterValue( query === null ? '' : query );
-		// Temporarily disabling exhaustive-deps to avoid introducing unexpected side effecst.
+		// We want to avoid introducing unexpected side effects.
 		// See https://github.com/WordPress/gutenberg/pull/41820
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ textContent ] );
 
 	const { key: selectedKey = '' } = filteredOptions[ selectedIndex ] || {};

diff --git a/packages/components/src/color-palette/index.native.js b/packages/components/src/color-palette/index.native.js
@@ -115,9 +115,8 @@ function ColorPalette( {
 				scrollViewRef.current.scrollTo( { x: 0, y: 0 } );
 			}
 		}
-		// Temporarily disabling exhuastive-deps until the component can be refactored and updated safely.
+		// Not adding additional dependencies until the component can be refactored and updated safely.
 		// Please see https://github.com/WordPress/gutenberg/pull/41253 for discussion and details.
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ currentSegment ] );
 
 	function isSelectedCustom() {

diff --git a/packages/components/src/color-picker/index.native.js b/packages/components/src/color-picker/index.native.js
@@ -111,7 +111,6 @@ function ColorPicker( {
 		// the hook’s dependencies and running it a single time. Ideally there
 		// may be a way to refactor and obviate the disabled lint rule. If not,
 		// this comment should be replaced by one that explains the reasoning.
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [] );
 
 	function onButtonPress( action ) {

diff --git a/packages/components/src/date-time/date/index.tsx b/packages/components/src/date-time/date/index.tsx
@@ -302,7 +302,6 @@ function Day( {
 		}
 		// isFocusAllowed is not a dep as there is no point calling focus() on
 		// an already focused element.
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ isFocusable ] );
 
 	return (

diff --git a/packages/components/src/form-token-field/index.tsx b/packages/components/src/form-token-field/index.tsx
@@ -124,17 +124,14 @@ export function FormTokenField( props: FormTokenFieldProps ) {
 		}
 
 		// TODO: updateSuggestions() should first be refactored so its actual deps are clearer.
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ suggestions, prevSuggestions, value, prevValue ] );
 
 	useEffect( () => {
 		updateSuggestions();
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ incompleteTokenValue ] );
 
 	useEffect( () => {
 		updateSuggestions();
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ __experimentalAutoSelectFirstMatch ] );
 
 	if ( disabled && isActive ) {

diff --git a/...es/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js b/...es/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js
@@ -71,9 +71,9 @@ const BottomSheetNavigationScreen = ( {
 			 * callbacks triggered based upon which screen is currently active.
 			 *
 			 * Related: https://github.com/WordPress/gutenberg/pull/36328#discussion_r768897546
+			 *
+			 * Also see https://github.com/WordPress/gutenberg/pull/41166.
 			 */
-			// see https://github.com/WordPress/gutenberg/pull/41166
-			// eslint-disable-next-line react-hooks/exhaustive-deps
 		}, [] )
 	);
 
@@ -130,9 +130,7 @@ const BottomSheetNavigationScreen = ( {
 				</TouchableHighlight>
 			</ScrollView>
 		);
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [
 		children,
 		isFocused,

diff --git a/packages/components/src/mobile/bottom-sheet/sub-sheet/index.native.js b/packages/components/src/mobile/bottom-sheet/sub-sheet/index.native.js
@@ -28,9 +28,7 @@ const BottomSheetSubSheet = ( {
 		if ( showSheet ) {
 			setIsFullScreen( isFullScreen );
 		}
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ showSheet, isFullScreen ] );
 
 	return (

diff --git a/packages/components/src/mobile/color-settings/index.native.js b/packages/components/src/mobile/color-settings/index.native.js
@@ -35,9 +35,7 @@ const ColorSettingsMemo = memo(
 		useEffect( () => {
 			shouldEnableBottomSheetMaxHeight( true );
 			onHandleClosingBottomSheet( null );
-			// Disable reason: deferring this refactor to the native team.
-			// see https://github.com/WordPress/gutenberg/pull/41166
-			// eslint-disable-next-line react-hooks/exhaustive-deps
+			// See https://github.com/WordPress/gutenberg/pull/41166
 		}, [] );
 		return (
 			<BottomSheet.NavigationContainer>

diff --git a/packages/components/src/mobile/color-settings/picker-screen.native.js b/packages/components/src/mobile/color-settings/picker-screen.native.js
@@ -43,9 +43,7 @@ const PickerScreen = () => {
 				onHandleHardwareButtonPress={ onHandleHardwareButtonPress }
 			/>
 		);
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [
 		setColor,
 		currentValue,

diff --git a/packages/components/src/mobile/image/index.native.js b/packages/components/src/mobile/image/index.native.js
@@ -112,9 +112,7 @@ const ImageComponent = ( {
 			}
 		}
 		return () => ( isCurrent = false );
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ url ] );
 
 	const onContainerLayout = ( event ) => {

diff --git a/packages/components/src/mobile/keyboard-avoiding-view/index.ios.js b/packages/components/src/mobile/keyboard-avoiding-view/index.ios.js
@@ -69,9 +69,7 @@ export const KeyboardAvoidingView = ( {
 			keyboardShowSubscription.remove();
 			keyboardHideSubscription.remove();
 		};
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [] );
 
 	function onSafeAreaInsetsUpdate( { safeAreaInsets } ) {

diff --git a/packages/components/src/mobile/link-picker/link-picker-results.native.js b/packages/components/src/mobile/link-picker/link-picker-results.native.js
@@ -75,9 +75,8 @@ export default function LinkPickerResults( {
 		return {
 			fetchMoreSuggestions: debounce( fetchMore, REQUEST_DEBOUNCE_DELAY ),
 		};
-		// Disable eslint rule for now, to avoid introducing a regression
+		// Not adding dependencies for now, to avoid introducing a regression
 		// (see https://github.com/WordPress/gutenberg/pull/23922#discussion_r1170634879).
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [] );
 
 	// Prevent setting state when unmounted.
@@ -90,9 +89,7 @@ export default function LinkPickerResults( {
 		setHasAllSuggestions( false );
 		setLinks( [ directEntry ] );
 		fetchMoreSuggestions( { query, links: [ directEntry ] } );
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ query ] );
 
 	const onEndReached = () => fetchMoreSuggestions( { query, links } );

diff --git a/packages/components/src/mobile/link-picker/link-picker-screen.native.js b/packages/components/src/mobile/link-picker/link-picker-screen.native.js
@@ -53,9 +53,7 @@ const LinkPickerScreen = ( { returnScreenName } ) => {
 				onCancel={ onCancel }
 			/>
 		);
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ inputValue ] );
 };
 

diff --git a/packages/components/src/mobile/link-settings/index.native.js b/packages/components/src/mobile/link-settings/index.native.js
@@ -101,9 +101,7 @@ function LinkSettings( {
 		if ( onHandleClosingBottomSheet ) {
 			onHandleClosingBottomSheet( onCloseSettingsSheet );
 		}
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ urlInputValue, labelInputValue, linkRelInputValue ] );
 
 	useEffect( () => {
@@ -115,9 +113,7 @@ function LinkSettings( {
 		if ( url !== urlInputValue ) {
 			setUrlInputValue( url || '' );
 		}
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ url ] );
 
 	useEffect( () => {
@@ -141,9 +137,7 @@ function LinkSettings( {
 		if ( prevEditorSidebarOpened && ! editorSidebarOpened ) {
 			onSetAttributes();
 		}
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ editorSidebarOpened, isVisible ] );
 
 	useEffect( () => {
@@ -156,9 +150,7 @@ function LinkSettings( {
 				url: prependHTTP( urlValue ),
 			} );
 		}
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ urlValue ] );
 
 	const onChangeURL = useCallback(
@@ -188,9 +180,7 @@ function LinkSettings( {
 				rel: linkRelInputValue,
 			} );
 		}
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ urlInputValue, labelInputValue, linkRelInputValue, setAttributes ] );
 
 	const onCloseSettingsSheet = useCallback( () => {
@@ -223,9 +213,7 @@ function LinkSettings( {
 				rel: updatedRel,
 			} );
 		},
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 		[ linkRelInputValue ]
 	);
 

diff --git a/packages/components/src/mobile/link-settings/link-settings-screen.native.js b/packages/components/src/mobile/link-settings/link-settings-screen.native.js
@@ -37,9 +37,7 @@ const LinkSettingsScreen = ( props ) => {
 				urlValue={ inputValue }
 			/>
 		);
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ props, inputValue, navigation, route ] );
 };
 

diff --git a/packages/components/src/mobile/segmented-control/index.native.js b/packages/components/src/mobile/segmented-control/index.native.js
@@ -74,18 +74,14 @@ const SegmentedControls = ( {
 	useEffect( () => {
 		setActiveSegmentIndex( selectedSegmentIndex );
 		segmentHandler( segments[ selectedSegmentIndex ] );
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [] );
 
 	useEffect( () => {
 		positionAnimationValue.setValue(
 			calculateEndValue( activeSegmentIndex )
 		);
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ segmentsDimensions ] );
 
 	const containerStyle = usePreferredColorSchemeStyle(

diff --git a/packages/components/src/mobile/utils/use-unit-converter-to-mobile.native.js b/packages/components/src/mobile/utils/use-unit-converter-to-mobile.native.js
@@ -66,9 +66,7 @@ const useConvertUnitToMobile = ( value, unit, styles ) => {
 		return () => {
 			dimensionsChangeSubscription.remove();
 		};
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [] );
 
 	const onDimensionsChange = useCallback( ( { window } ) => {
@@ -85,9 +83,7 @@ const useConvertUnitToMobile = ( value, unit, styles ) => {
 			valueToConvert,
 			valueUnit
 		);
-		// Disable reason: deferring this refactor to the native team.
-		// see https://github.com/WordPress/gutenberg/pull/41166
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// See https://github.com/WordPress/gutenberg/pull/41166
 	}, [ windowSizes, value, unit ] );
 };
 

diff --git a/packages/components/src/navigation/index.tsx b/packages/components/src/navigation/index.tsx
@@ -104,9 +104,8 @@ export function Navigation( {
 		if ( activeMenu !== menu ) {
 			setActiveMenu( activeMenu );
 		}
-		// Ignore exhaustive-deps here, as it would require either a larger refactor or some questionable workarounds.
+		// Not adding deps for now, as it would require either a larger refactor or some questionable workarounds.
 		// See https://github.com/WordPress/gutenberg/pull/41612 for context.
-		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ activeMenu ] );
 
 	const context = {

diff --git a/packages/components/src/navigation/item/use-navigation-tree-item.tsx b/packages/components/src/navigation/item/use-navigation-tree-item.tsx
@@ -41,7 +41,7 @@ export const useNavigationTreeItem = (
 		return () => {
 			removeItem( itemId );
 		};
-		// Ignore exhaustive-deps rule for now. See https://github.com/WordPress/gutenberg/pull/41639
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// Not adding deps for now, as it would require either a larger refactor.
+		// See https://github.com/WordPress/gutenberg/pull/41639
 	}, [ activeMenu, search ] );
 };
diff --git a/packages/components/src/navigation/menu/menu-title-search.tsx b/packages/components/src/navigation/menu/menu-title-search.tsx
@@ -55,8 +55,8 @@ function MenuTitleSearch( {
 			count
 		);
 		debouncedSpeak( resultsFoundMessage );
-		// Ignore exhaustive-deps rule for now. See https://github.com/WordPress/gutenberg/pull/44090
-		// eslint-disable-next-line react-hooks/exhaustive-deps
+		// Not adding deps for now, as it would require either a larger refactor.
+		// See https://github.com/WordPress/gutenberg/pull/44090
 	}, [ items, search ] );
 
 	const onClose = () => {