Fix React hook dependency warnings in filter an All Products blocks (#…

…3285)

* Fix wrong JSDocs

* Fix React hook dependency warnings in usePrevious hook

* Fix React hook dependency warnings in Filter an All Products blocks

assets/js/base/components/checkbox-list/index.js

@@ -16,7 +16,7 @@ import './style.scss';
  *
  * @param {Object} props Incoming props for the component.
  * @param {string} props.className CSS class used.
- * @param {function():any} props.onChange Function called when inputs change.
+ * @param {function(string):any} props.onChange Function called when inputs change.
  * @param {Array} props.options Options for list.
  * @param {Array} props.checked Which items are checked.
  * @param {boolean} props.isLoading If loading or not.
@@ -142,6 +142,7 @@ const CheckboxList = ( {
 		);
 	}, [
 		options,
+		onChange,
 		checked,
 		showExpanded,
 		limit,

assets/js/base/components/price-slider/constrain-range-slider-values.js

@@ -2,8 +2,8 @@
  * Validate a min and max value for a range slider against defined constraints (min, max, step).
  *
  * @param {Array} values Array containing min and max values.
- * @param {number} min Min allowed value for the sliders.
- * @param {number} max Max allowed value for the sliders.
+ * @param {number|null} min Min allowed value for the sliders.
+ * @param {number|null} max Max allowed value for the sliders.
  * @param {number} step Step value for the sliders.
  * @param {boolean} isMin Whether we're currently interacting with the min range slider or not, so we update the correct values.
  * @return {Array} Validated and updated min/max values that fit within the range slider constraints.

assets/js/base/components/price-slider/index.js

@@ -112,7 +112,6 @@ const PriceSlider = ( {
 		minConstraint,
 		maxConstraint,
 		hasValidConstraints,
-		stepValue,
 	] );
 
 	/**
@@ -186,7 +185,14 @@ const PriceSlider = ( {
 				parseInt( values[ 1 ], 10 ),
 			] );
 		},
-		[ minPrice, maxPrice, minConstraint, maxConstraint, stepValue ]
+		[
+			onChange,
+			minPrice,
+			maxPrice,
+			minConstraint,
+			maxConstraint,
+			stepValue,
+		]
 	);
 
 	/**
@@ -221,14 +227,7 @@ const PriceSlider = ( {
 				parseInt( values[ 1 ], 10 ),
 			] );
 		},
-		[
-			minConstraint,
-			maxConstraint,
-			stepValue,
-			minPriceInput,
-			maxPriceInput,
-			currency,
-		]
+		[ onChange, stepValue, minPriceInput, maxPriceInput ]
 	);
 
 	const classes = classnames(

assets/js/base/components/product-list/index.js

@@ -70,7 +70,7 @@ const generateQuery = ( { sortValue, currentPage, attributes } ) => {
 const extractPaginationAndSortAttributes = ( query ) => {
 	/* eslint-disable-next-line no-unused-vars, camelcase */
 	const { order, orderby, page, per_page, ...totalQuery } = query;
-	return totalQuery;
+	return totalQuery || {};
 };
 
 const announceLoadingCompletion = ( totalProducts ) => {
@@ -96,6 +96,11 @@ const announceLoadingCompletion = ( totalProducts ) => {
 	}
 };
 
+const areQueryTotalsDifferent = (
+	{ totalQuery: nextQuery, totalProducts: nextProducts },
+	{ totalQuery: currentQuery } = {}
+) => ! isEqual( nextQuery, currentQuery ) && Number.isFinite( nextProducts );
+
 const ProductList = ( {
 	attributes,
 	currentPage,
@@ -129,28 +134,27 @@ const ProductList = ( {
 	// the total number of products is a finite number.
 	const previousQueryTotals = usePrevious(
 		{ totalQuery, totalProducts },
-		(
-			{ totalQuery: nextQuery, totalProducts: nextProducts },
-			{ totalQuery: currentQuery } = {}
-		) =>
-			! isEqual( nextQuery, currentQuery ) &&
-			Number.isFinite( nextProducts )
+		areQueryTotalsDifferent
 	);
-	const isPreviousTotalQueryEqual =
-		typeof previousQueryTotals === 'object' &&
-		isEqual( totalQuery, previousQueryTotals.totalQuery );
+
+	// If query state (excluding pagination/sorting attributes) changed,
+	// reset pagination to the first page.
 	useEffect( () => {
-		// If query state (excluding pagination/sorting attributes) changed,
-		// reset pagination to the first page.
-		if ( ! isPreviousTotalQueryEqual ) {
-			onPageChange( 1 );
-
-			// Make sure there was a previous query, so we don't announce it on page load.
-			if ( previousQueryTotals ) {
-				announceLoadingCompletion( totalProducts );
-			}
+		if ( isEqual( totalQuery, previousQueryTotals?.totalQuery ) ) {
+			return;
+		}
+		onPageChange( 1 );
+
+		// Make sure there was a previous query, so we don't announce it on page load.
+		if ( previousQueryTotals?.totalQuery ) {
+			announceLoadingCompletion( totalProducts );
 		}
-	}, [ queryState ] );
+	}, [
+		previousQueryTotals?.totalQuery,
+		totalProducts,
+		onPageChange,
+		totalQuery,
+	] );
 
 	const onPaginationChange = ( newPage ) => {
 		scrollToTop( { focusableSelector: 'a, button' } );
@@ -175,7 +179,9 @@ const ProductList = ( {
 	const { contentVisibility } = attributes;
 	const perPage = attributes.columns * attributes.rows;
 	const totalPages =
-		! Number.isFinite( totalProducts ) && isPreviousTotalQueryEqual
+		! Number.isFinite( totalProducts ) &&
+		Number.isFinite( previousQueryTotals?.totalProducts ) &&
+		isEqual( totalQuery, previousQueryTotals?.totalQuery )
 			? Math.ceil( previousQueryTotals.totalProducts / perPage )
 			: Math.ceil( totalProducts / perPage );
 	const listProducts = products.length

assets/js/base/hooks/use-previous.js

@@ -20,7 +20,7 @@ export const usePrevious = ( value, validation ) => {
 		) {
 			ref.current = value;
 		}
-	}, [ value, ref.current ] );
+	}, [ value, validation ] );
 
 	return ref.current;
 };

assets/js/base/hooks/use-query-state.js

@@ -5,6 +5,8 @@ import { QUERY_STATE_STORE_KEY as storeKey } from '@woocommerce/block-data';
 import { useSelect, useDispatch } from '@wordpress/data';
 import { useRef, useEffect, useCallback } from '@wordpress/element';
 import { useQueryStateContext } from '@woocommerce/base-context';
+import { usePrevious } from '@woocommerce/base-hooks';
+import isShallowEqual from '@wordpress/is-shallow-equal';
 import { assign } from 'lodash';
 
 /**
@@ -43,7 +45,7 @@ export const useQueryStateByContext = ( context ) => {
 		( value ) => {
 			setValueForQueryContext( context, value );
 		},
-		[ context ]
+		[ context, setValueForQueryContext ]
 	);
 
 	return [ queryState, setQueryState ];
@@ -56,11 +58,11 @@ export const useQueryStateByContext = ( context ) => {
  * "Query State" is a wp.data store that keeps track of an arbitrary object of
  * query keys and their values.
  *
- * @param {*}      queryKey     The specific query key to retrieve the value for.
- * @param {*}      defaultValue Default value if query does not exist.
- * @param {string} [context]    What context to retrieve the query state for. If
- *                              not provided will attempt to use what is provided
- *                              by query state context.
+ * @param {*}      queryKey       The specific query key to retrieve the value for.
+ * @param {*}      [defaultValue] Default value if query does not exist.
+ * @param {string} [context]      What context to retrieve the query state for. If
+ *                                not provided will attempt to use what is provided
+ *                                by query state context.
  *
  * @return {*}  Whatever value is set at the query state index using the
  *              provided context and query key.
@@ -81,7 +83,7 @@ export const useQueryStateByKey = ( queryKey, defaultValue, context ) => {
 		( value ) => {
 			setQueryValue( context, queryKey, value );
 		},
-		[ context, queryKey ]
+		[ context, queryKey, setQueryValue ]
 	);
 
 	return [ queryValue, setQueryValueByKey ];
@@ -117,15 +119,31 @@ export const useSynchronizedQueryState = ( synchronizedQuery, context ) => {
 	const queryStateContext = useQueryStateContext();
 	context = context || queryStateContext;
 	const [ queryState, setQueryState ] = useQueryStateByContext( context );
+	const currentQueryState = useShallowEqual( queryState );
 	const currentSynchronizedQuery = useShallowEqual( synchronizedQuery );
+	const previousSynchronizedQuery = usePrevious( currentSynchronizedQuery );
 	// used to ensure we allow initial synchronization to occur before
 	// returning non-synced state.
 	const isInitialized = useRef( false );
 	// update queryState anytime incoming synchronizedQuery changes
 	useEffect( () => {
-		setQueryState( assign( {}, queryState, currentSynchronizedQuery ) );
-		isInitialized.current = true;
-	}, [ currentSynchronizedQuery ] );
+		if (
+			! isShallowEqual(
+				previousSynchronizedQuery,
+				currentSynchronizedQuery
+			)
+		) {
+			setQueryState(
+				assign( {}, currentQueryState, currentSynchronizedQuery )
+			);
+			isInitialized.current = true;
+		}
+	}, [
+		currentQueryState,
+		currentSynchronizedQuery,
+		previousSynchronizedQuery,
+		setQueryState,
+	] );
 	return isInitialized.current
 		? [ queryState, setQueryState ]
 		: [ synchronizedQuery, setQueryState ];

assets/js/blocks/active-filters/block.js

@@ -47,7 +47,13 @@ const ActiveFiltersBlock = ( {
 			},
 			displayStyle: blockAttributes.displayStyle,
 		} );
-	}, [ minPrice, maxPrice, formatPriceRange ] );
+	}, [
+		minPrice,
+		maxPrice,
+		blockAttributes.displayStyle,
+		setMinPrice,
+		setMaxPrice,
+	] );
 
 	const activeAttributeFilters = useMemo( () => {
 		return productAttributes.map( ( attribute ) => {
@@ -64,7 +70,7 @@ const ActiveFiltersBlock = ( {
 				/>
 			);
 		} );
-	}, [ productAttributes ] );
+	}, [ productAttributes, blockAttributes.displayStyle ] );
 
 	const hasFilters = () => {
 		return (

assets/js/blocks/attribute-filter/block.js

@@ -8,6 +8,7 @@ import {
 	useQueryStateByKey,
 	useQueryStateByContext,
 	useCollectionData,
+	usePrevious,
 	useShallowEqual,
 } from '@woocommerce/base-hooks';
 import {
@@ -113,7 +114,7 @@ const AttributeFilterBlock = ( {
 		 * @param {string} termSlug The term of the slug to check.
 		 */
 		const isTermInQueryState = ( termSlug ) => {
-			if ( ! queryState || ! queryState.attributes ) {
+			if ( ! queryState?.attributes ) {
 				return false;
 			}
 			return queryState.attributes.some(
@@ -168,16 +169,11 @@ const AttributeFilterBlock = ( {
 	] );
 
 	// Track checked STATE changes - if state changes, update the query.
-	useEffect(
-		() => {
-			if ( ! blockAttributes.showFilterButton ) {
-				onSubmit();
-			}
-		},
-		// There is no need to add blockAttributes.showFilterButton as a dependency.
-		// It will only change in the editor and there we don't need to call onSubmit in any case.
-		[ checked, onSubmit ]
-	);
+	useEffect( () => {
+		if ( ! blockAttributes.showFilterButton ) {
+			onSubmit( checked );
+		}
+	}, [ blockAttributes.showFilterButton, checked, onSubmit ] );
 
 	const checkedQuery = useMemo( () => {
 		return productAttributesQuery
@@ -188,17 +184,16 @@ const AttributeFilterBlock = ( {
 	}, [ productAttributesQuery, attributeObject.taxonomy ] );
 
 	const currentCheckedQuery = useShallowEqual( checkedQuery );
-
+	const previousCheckedQuery = usePrevious( currentCheckedQuery );
 	// Track ATTRIBUTES QUERY changes so the block reflects current filters.
-	useEffect(
-		() => {
-			if ( ! isShallowEqual( checked, checkedQuery ) ) {
-				setChecked( checkedQuery );
-			}
-		},
-		// We only want to apply this effect when the query changes, so we are intentionally leaving `checked` out of the dependencies.
-		[ currentCheckedQuery ]
-	);
+	useEffect( () => {
+		if (
+			! isShallowEqual( previousCheckedQuery, currentCheckedQuery ) && // checked query changed
+			! isShallowEqual( checked, currentCheckedQuery ) // checked query doesn't match the UI
+		) {
+			setChecked( currentCheckedQuery );
+		}
+	}, [ checked, currentCheckedQuery, previousCheckedQuery ] );
 
 	/**
 	 * Returns an array of term objects that have been chosen via the checkboxes.
@@ -215,19 +210,29 @@ const AttributeFilterBlock = ( {
 		[ attributeTerms ]
 	);
 
-	const onSubmit = () => {
-		if ( isEditor ) {
-			return;
-		}
+	const onSubmit = useCallback(
+		( isChecked ) => {
+			if ( isEditor ) {
+				return;
+			}
 
-		updateAttributeFilter(
+			updateAttributeFilter(
+				productAttributesQuery,
+				setProductAttributesQuery,
+				attributeObject,
+				getSelectedTerms( isChecked ),
+				blockAttributes.queryType === 'or' ? 'in' : 'and'
+			);
+		},
+		[
+			isEditor,
 			productAttributesQuery,
 			setProductAttributesQuery,
 			attributeObject,
-			getSelectedTerms( checked ),
-			blockAttributes.queryType === 'or' ? 'in' : 'and'
-		);
-	};
+			getSelectedTerms,
+			blockAttributes.queryType,
+		]
+	);
 
 	const multiple =
 		blockAttributes.displayStyle !== 'dropdown' ||
@@ -357,7 +362,7 @@ const AttributeFilterBlock = ( {
 					<FilterSubmitButton
 						className="wc-block-attribute-filter__button"
 						disabled={ isLoading || isDisabled }
-						onClick={ onSubmit }
+						onClick={ () => onSubmit( checked ) }
 					/>
 				) }
 			</div>