Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import {
fireEvent,
render,
render as baseRender,
screen,
waitFor,
within,
Expand All @@ -13,6 +13,7 @@ import userEvent from '@testing-library/user-event';
/**
* WordPress dependencies
*/
import { SlotFillProvider } from '@wordpress/components';
import { useState, createElement } from '@wordpress/element';
import { useSelect } from '@wordpress/data';

Expand Down Expand Up @@ -79,6 +80,10 @@ afterEach( () => {
mockFetchRichUrlData?.mockReset(); // Conditionally reset as it may NOT be a mock.
} );

function render( ui ) {
return baseRender( ui, { wrapper: SlotFillProvider } );
}

/**
* Workaround to trigger an arrow up keypress event.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import userEvent from '@testing-library/user-event';
* WordPress dependencies
*/
import { useState } from '@wordpress/element';

import { SlotFillProvider } from '@wordpress/components';
/**
* Internal dependencies
*/
Expand All @@ -19,16 +19,18 @@ const noop = () => {};
function TestWrapper() {
const [ mediaURL, setMediaURL ] = useState( 'https://example.media' );
return (
<MediaReplaceFlow
mediaId={ 1 }
mediaURL={ mediaURL }
allowedTypes={ [ 'png' ] }
accept="image/*"
onSelect={ noop }
onSelectURL={ setMediaURL }
onError={ noop }
onCloseModal={ noop }
/>
<SlotFillProvider>
<MediaReplaceFlow
mediaId={ 1 }
mediaURL={ mediaURL }
allowedTypes={ [ 'png' ] }
accept="image/*"
onSelect={ noop }
onSelectURL={ setMediaURL }
onError={ noop }
onCloseModal={ noop }
/>
</SlotFillProvider>
);
}

Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
- `Menu`: Clean up popover wrappers ([#74207](https://github.com/WordPress/gutenberg/pull/74207)).
- `Button`, `DateCalendar`, `DateRangeCalendar`, `CheckboxControl`, `Panel`, `Placeholder`: Remove outdated vendor prefix properties in CSS ([#74213](https://github.com/WordPress/gutenberg/pull/74213)).
- Updated `fast-deep-equal` imports for compatibility with strict Node.js resolution ([#74530](https://github.com/WordPress/gutenberg/pull/74530)).
- `SlotFill`: unify context providers and `Fill` implementations ([#68056](https://github.com/WordPress/gutenberg/pull/68056)).

## 30.9.0 (2025-11-26)

Expand Down
50 changes: 0 additions & 50 deletions packages/components/src/slot-fill/bubbles-virtually/fill.tsx

This file was deleted.

This file was deleted.

This file was deleted.

25 changes: 16 additions & 9 deletions packages/components/src/slot-fill/bubbles-virtually/slot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { useMergeRefs } from '@wordpress/compose';
* Internal dependencies
*/
import { View } from '../../view';
import SlotFillContext from './slot-fill-context';
import SlotFillContext from '../context';
import type { WordPressComponentProps } from '../../context';
import type { SlotComponentProps } from '../types';

Expand All @@ -40,25 +40,32 @@ function Slot(
} = props;

const registry = useContext( SlotFillContext );

const instanceRef = useRef( {} );
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( () => {
registry.registerSlot( name, ref, fillPropsRef.current );
return () => registry.unregisterSlot( name, ref );
const instance = instanceRef.current;
registry.registerSlot( name, {
type: 'portal',
instance,
ref,
fillProps: fillPropsRef.current,
} );
return () => registry.unregisterSlot( name, instance );
}, [ registry, name ] );

useLayoutEffect( () => {
registry.updateSlot( name, ref, fillPropsRef.current );
registry.updateSlot( name, {
type: 'portal',
instance: instanceRef.current,
ref,
fillProps: fillPropsRef.current,
} );
} );

return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,42 @@
/**
* WordPress dependencies
*/
import { useContext } from '@wordpress/element';
import { useObservableValue } from '@wordpress/compose';
import { useContext, useMemo, useSyncExternalStore } from '@wordpress/element';
import type { ObservableMap } from '@wordpress/compose';

/**
* Internal dependencies
*/
import SlotFillContext from './slot-fill-context';
import SlotFillContext from '../context';
import type { SlotKey } from '../types';

function useObservableValueWithSelector< K, V, S >(
Comment thread
ciampo marked this conversation as resolved.
Resolved
map: ObservableMap< K, V >,
name: K,
selector: ( v: V | undefined ) => S
) {
const subscribe = useMemo(
() => ( listener: () => void ) => map.subscribe( name, listener ),
[ map, name ]
);
const getValue = () => selector( map.get( name ) );
return useSyncExternalStore( subscribe, getValue, getValue );
}

function getLength< T >( array: T[] | undefined ) {
return array?.length;
}

export default function useSlotFills( name: SlotKey ) {
const registry = useContext( SlotFillContext );
return useObservableValue( registry.fills, name );
const length = useObservableValueWithSelector(
registry.fills,
name,
getLength

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is observing the array length enough? Any concerns if the fills change but have the same length?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the useSlotFills hook is an odd one, it returns an array for back-compat reasons, but the only thing that really matters is the .length of the array. Ideally the hook would return just a number. Now it returns an "opaque" array with a given length but no contents.

If you review usages of useSlotFills, we always look just at the .length. The hook is asking: does this slot have any fills registered? If it doesn't, we typically don't render the slot at all, avoiding an empty element in the UI.

);
// callers expect an opaque array with length `length`, so create that array
const fills = useMemo( () => {
return length !== undefined ? Array.from( { length } ) : undefined;
}, [ length ] );
Comment on lines +38 to +40

Copilot AI Jan 16, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 37 states 'callers expect an opaque array with length', but this creates an actual array of undefined values. Consider clarifying what 'opaque' means in this context and whether callers truly need an array or just the length property. If only length is needed, consider returning a simpler object like { length } instead.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're returning an array for back-compat reasons. The known consumers need to know only the length, but we're returning a full array so that legacy consumers don't fail when doing any legitimate array operation.

return fills;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@ import { useObservableValue } from '@wordpress/compose';
/**
* Internal dependencies
*/
import SlotFillContext from './slot-fill-context';
import type { SlotKey } from '../types';
import SlotFillContext from '../context';
import type { SlotKey, SlotRef } from '../types';

export default function useSlot( name: SlotKey ) {
const registry = useContext( SlotFillContext );
const slot = useObservableValue( registry.slots, name );
return { ...slot };
let ref: SlotRef | undefined;
if ( slot && slot.type === 'portal' ) {
ref = slot.ref;
}
return { ref };
}
Loading
Loading