diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 2fddf51173e832..dc51518eb32f10 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -10,6 +10,7 @@ - `Popover`: Fix missing label of the headerTitle Close button ([#66813](https://github.com/WordPress/gutenberg/pull/66813)). - `ToggleGroupControl`: Fix active background for `0` value ([#66855](https://github.com/WordPress/gutenberg/pull/66855)). +- `SlotFill`: Fix a bug where a stale value of `fillProps` could be used ([#67000](https://github.com/WordPress/gutenberg/pull/67000)). ### Enhancements diff --git a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.tsx b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.tsx index 4d68db6fd175ee..16a19c6569fda6 100644 --- a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.tsx +++ b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.tsx @@ -34,15 +34,23 @@ function createSlotRegistry(): SlotFillBubblesVirtuallyContext { const unregisterSlot: SlotFillBubblesVirtuallyContext[ 'unregisterSlot' ] = ( name, ref ) => { + const slot = slots.get( name ); + if ( ! slot ) { + return; + } + // Make sure we're not unregistering a slot registered by another element // See https://github.com/WordPress/gutenberg/pull/19242#issuecomment-590295412 - if ( slots.get( name )?.ref === ref ) { - slots.delete( name ); + if ( slot.ref !== ref ) { + return; } + + slots.delete( name ); }; const updateSlot: SlotFillBubblesVirtuallyContext[ 'updateSlot' ] = ( name, + ref, fillProps ) => { const slot = slots.get( name ); @@ -50,6 +58,10 @@ function createSlotRegistry(): SlotFillBubblesVirtuallyContext { return; } + if ( slot.ref !== ref ) { + return; + } + if ( isShallowEqual( slot.fillProps, fillProps ) ) { return; } @@ -69,20 +81,18 @@ function createSlotRegistry(): SlotFillBubblesVirtuallyContext { fills.set( name, [ ...( fills.get( name ) || [] ), ref ] ); }; - const unregisterFill: SlotFillBubblesVirtuallyContext[ 'registerFill' ] = ( - name, - ref - ) => { - const fillsForName = fills.get( name ); - if ( ! fillsForName ) { - return; - } + const unregisterFill: SlotFillBubblesVirtuallyContext[ 'unregisterFill' ] = + ( name, ref ) => { + const fillsForName = fills.get( name ); + if ( ! fillsForName ) { + return; + } - fills.set( - name, - fillsForName.filter( ( fillRef ) => fillRef !== ref ) - ); - }; + fills.set( + name, + fillsForName.filter( ( fillRef ) => fillRef !== ref ) + ); + }; return { slots, diff --git a/packages/components/src/slot-fill/bubbles-virtually/slot.tsx b/packages/components/src/slot-fill/bubbles-virtually/slot.tsx index 6ac2d51e1f857f..b8ead7fc7ea8ba 100644 --- a/packages/components/src/slot-fill/bubbles-virtually/slot.tsx +++ b/packages/components/src/slot-fill/bubbles-virtually/slot.tsx @@ -35,29 +35,33 @@ function Slot( as, // `children` is not allowed. However, if it is passed, // it will be displayed as is, so remove `children`. - // @ts-ignore children, ...restProps } = props; const { registerSlot, unregisterSlot, ...registry } = useContext( SlotFillContext ); + const ref = useRef< HTMLElement >( null ); + // 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 + const fillPropsRef = useRef( fillProps ); + useLayoutEffect( () => { + fillPropsRef.current = fillProps; + }, [ fillProps ] ); + useLayoutEffect( () => { - registerSlot( name, ref, fillProps ); + registerSlot( name, ref, fillPropsRef.current ); return () => { unregisterSlot( name, ref ); }; - // 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 }, [ registerSlot, unregisterSlot, name ] ); - // fillProps may be an update that interacts with the layout, so we - // useLayoutEffect. + useLayoutEffect( () => { - registry.updateSlot( name, fillProps ); + registry.updateSlot( name, ref, fillPropsRef.current ); } ); return ( diff --git a/packages/components/src/slot-fill/bubbles-virtually/use-slot.ts b/packages/components/src/slot-fill/bubbles-virtually/use-slot.ts index d1d37e1d8e541c..ec78771bfa92af 100644 --- a/packages/components/src/slot-fill/bubbles-virtually/use-slot.ts +++ b/packages/components/src/slot-fill/bubbles-virtually/use-slot.ts @@ -21,8 +21,10 @@ export default function useSlot( name: SlotKey ) { const api = useMemo( () => ( { - updateSlot: ( fillProps: FillProps ) => - registry.updateSlot( name, fillProps ), + updateSlot: ( + ref: SlotFillBubblesVirtuallySlotRef, + fillProps: FillProps + ) => registry.updateSlot( name, ref, fillProps ), unregisterSlot: ( ref: SlotFillBubblesVirtuallySlotRef ) => registry.unregisterSlot( name, ref ), registerFill: ( ref: SlotFillBubblesVirtuallyFillRef ) => diff --git a/packages/components/src/slot-fill/types.ts b/packages/components/src/slot-fill/types.ts index 1711e04cbb1f47..7e1b8b7e1f3f9f 100644 --- a/packages/components/src/slot-fill/types.ts +++ b/packages/components/src/slot-fill/types.ts @@ -131,7 +131,11 @@ export type SlotFillBubblesVirtuallyContext = { name: SlotKey, ref: SlotFillBubblesVirtuallySlotRef ) => void; - updateSlot: ( name: SlotKey, fillProps: FillProps ) => void; + updateSlot: ( + name: SlotKey, + ref: SlotFillBubblesVirtuallySlotRef, + fillProps: FillProps + ) => void; registerFill: ( name: SlotKey, ref: SlotFillBubblesVirtuallyFillRef