From 544528bed02fceff3d1ab01e6fbd5cb8d41a94f6 Mon Sep 17 00:00:00 2001 From: Marin Atanasov <8436925+tyxla@users.noreply.github.com> Date: Thu, 24 Oct 2024 15:27:33 +0300 Subject: [PATCH] Retain some context in comments --- .../components/src/autocomplete/autocompleter-ui.native.js | 4 ++++ packages/components/src/autocomplete/autocompleter-ui.tsx | 2 ++ packages/components/src/autocomplete/index.tsx | 2 ++ packages/components/src/color-palette/index.native.js | 2 ++ packages/components/src/color-picker/index.native.js | 4 ++++ .../bottom-sheet-navigation/navigation-screen.native.js | 3 +++ .../src/mobile/bottom-sheet/sub-sheet/index.native.js | 1 + .../components/src/mobile/color-settings/index.native.js | 1 + .../src/mobile/color-settings/picker-screen.native.js | 1 + packages/components/src/mobile/image/index.native.js | 1 + .../src/mobile/keyboard-avoiding-view/index.ios.js | 1 + .../src/mobile/link-picker/link-picker-results.native.js | 3 +++ .../src/mobile/link-picker/link-picker-screen.native.js | 1 + .../components/src/mobile/link-settings/index.native.js | 6 ++++++ .../src/mobile/link-settings/link-settings-screen.native.js | 1 + .../components/src/mobile/segmented-control/index.native.js | 2 ++ .../src/mobile/utils/use-unit-converter-to-mobile.native.js | 2 ++ packages/components/src/navigation/index.tsx | 2 +- .../src/navigation/item/use-navigation-tree-item.tsx | 3 ++- .../components/src/navigation/menu/menu-title-search.tsx | 3 ++- .../src/navigation/menu/use-navigation-tree-menu.tsx | 3 ++- packages/components/src/sandbox/index.native.js | 1 + packages/components/src/sandbox/index.tsx | 6 ++++++ packages/components/src/search-control/index.native.js | 1 + packages/components/src/tooltip/index.native.js | 5 +++++ packages/components/src/unit-control/index.native.js | 4 ++++ 26 files changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/components/src/autocomplete/autocompleter-ui.native.js b/packages/components/src/autocomplete/autocompleter-ui.native.js index aa979f6b068ea5..daf4fe301abf77 100644 --- a/packages/components/src/autocomplete/autocompleter-ui.native.js +++ b/packages/components/src/autocomplete/autocompleter-ui.native.js @@ -69,6 +69,8 @@ export function getAutoCompleterUI( autocompleter ) { } else if ( isVisible && text.length === 0 ) { startAnimation( false ); } + // We want to avoid introducing unexpected side effects. + // See https://github.com/WordPress/gutenberg/pull/41820 }, [ items, isVisible, text ] ); const activeItemStyles = usePreferredColorSchemeStyle( @@ -109,6 +111,8 @@ export function getAutoCompleterUI( autocompleter ) { } } ); }, + // We want to avoid introducing unexpected side effects. + // See https://github.com/WordPress/gutenberg/pull/41820 [ isVisible ] ); diff --git a/packages/components/src/autocomplete/autocompleter-ui.tsx b/packages/components/src/autocomplete/autocompleter-ui.tsx index fa38302d40009c..69105f6c9d3b44 100644 --- a/packages/components/src/autocomplete/autocompleter-ui.tsx +++ b/packages/components/src/autocomplete/autocompleter-ui.tsx @@ -165,6 +165,8 @@ export function getAutoCompleterUI( autocompleter: WPCompleter ) { useLayoutEffect( () => { onChangeOptions( items ); announce( items ); + // We want to avoid introducing unexpected side effects. + // See https://github.com/WordPress/gutenberg/pull/41820 }, [ items ] ); if ( items.length === 0 ) { diff --git a/packages/components/src/autocomplete/index.tsx b/packages/components/src/autocomplete/index.tsx index dd2dd0d2fe1491..f3278a46015c3c 100644 --- a/packages/components/src/autocomplete/index.tsx +++ b/packages/components/src/autocomplete/index.tsx @@ -380,6 +380,8 @@ export function useAutocomplete( { : AutocompleterUI ); setFilterValue( query === null ? '' : query ); + // We want to avoid introducing unexpected side effects. + // See https://github.com/WordPress/gutenberg/pull/41820 }, [ 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 index a34d05d294fd86..ab86cd6f05f127 100644 --- a/packages/components/src/color-palette/index.native.js +++ b/packages/components/src/color-palette/index.native.js @@ -115,6 +115,8 @@ function ColorPalette( { scrollViewRef.current.scrollTo( { x: 0, y: 0 } ); } } + // 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. }, [ currentSegment ] ); function isSelectedCustom() { diff --git a/packages/components/src/color-picker/index.native.js b/packages/components/src/color-picker/index.native.js index 8c0b9b6c9e7725..302de6b7d79b89 100644 --- a/packages/components/src/color-picker/index.native.js +++ b/packages/components/src/color-picker/index.native.js @@ -107,6 +107,10 @@ function ColorPicker( { if ( onHandleHardwareButtonPress ) { onHandleHardwareButtonPress( onButtonPress ); } + // TODO: Revisit this to discover if there's a good reason for omitting + // 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. }, [] ); function onButtonPress( action ) { diff --git a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js index 133ff27b28049b..8c43bc85612762 100644 --- a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js +++ b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js @@ -71,6 +71,8 @@ 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. */ }, [] ) ); @@ -128,6 +130,7 @@ const BottomSheetNavigationScreen = ( { ); + // 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 index cfc72aecbc5bbe..14040be0cc239e 100644 --- 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,6 +28,7 @@ const BottomSheetSubSheet = ( { if ( showSheet ) { setIsFullScreen( isFullScreen ); } + // 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 index 27d8119a38c4af..c7956df0e935dd 100644 --- a/packages/components/src/mobile/color-settings/index.native.js +++ b/packages/components/src/mobile/color-settings/index.native.js @@ -35,6 +35,7 @@ const ColorSettingsMemo = memo( useEffect( () => { shouldEnableBottomSheetMaxHeight( true ); onHandleClosingBottomSheet( null ); + // See https://github.com/WordPress/gutenberg/pull/41166 }, [] ); return ( diff --git a/packages/components/src/mobile/color-settings/picker-screen.native.js b/packages/components/src/mobile/color-settings/picker-screen.native.js index ebaecb2fa0a6f1..e924580d939794 100644 --- a/packages/components/src/mobile/color-settings/picker-screen.native.js +++ b/packages/components/src/mobile/color-settings/picker-screen.native.js @@ -43,6 +43,7 @@ const PickerScreen = () => { onHandleHardwareButtonPress={ onHandleHardwareButtonPress } /> ); + // 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 index 3e0a993e9fbea8..3c7e4ddc19c006 100644 --- a/packages/components/src/mobile/image/index.native.js +++ b/packages/components/src/mobile/image/index.native.js @@ -112,6 +112,7 @@ const ImageComponent = ( { } } return () => ( isCurrent = false ); + // 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 index 6fd716a527ee3b..7630fd21fbc429 100644 --- a/packages/components/src/mobile/keyboard-avoiding-view/index.ios.js +++ b/packages/components/src/mobile/keyboard-avoiding-view/index.ios.js @@ -69,6 +69,7 @@ export const KeyboardAvoidingView = ( { keyboardShowSubscription.remove(); keyboardHideSubscription.remove(); }; + // 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 index 803635b5644228..4751ebc1ee2111 100644 --- 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,6 +75,8 @@ export default function LinkPickerResults( { return { fetchMoreSuggestions: debounce( fetchMore, REQUEST_DEBOUNCE_DELAY ), }; + // Not adding dependencies for now, to avoid introducing a regression + // (see https://github.com/WordPress/gutenberg/pull/23922#discussion_r1170634879). }, [] ); // Prevent setting state when unmounted. @@ -87,6 +89,7 @@ export default function LinkPickerResults( { setHasAllSuggestions( false ); setLinks( [ directEntry ] ); fetchMoreSuggestions( { query, links: [ directEntry ] } ); + // 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 index f77cf4e18a1b07..999c9053c59862 100644 --- 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,6 +53,7 @@ const LinkPickerScreen = ( { returnScreenName } ) => { onCancel={ onCancel } /> ); + // 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 index 794650adefdd79..a01e33b410a5c0 100644 --- a/packages/components/src/mobile/link-settings/index.native.js +++ b/packages/components/src/mobile/link-settings/index.native.js @@ -101,6 +101,7 @@ function LinkSettings( { if ( onHandleClosingBottomSheet ) { onHandleClosingBottomSheet( onCloseSettingsSheet ); } + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ urlInputValue, labelInputValue, linkRelInputValue ] ); useEffect( () => { @@ -112,6 +113,7 @@ function LinkSettings( { if ( url !== urlInputValue ) { setUrlInputValue( url || '' ); } + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ url ] ); useEffect( () => { @@ -135,6 +137,7 @@ function LinkSettings( { if ( prevEditorSidebarOpened && ! editorSidebarOpened ) { onSetAttributes(); } + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ editorSidebarOpened, isVisible ] ); useEffect( () => { @@ -147,6 +150,7 @@ function LinkSettings( { url: prependHTTP( urlValue ), } ); } + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ urlValue ] ); const onChangeURL = useCallback( @@ -176,6 +180,7 @@ function LinkSettings( { rel: linkRelInputValue, } ); } + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ urlInputValue, labelInputValue, linkRelInputValue, setAttributes ] ); const onCloseSettingsSheet = useCallback( () => { @@ -208,6 +213,7 @@ function LinkSettings( { rel: updatedRel, } ); }, + // 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 index 65c95187d2fcb8..7147becd67b22d 100644 --- 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,6 +37,7 @@ const LinkSettingsScreen = ( props ) => { urlValue={ inputValue } /> ); + // 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 index 3bd7ffb319573b..36b92bb0d4ea32 100644 --- a/packages/components/src/mobile/segmented-control/index.native.js +++ b/packages/components/src/mobile/segmented-control/index.native.js @@ -74,12 +74,14 @@ const SegmentedControls = ( { useEffect( () => { setActiveSegmentIndex( selectedSegmentIndex ); segmentHandler( segments[ selectedSegmentIndex ] ); + // See https://github.com/WordPress/gutenberg/pull/41166 }, [] ); useEffect( () => { positionAnimationValue.setValue( calculateEndValue( activeSegmentIndex ) ); + // 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 index a312f59db332fd..0b1dfd31147dee 100644 --- 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,6 +66,7 @@ const useConvertUnitToMobile = ( value, unit, styles ) => { return () => { dimensionsChangeSubscription.remove(); }; + // See https://github.com/WordPress/gutenberg/pull/41166 }, [] ); const onDimensionsChange = useCallback( ( { window } ) => { @@ -82,6 +83,7 @@ const useConvertUnitToMobile = ( value, unit, styles ) => { valueToConvert, valueUnit ); + // 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 index 18b1c3619e41c5..92f431dfb22fc7 100644 --- a/packages/components/src/navigation/index.tsx +++ b/packages/components/src/navigation/index.tsx @@ -104,7 +104,7 @@ 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. }, [ activeMenu ] ); diff --git a/packages/components/src/navigation/item/use-navigation-tree-item.tsx b/packages/components/src/navigation/item/use-navigation-tree-item.tsx index 571b109910bfa5..335e55f2401bf4 100644 --- a/packages/components/src/navigation/item/use-navigation-tree-item.tsx +++ b/packages/components/src/navigation/item/use-navigation-tree-item.tsx @@ -41,6 +41,7 @@ export const useNavigationTreeItem = ( return () => { removeItem( itemId ); }; - // Ignore exhaustive-deps rule for now. See https://github.com/WordPress/gutenberg/pull/41639 + // 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 index 567da2405fc60a..f0014fa4768046 100644 --- a/packages/components/src/navigation/menu/menu-title-search.tsx +++ b/packages/components/src/navigation/menu/menu-title-search.tsx @@ -55,7 +55,8 @@ function MenuTitleSearch( { count ); debouncedSpeak( resultsFoundMessage ); - // Ignore exhaustive-deps rule for now. See https://github.com/WordPress/gutenberg/pull/44090 + // 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 = () => { diff --git a/packages/components/src/navigation/menu/use-navigation-tree-menu.tsx b/packages/components/src/navigation/menu/use-navigation-tree-menu.tsx index 3c46ed126bc781..8631812445cd7d 100644 --- a/packages/components/src/navigation/menu/use-navigation-tree-menu.tsx +++ b/packages/components/src/navigation/menu/use-navigation-tree-menu.tsx @@ -23,6 +23,7 @@ export const useNavigationTreeMenu = ( props: NavigationMenuProps ) => { return () => { removeMenu( key ); }; - // Ignore exhaustive-deps rule for now. See https://github.com/WordPress/gutenberg/pull/44090 + // Not adding deps for now, as it would require either a larger refactor + // See https://github.com/WordPress/gutenberg/pull/44090 }, [] ); }; diff --git a/packages/components/src/sandbox/index.native.js b/packages/components/src/sandbox/index.native.js index 8eaecf3a6a39e7..72070bf6efcd08 100644 --- a/packages/components/src/sandbox/index.native.js +++ b/packages/components/src/sandbox/index.native.js @@ -336,6 +336,7 @@ const Sandbox = forwardRef( function Sandbox( useEffect( () => { updateContentHtml(); + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ html, title, type, styles, scripts ] ); useEffect( () => { diff --git a/packages/components/src/sandbox/index.tsx b/packages/components/src/sandbox/index.tsx index ff1f2ca54deed2..5288fea3f7e43e 100644 --- a/packages/components/src/sandbox/index.tsx +++ b/packages/components/src/sandbox/index.tsx @@ -262,14 +262,20 @@ function SandBox( { checkMessageForResize ); }; + // Passing `exhaustive-deps` will likely involve a more detailed refactor. + // See https://github.com/WordPress/gutenberg/pull/44378 }, [] ); useEffect( () => { trySandBox(); + // Passing `exhaustive-deps` will likely involve a more detailed refactor. + // See https://github.com/WordPress/gutenberg/pull/44378 }, [ title, styles, scripts ] ); useEffect( () => { trySandBox( true ); + // Passing `exhaustive-deps` will likely involve a more detailed refactor. + // See https://github.com/WordPress/gutenberg/pull/44378 }, [ html, type ] ); return ( diff --git a/packages/components/src/search-control/index.native.js b/packages/components/src/search-control/index.native.js index b1d2b8d0098d91..5bb03196d1de14 100644 --- a/packages/components/src/search-control/index.native.js +++ b/packages/components/src/search-control/index.native.js @@ -122,6 +122,7 @@ function SearchControl( { mergeFutureStyles( activeDarkStyles, [ isActive, isDark ] ); setCurrentStyles( futureStyles ); + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ isActive, isDark ] ); const clearInput = useCallback( () => { diff --git a/packages/components/src/tooltip/index.native.js b/packages/components/src/tooltip/index.native.js index 4fd8de0814e66f..ff880d38e19822 100644 --- a/packages/components/src/tooltip/index.native.js +++ b/packages/components/src/tooltip/index.native.js @@ -62,6 +62,7 @@ const useKeyboardVisibility = () => { showListener.remove(); hideListener.remove(); }; + // See https://github.com/WordPress/gutenberg/pull/41166 }, [] ); return keyboardVisible; @@ -102,6 +103,7 @@ const Tooltip = ( { } ); } return () => onHandleScreenTouch( null ); + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ visible ] ); // Manage visibility animation. @@ -115,6 +117,7 @@ const Tooltip = ( { setAnimating( true ); startAnimation(); } + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ visible ] ); // Manage tooltip visibility and position in relation to keyboard. @@ -133,6 +136,7 @@ const Tooltip = ( { setAnimating( true ); setVisible( false ); } + // See https://github.com/WordPress/gutenberg/pull/41166 }, [ visible, keyboardVisible ] ); // Manage tooltip position during keyboard frame changes. @@ -261,6 +265,7 @@ const TooltipSlot = ( { children, ...rest } ) => { setHandleScreenTouch( null ); }; // Memoize context value to avoid unnecessary rerenders of the Provider's children + // See https://github.com/WordPress/gutenberg/pull/41166 const value = useMemo( () => ( { onHandleScreenTouch } ) ); return ( diff --git a/packages/components/src/unit-control/index.native.js b/packages/components/src/unit-control/index.native.js index d6f63f0a98447c..4aae14de65779e 100644 --- a/packages/components/src/unit-control/index.native.js +++ b/packages/components/src/unit-control/index.native.js @@ -47,6 +47,8 @@ function UnitControl( { if ( pickerRef?.current ) { pickerRef.current.presentPicker(); } + // It would be great if the deps could be addressed in the context of + // https://github.com/WordPress/gutenberg/pull/39218 }, [ pickerRef?.current ] ); const currentInputValue = currentInput === null ? value : currentInput; @@ -102,6 +104,8 @@ function UnitControl( { anchorNodeRef?.current ? findNodeHandle( anchorNodeRef?.current ) : undefined, + // It would be great if the deps could be addressed in the context of + // https://github.com/WordPress/gutenberg/pull/39218 [ anchorNodeRef?.current ] );