Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SlotFill: replace valtio with custom ObservableMap #60879

Merged
merged 3 commits into from
Apr 19, 2024
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
65 changes: 2 additions & 63 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
- `CheckboxControl`: Streamline size styles ([#60475](https://github.com/WordPress/gutenberg/pull/60475)).
- Deprecate `reduceMotion` util ([#60839](https://github.com/WordPress/gutenberg/pull/60839)).
- `InputBase`: Simplify management of focus styles. Affects all components based on `InputControl` (e.g. `SearchControl`, `NumberControl`, `UnitControl`), as well as `SelectControl`, `CustomSelectControl`, and `TreeSelect` ([#60226](https://github.com/WordPress/gutenberg/pull/60226)).
- Removed dependency on `valtio`, replaced its usage in `SlotFill` with a custom object [#60879](https://github.com/WordPress/gutenberg/pull/60879)).

## 27.3.0 (2024-04-03)

Expand Down
3 changes: 1 addition & 2 deletions packages/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@
"react-colorful": "^5.3.1",
"remove-accents": "^0.5.0",
"use-lilius": "^2.0.5",
"uuid": "^9.0.1",
"valtio": "1.7.0"
"uuid": "^9.0.1"
},
"peerDependencies": {
"react": "^18.0.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/**
* WordPress dependencies
*/
import { useMemo, useSyncExternalStore } from '@wordpress/element';

export type ObservableMap< K, V > = {
get( name: K ): V | undefined;
set( name: K, value: V ): void;
delete( name: K ): void;
subscribe( name: K, listener: () => void ): () => void;
};

export function observableMap< K, V >(): ObservableMap< K, V > {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be nice to add some inline docs and/or some tests.

const map = new Map< K, V >();
const listeners = new Map< K, Set< () => void > >();

function callListeners( name: K ) {
const list = listeners.get( name );
if ( ! list ) {
return;
}
for ( const listener of list ) {
listener();
}
}

function unsubscribe( name: K, listener: () => void ) {
return () => {
const list = listeners.get( name );
if ( ! list ) {
return;
}

list.delete( listener );
if ( list.size === 0 ) {
listeners.delete( name );
}
};
}

return {
get( name ) {
return map.get( name );
},
set( name, value ) {
map.set( name, value );
callListeners( name );
},
delete( name ) {
map.delete( name );
callListeners( name );
},
subscribe( name, listener ) {
let list = listeners.get( name );
if ( ! list ) {
list = new Set();
listeners.set( name, list );
}
list.add( listener );

return unsubscribe( name, listener );
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be possible to avoid having to get the list again if it's inlined? Or can it disappear/change some other way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, will do in a followup 👍

},
};
}

export function useObservableValue< K, V >(
Copy link
Member

Choose a reason for hiding this comment

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

I'm not particularly convinced this belongs here, but if we don't need it outside of the component for now, that should be fine.

map: ObservableMap< K, V >,
name: K
): V | undefined {
const [ subscribe, getValue ] = useMemo(
() => [
( listener: () => void ) => map.subscribe( name, listener ),
() => map.get( name ),
],
[ map, name ]
);
return useSyncExternalStore( subscribe, getValue );
}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me figure out what useBlockRefs does 🙂

Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
/**
* External dependencies
*/
import { proxyMap } from 'valtio/utils';
/**
* WordPress dependencies
*/
Expand All @@ -11,10 +7,11 @@ import warning from '@wordpress/warning';
* Internal dependencies
*/
import type { SlotFillBubblesVirtuallyContext } from '../types';
import { observableMap } from './observable-map';

const initialContextValue: SlotFillBubblesVirtuallyContext = {
slots: proxyMap(),
fills: proxyMap(),
slots: observableMap(),
fills: observableMap(),
registerSlot: () => {
warning(
'Components must be wrapped within `SlotFillProvider`. ' +
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
/**
* External dependencies
*/
import { ref as valRef } from 'valtio';
import { proxyMap } from 'valtio/utils';

/**
* WordPress dependencies
*/
Expand All @@ -18,10 +12,11 @@ import type {
SlotFillProviderProps,
SlotFillBubblesVirtuallyContext,
} from '../types';
import { observableMap } from './observable-map';

function createSlotRegistry(): SlotFillBubblesVirtuallyContext {
const slots: SlotFillBubblesVirtuallyContext[ 'slots' ] = proxyMap();
const fills: SlotFillBubblesVirtuallyContext[ 'fills' ] = proxyMap();
const slots: SlotFillBubblesVirtuallyContext[ 'slots' ] = observableMap();
const fills: SlotFillBubblesVirtuallyContext[ 'fills' ] = observableMap();

const registerSlot: SlotFillBubblesVirtuallyContext[ 'registerSlot' ] = (
name,
Expand All @@ -30,14 +25,11 @@ function createSlotRegistry(): SlotFillBubblesVirtuallyContext {
) => {
const slot = slots.get( name );

slots.set(
name,
valRef( {
...slot,
ref: ref || slot?.ref,
fillProps: fillProps || slot?.fillProps || {},
} )
);
slots.set( name, {
...slot,
ref: ref || slot?.ref,
fillProps: fillProps || slot?.fillProps || {},
} );
};

const unregisterSlot: SlotFillBubblesVirtuallyContext[ 'unregisterSlot' ] =
Expand Down Expand Up @@ -74,7 +66,7 @@ function createSlotRegistry(): SlotFillBubblesVirtuallyContext {
name,
ref
) => {
fills.set( name, valRef( [ ...( fills.get( name ) || [] ), ref ] ) );
fills.set( name, [ ...( fills.get( name ) || [] ), ref ] );
};

const unregisterFill: SlotFillBubblesVirtuallyContext[ 'registerFill' ] = (
Expand All @@ -88,7 +80,7 @@ function createSlotRegistry(): SlotFillBubblesVirtuallyContext {

fills.set(
name,
valRef( fillsForName.filter( ( fillRef ) => fillRef !== ref ) )
fillsForName.filter( ( fillRef ) => fillRef !== ref )
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { useSnapshot } from 'valtio';

/**
* WordPress dependencies
*/
Expand All @@ -13,12 +8,9 @@ import { useContext } from '@wordpress/element';
*/
import SlotFillContext from './slot-fill-context';
import type { SlotKey } from '../types';
import { useObservableValue } from './observable-map';

export default function useSlotFills( name: SlotKey ) {
const registry = useContext( SlotFillContext );
const fills = useSnapshot( registry.fills, { sync: true } );
// The important bit here is that this call ensures that the hook
// only causes a re-render if the "fills" of a given slot name
// change, not any fills.
return fills.get( name );
return useObservableValue( registry.fills, name );
}
12 changes: 2 additions & 10 deletions packages/components/src/slot-fill/bubbles-virtually/use-slot.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { useSnapshot } from 'valtio';

/**
* WordPress dependencies
*/
Expand All @@ -18,14 +13,11 @@ import type {
FillProps,
SlotKey,
} from '../types';
import { useObservableValue } from './observable-map';

export default function useSlot( name: SlotKey ) {
const registry = useContext( SlotFillContext );
const slots = useSnapshot( registry.slots, { sync: true } );
// The important bit here is that the `useSnapshot` call ensures that the
// hook only causes a re-render if the slot with the given name changes,
// not any other slot.
const slot = slots.get( name );
const slot = useObservableValue( registry.slots, name );

const api = useMemo(
() => ( {
Expand Down
9 changes: 7 additions & 2 deletions packages/components/src/slot-fill/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
*/
import type { Component, MutableRefObject, ReactNode, RefObject } from 'react';

/**
* Internal dependencies
*/
import type { ObservableMap } from './bubbles-virtually/observable-map';

export type DistributiveOmit< T, K extends keyof any > = T extends any
? Omit< T, K >
: never;
Expand Down Expand Up @@ -109,14 +114,14 @@ export type SlotFillBubblesVirtuallyFillRef = MutableRefObject< {
} >;

export type SlotFillBubblesVirtuallyContext = {
slots: Map<
slots: ObservableMap<
SlotKey,
{
ref: SlotFillBubblesVirtuallySlotRef;
fillProps: FillProps;
}
>;
fills: Map< SlotKey, SlotFillBubblesVirtuallyFillRef[] >;
fills: ObservableMap< SlotKey, SlotFillBubblesVirtuallyFillRef[] >;
registerSlot: (
name: SlotKey,
ref: SlotFillBubblesVirtuallySlotRef,
Expand Down
Loading