Components: refactor SlotFill to pass exhaustive-deps (#44403)

packages/components/CHANGELOG.md

@@ -37,6 +37,7 @@
 -   `withFocusReturn`: Refactor tests to `@testing-library/react` ([#45012](https://github.com/WordPress/gutenberg/pull/45012)).
 -   `ToolsPanel`: updated to satisfy `react/exhaustive-deps` eslint rule ([#45028](https://github.com/WordPress/gutenberg/pull/45028))
 -   `Tooltip`: updated to ignore `react/exhaustive-deps` eslint rule ([#45043](https://github.com/WordPress/gutenberg/pull/45043))
+-   `SlotFill`: updated to satisfy `react/exhaustive-deps` eslint rule ([#44403](https://github.com/WordPress/gutenberg/pull/44403))
 
 ## 21.2.0 (2022-10-05)
 

packages/components/src/slot-fill/bubbles-virtually/fill.js

@@ -28,18 +28,18 @@ function useForceUpdate() {
 }
 
 export default function Fill( { name, children } ) {
-	const slot = useSlot( name );
+	const { registerFill, unregisterFill, ...slot } = useSlot( name );
 	const ref = useRef( { rerender: useForceUpdate() } );
 
 	useEffect( () => {
 		// We register fills so we can keep track of their existance.
 		// Some Slot implementations need to know if there're already fills
 		// registered so they can choose to render themselves or not.
-		slot.registerFill( ref );
+		registerFill( ref );
 		return () => {
-			slot.unregisterFill( ref );
+			unregisterFill( ref );
 		};
-	}, [ slot.registerFill, slot.unregisterFill ] );
+	}, [ registerFill, unregisterFill ] );
 
 	if ( ! slot.ref || ! slot.ref.current ) {
 		return null;

packages/components/src/slot-fill/bubbles-virtually/slot.js

@@ -19,19 +19,21 @@ function Slot(
 	{ name, fillProps = {}, as: Component = 'div', ...props },
 	forwardedRef
 ) {
-	const registry = useContext( SlotFillContext );
+	const { registerSlot, unregisterSlot, ...registry } =
+		useContext( SlotFillContext );
 	const ref = useRef();
 
 	useLayoutEffect( () => {
-		registry.registerSlot( name, ref, fillProps );
+		registerSlot( name, ref, fillProps );
 		return () => {
-			registry.unregisterSlot( name, ref );
+			unregisterSlot( name, ref );
 		};
-		// We are not including fillProps in the deps because we don't want to
-		// unregister and register the slot whenever fillProps change, which would
-		// cause the fill to be re-mounted. We are only considering the initial value
-		// of fillProps.
-	}, [ registry.registerSlot, registry.unregisterSlot, name ] );
+		// Ignore reason: We don't want to unregister and register the slot whenever
+		// `fillProps` change, which would cause the fill to be re-mounted. Instead,
+		// we can just update the slot (see hook below).
+		// For more context, see https://github.com/WordPress/gutenberg/pull/44403#discussion_r994415973
+		// eslint-disable-next-line react-hooks/exhaustive-deps
+	}, [ registerSlot, unregisterSlot, name ] );
 	// fillProps may be an update that interacts with the layout, so we
 	// useLayoutEffect.
 	useLayoutEffect( () => {

packages/components/src/slot-fill/bubbles-virtually/use-slot.js

@@ -15,7 +15,13 @@ import { useCallback, useContext } from '@wordpress/element';
 import SlotFillContext from './slot-fill-context';
 
 export default function useSlot( name ) {
-	const registry = useContext( SlotFillContext );
+	const {
+		updateSlot: registryUpdateSlot,
+		unregisterSlot: registryUnregisterSlot,
+		registerFill: registryRegisterFill,
+		unregisterFill: registryUnregisterFill,
+		...registry
+	} = useContext( SlotFillContext );
 	const slots = useSnapshot( registry.slots, { sync: true } );
 	// The important bit here is that this call ensures
 	// the hook only causes a re-render if the slot
@@ -24,30 +30,30 @@ export default function useSlot( name ) {
 
 	const updateSlot = useCallback(
 		( fillProps ) => {
-			registry.updateSlot( name, fillProps );
+			registryUpdateSlot( name, fillProps );
 		},
-		[ name, registry.updateSlot ]
+		[ name, registryUpdateSlot ]
 	);
 
 	const unregisterSlot = useCallback(
 		( slotRef ) => {
-			registry.unregisterSlot( name, slotRef );
+			registryUnregisterSlot( name, slotRef );
 		},
-		[ name, registry.unregisterSlot ]
+		[ name, registryUnregisterSlot ]
 	);
 
 	const registerFill = useCallback(
 		( fillRef ) => {
-			registry.registerFill( name, fillRef );
+			registryRegisterFill( name, fillRef );
 		},
-		[ name, registry.registerFill ]
+		[ name, registryRegisterFill ]
 	);
 
 	const unregisterFill = useCallback(
 		( fillRef ) => {
-			registry.unregisterFill( name, fillRef );
+			registryUnregisterFill( name, fillRef );
 		},
-		[ name, registry.unregisterFill ]
+		[ name, registryUnregisterFill ]
 	);
 
 	return {

packages/components/src/slot-fill/fill.js

@@ -20,15 +20,22 @@ function FillComponent( { name, children, registerFill, unregisterFill } ) {
 	} );
 
 	useLayoutEffect( () => {
-		registerFill( name, ref.current );
-		return () => unregisterFill( name, ref.current );
+		const refValue = ref.current;
+		registerFill( name, refValue );
+		return () => unregisterFill( name, refValue );
+		// Ignore reason: the useLayoutEffects here are written to fire at specific times, and introducing new dependencies could cause unexpected changes in behavior.
+		// We'll leave them as-is until a more detailed investigation/refactor can be performed.
+		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [] );
 
 	useLayoutEffect( () => {
 		ref.current.children = children;
 		if ( slot ) {
 			slot.forceUpdate();
 		}
+		// Ignore reason: the useLayoutEffects here are written to fire at specific times, and introducing new dependencies could cause unexpected changes in behavior.
+		// We'll leave them as-is until a more detailed investigation/refactor can be performed.
+		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ children ] );
 
 	useLayoutEffect( () => {
@@ -39,6 +46,9 @@ function FillComponent( { name, children, registerFill, unregisterFill } ) {
 		unregisterFill( ref.current.name, ref.current );
 		ref.current.name = name;
 		registerFill( name, ref.current );
+		// Ignore reason: the useLayoutEffects here are written to fire at specific times, and introducing new dependencies could cause unexpected changes in behavior.
+		// We'll leave them as-is until a more detailed investigation/refactor can be performed.
+		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ name ] );
 
 	if ( ! slot || ! slot.node ) {

packages/components/src/slot-fill/use-slot.js

@@ -26,6 +26,9 @@ const useSlot = ( name ) => {
 		} );
 
 		return unsubscribe;
+		// Ignore reason: Modifying this dep array could introduce unexpected changes in behavior,
+		// so we'll leave it as=is until the hook can be properly refactored for exhaustive-deps.
+		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, [ name ] );
 
 	return slot;