From 4c48dcbc6ba60f5f63522dc44ae26787cc7340a0 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 23 Apr 2024 12:22:37 +0200 Subject: [PATCH] Rewrite block refs with observableMap, which moves to compose --- .../use-block-props/use-block-refs.js | 100 +++--------------- .../provider/block-refs-provider.js | 11 +- .../bubbles-virtually/slot-fill-context.ts | 3 +- .../bubbles-virtually/slot-fill-provider.tsx | 2 +- .../bubbles-virtually/use-slot-fills.ts | 2 +- .../slot-fill/bubbles-virtually/use-slot.ts | 2 +- packages/components/src/slot-fill/types.ts | 4 +- packages/compose/README.md | 13 +++ .../src/hooks/use-observable-value/index.ts | 29 +++++ .../hooks/use-observable-value/test/index.js | 42 ++++++++ packages/compose/src/index.js | 3 + packages/compose/src/index.native.js | 3 + .../src/utils/observable-map/index.ts} | 25 ----- .../src/utils/observable-map/test/index.js} | 42 +------- 14 files changed, 113 insertions(+), 168 deletions(-) create mode 100644 packages/compose/src/hooks/use-observable-value/index.ts create mode 100644 packages/compose/src/hooks/use-observable-value/test/index.js rename packages/{components/src/slot-fill/bubbles-virtually/observable-map.ts => compose/src/utils/observable-map/index.ts} (65%) rename packages/{components/src/slot-fill/test/observable-map.js => compose/src/utils/observable-map/test/index.js} (55%) diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-block-refs.js b/packages/block-editor/src/components/block-list/use-block-props/use-block-refs.js index 9554eca3e666d9..297aed456df7fc 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-block-refs.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-block-refs.js @@ -1,14 +1,8 @@ /** * WordPress dependencies */ -import { - useCallback, - useContext, - useLayoutEffect, - useMemo, - useRef, - useSyncExternalStore, -} from '@wordpress/element'; +import { useContext, useMemo, useRef } from '@wordpress/element'; +import { useRefEffect, useObservableValue } from '@wordpress/compose'; /** * Internal dependencies @@ -18,50 +12,6 @@ import { BlockRefs } from '../../provider/block-refs-provider'; /** @typedef {import('@wordpress/element').RefCallback} RefCallback */ /** @typedef {import('@wordpress/element').RefObject} RefObject */ -function addToMap( map, id, value ) { - let setForId = map.get( id ); - if ( ! setForId ) { - setForId = new Set(); - map.set( id, setForId ); - } - setForId.add( value ); -} - -function deleteFromMap( map, id, value ) { - const setForId = map.get( id ); - if ( ! setForId ) { - return; - } - setForId.delete( value ); - if ( setForId.size === 0 ) { - map.delete( id ); - } -} - -function getRefElement( refs, clientId ) { - // Multiple refs may be created for a single block. Find the - // first that has an element set. - const refsForId = refs.get( clientId ); - if ( refsForId ) { - for ( const ref of refsForId ) { - if ( ref.current ) { - return ref.current; - } - } - } - return null; -} - -function callListeners( callbacks, clientId ) { - const list = callbacks.get( clientId ); - if ( ! list ) { - return; - } - for ( const listener of list ) { - listener(); - } -} - /** * Provides a ref to the BlockRefs context. * @@ -70,26 +20,13 @@ function callListeners( callbacks, clientId ) { * @return {RefCallback} Ref callback. */ export function useBlockRefProvider( clientId ) { - const { refs, callbacks } = useContext( BlockRefs ); - const ref = useRef(); - useLayoutEffect( () => { - addToMap( refs, clientId, ref ); - return () => deleteFromMap( refs, clientId, ref ); - }, [ refs, clientId ] ); - - return useCallback( + const { refsMap } = useContext( BlockRefs ); + return useRefEffect( ( element ) => { - if ( ! element ) { - return; - } - - // Update the ref in the provider. - ref.current = element; - - // Notify the `useBlockElement` hooks that are observing this `clientId` - callListeners( callbacks, clientId ); + refsMap.set( clientId, element ); + return () => refsMap.delete( clientId ); }, - [ callbacks, clientId ] + [ clientId ] ); } @@ -104,7 +41,7 @@ export function useBlockRefProvider( clientId ) { * @return {RefObject} A ref containing the element. */ function useBlockRef( clientId ) { - const { refs } = useContext( BlockRefs ); + const { refsMap } = useContext( BlockRefs ); const latestClientId = useRef(); latestClientId.current = clientId; @@ -113,10 +50,10 @@ function useBlockRef( clientId ) { return useMemo( () => ( { get current() { - return getRefElement( refs, latestClientId.current ); + return refsMap.get( latestClientId.current ) ?? null; }, } ), - [ refs ] + [ refsMap ] ); } @@ -129,21 +66,8 @@ function useBlockRef( clientId ) { * @return {Element|null} The block's wrapper element. */ function useBlockElement( clientId ) { - const { refs, callbacks } = useContext( BlockRefs ); - const [ subscribe, getValue ] = useMemo( - () => [ - ( listener ) => { - addToMap( callbacks, clientId, listener ); - return () => { - deleteFromMap( callbacks, clientId, listener ); - }; - }, - () => getRefElement( refs, clientId ), - ], - [ refs, callbacks, clientId ] - ); - - return useSyncExternalStore( subscribe, getValue, getValue ); + const { refsMap } = useContext( BlockRefs ); + return useObservableValue( refsMap, clientId ) ?? null; } export { useBlockRef as __unstableUseBlockRef }; diff --git a/packages/block-editor/src/components/provider/block-refs-provider.js b/packages/block-editor/src/components/provider/block-refs-provider.js index 3f2d19b658a630..e54680356efda2 100644 --- a/packages/block-editor/src/components/provider/block-refs-provider.js +++ b/packages/block-editor/src/components/provider/block-refs-provider.js @@ -2,17 +2,12 @@ * WordPress dependencies */ import { createContext, useMemo } from '@wordpress/element'; +import { observableMap } from '@wordpress/compose'; -export const BlockRefs = createContext( { - refs: new Map(), - callbacks: new Map(), -} ); +export const BlockRefs = createContext( { refsMap: observableMap() } ); export function BlockRefsProvider( { children } ) { - const value = useMemo( - () => ( { refs: new Map(), callbacks: new Map() } ), - [] - ); + const value = useMemo( () => ( { refsMap: observableMap() } ), [] ); return ( { children } ); diff --git a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-context.ts b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-context.ts index e5af99fd3c95a7..a144a7dc33f464 100644 --- a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-context.ts +++ b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-context.ts @@ -3,11 +3,12 @@ */ import { createContext } from '@wordpress/element'; import warning from '@wordpress/warning'; +import { observableMap } from '@wordpress/compose'; + /** * Internal dependencies */ import type { SlotFillBubblesVirtuallyContext } from '../types'; -import { observableMap } from './observable-map'; const initialContextValue: SlotFillBubblesVirtuallyContext = { slots: observableMap(), 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 b68e06d05e60ad..bce3175e658c39 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 @@ -3,6 +3,7 @@ */ import { useMemo } from '@wordpress/element'; import isShallowEqual from '@wordpress/is-shallow-equal'; +import { observableMap } from '@wordpress/compose'; /** * Internal dependencies @@ -12,7 +13,6 @@ import type { SlotFillProviderProps, SlotFillBubblesVirtuallyContext, } from '../types'; -import { observableMap } from './observable-map'; function createSlotRegistry(): SlotFillBubblesVirtuallyContext { const slots: SlotFillBubblesVirtuallyContext[ 'slots' ] = observableMap(); diff --git a/packages/components/src/slot-fill/bubbles-virtually/use-slot-fills.ts b/packages/components/src/slot-fill/bubbles-virtually/use-slot-fills.ts index 819c43c4e78917..6229d20f2da513 100644 --- a/packages/components/src/slot-fill/bubbles-virtually/use-slot-fills.ts +++ b/packages/components/src/slot-fill/bubbles-virtually/use-slot-fills.ts @@ -2,13 +2,13 @@ * WordPress dependencies */ import { useContext } from '@wordpress/element'; +import { useObservableValue } from '@wordpress/compose'; /** * Internal dependencies */ 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 ); 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 6d211fbb3fa37f..d1d37e1d8e541c 100644 --- a/packages/components/src/slot-fill/bubbles-virtually/use-slot.ts +++ b/packages/components/src/slot-fill/bubbles-virtually/use-slot.ts @@ -2,6 +2,7 @@ * WordPress dependencies */ import { useMemo, useContext } from '@wordpress/element'; +import { useObservableValue } from '@wordpress/compose'; /** * Internal dependencies @@ -13,7 +14,6 @@ import type { FillProps, SlotKey, } from '../types'; -import { useObservableValue } from './observable-map'; export default function useSlot( name: SlotKey ) { const registry = useContext( SlotFillContext ); diff --git a/packages/components/src/slot-fill/types.ts b/packages/components/src/slot-fill/types.ts index 5e24ba20c72b4e..1711e04cbb1f47 100644 --- a/packages/components/src/slot-fill/types.ts +++ b/packages/components/src/slot-fill/types.ts @@ -4,9 +4,9 @@ import type { Component, MutableRefObject, ReactNode, RefObject } from 'react'; /** - * Internal dependencies + * WordPress dependencies */ -import type { ObservableMap } from './bubbles-virtually/observable-map'; +import type { ObservableMap } from '@wordpress/compose'; export type DistributiveOmit< T, K extends keyof any > = T extends any ? Omit< T, K > diff --git a/packages/compose/README.md b/packages/compose/README.md index e2c84e17bb849d..374d01a56b90f8 100644 --- a/packages/compose/README.md +++ b/packages/compose/README.md @@ -129,6 +129,10 @@ _Returns_ - Higher-order component. +### observableMap + +A key/value map where the individual entries are observable by subscribing to them with the `subscribe` methods. + ### pipe Composes multiple higher-order components into a single higher-order component. Performs left-to-right function composition, where each successive invocation is supplied the return value of the previous. @@ -442,6 +446,15 @@ _Returns_ - `import('react').RefCallback>`: The merged ref callback. +### useObservableValue + +React hook that lets you observe an individual entry in an `ObservableMap`. + +_Parameters_ + +- _map_ `ObservableMap< K, V >`: The `ObservableMap` to observe. +- _name_ `K`: The map key to observe. + ### usePrevious Use something's value from the previous render. Based on . diff --git a/packages/compose/src/hooks/use-observable-value/index.ts b/packages/compose/src/hooks/use-observable-value/index.ts new file mode 100644 index 00000000000000..8a52d415f3faad --- /dev/null +++ b/packages/compose/src/hooks/use-observable-value/index.ts @@ -0,0 +1,29 @@ +/** + * WordPress dependencies + */ +import { useMemo, useSyncExternalStore } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import type { ObservableMap } from '../../utils/observable-map'; + +/** + * React hook that lets you observe an individual entry in an `ObservableMap`. + * + * @param map The `ObservableMap` to observe. + * @param name The map key to observe. + */ +export default function useObservableValue< K, V >( + 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, getValue ); +} diff --git a/packages/compose/src/hooks/use-observable-value/test/index.js b/packages/compose/src/hooks/use-observable-value/test/index.js new file mode 100644 index 00000000000000..b53adf22f76b1e --- /dev/null +++ b/packages/compose/src/hooks/use-observable-value/test/index.js @@ -0,0 +1,42 @@ +/** + * External dependencies + */ +import { render, screen, act } from '@testing-library/react'; + +/** + * Internal dependencies + */ +import { observableMap } from '../../../utils/observable-map'; +import useObservableValue from '..'; + +describe( 'useObservableValue', () => { + test( 'reacts only to the specified key', () => { + const map = observableMap(); + map.set( 'a', 1 ); + + const MapUI = jest.fn( () => { + const value = useObservableValue( map, 'a' ); + return
value is { value }
; + } ); + + render( ); + expect( screen.getByText( /^value is/ ) ).toHaveTextContent( + 'value is 1' + ); + expect( MapUI ).toHaveBeenCalledTimes( 1 ); + + act( () => { + map.set( 'a', 2 ); + } ); + expect( screen.getByText( /^value is/ ) ).toHaveTextContent( + 'value is 2' + ); + expect( MapUI ).toHaveBeenCalledTimes( 2 ); + + // check that setting unobserved map key doesn't trigger a render at all + act( () => { + map.set( 'b', 1 ); + } ); + expect( MapUI ).toHaveBeenCalledTimes( 2 ); + } ); +} ); diff --git a/packages/compose/src/index.js b/packages/compose/src/index.js index 3d03463f490794..f7e1d1618f97fb 100644 --- a/packages/compose/src/index.js +++ b/packages/compose/src/index.js @@ -4,6 +4,8 @@ export * from './utils/create-higher-order-component'; export * from './utils/debounce'; // The `throttle` helper and its types. export * from './utils/throttle'; +// The `ObservableMap` data structure +export * from './utils/observable-map'; // The `compose` and `pipe` helpers (inspired by `flowRight` and `flow` from Lodash). export { default as compose } from './higher-order/compose'; @@ -46,3 +48,4 @@ export { default as useRefEffect } from './hooks/use-ref-effect'; export { default as __experimentalUseDropZone } from './hooks/use-drop-zone'; export { default as useFocusableIframe } from './hooks/use-focusable-iframe'; export { default as __experimentalUseFixedWindowList } from './hooks/use-fixed-window-list'; +export { default as useObservableValue } from './hooks/use-observable-value'; diff --git a/packages/compose/src/index.native.js b/packages/compose/src/index.native.js index 8d0953b81a14e5..4f3bf5f7603816 100644 --- a/packages/compose/src/index.native.js +++ b/packages/compose/src/index.native.js @@ -4,6 +4,8 @@ export * from './utils/create-higher-order-component'; export * from './utils/debounce'; // The `throttle` helper and its types. export * from './utils/throttle'; +// The `ObservableMap` data structure +export * from './utils/observable-map'; // The `compose` and `pipe` helpers (inspired by `flowRight` and `flow` from Lodash). export { default as compose } from './higher-order/compose'; @@ -39,3 +41,4 @@ export { default as useThrottle } from './hooks/use-throttle'; export { default as useMergeRefs } from './hooks/use-merge-refs'; export { default as useRefEffect } from './hooks/use-ref-effect'; export { default as useNetworkConnectivity } from './hooks/use-network-connectivity'; +export { default as useObservableValue } from './hooks/use-observable-value'; diff --git a/packages/components/src/slot-fill/bubbles-virtually/observable-map.ts b/packages/compose/src/utils/observable-map/index.ts similarity index 65% rename from packages/components/src/slot-fill/bubbles-virtually/observable-map.ts rename to packages/compose/src/utils/observable-map/index.ts index f4c27077e3f458..c8af7db6e0d4ca 100644 --- a/packages/components/src/slot-fill/bubbles-virtually/observable-map.ts +++ b/packages/compose/src/utils/observable-map/index.ts @@ -1,8 +1,3 @@ -/** - * WordPress dependencies - */ -import { useMemo, useSyncExternalStore } from '@wordpress/element'; - export type ObservableMap< K, V > = { get( name: K ): V | undefined; set( name: K, value: V ): void; @@ -57,23 +52,3 @@ export function observableMap< K, V >(): ObservableMap< K, V > { }, }; } - -/** - * React hook that lets you observe an individual entry in an `ObservableMap`. - * - * @param map The `ObservableMap` to observe. - * @param name The map key to observe. - */ -export function useObservableValue< K, V >( - 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, getValue ); -} diff --git a/packages/components/src/slot-fill/test/observable-map.js b/packages/compose/src/utils/observable-map/test/index.js similarity index 55% rename from packages/components/src/slot-fill/test/observable-map.js rename to packages/compose/src/utils/observable-map/test/index.js index ee3b3533bdd3ca..5383189c106306 100644 --- a/packages/components/src/slot-fill/test/observable-map.js +++ b/packages/compose/src/utils/observable-map/test/index.js @@ -1,15 +1,7 @@ -/** - * External dependencies - */ -import { render, screen, act } from '@testing-library/react'; - /** * Internal dependencies */ -import { - observableMap, - useObservableValue, -} from '../bubbles-virtually/observable-map'; +import { observableMap } from '..'; describe( 'ObservableMap', () => { test( 'should observe individual values', () => { @@ -49,35 +41,3 @@ describe( 'ObservableMap', () => { expect( listenerB ).toHaveBeenCalledTimes( 1 ); } ); } ); - -describe( 'useObservableValue', () => { - test( 'reacts only to the specified key', () => { - const map = observableMap(); - map.set( 'a', 1 ); - - const MapUI = jest.fn( () => { - const value = useObservableValue( map, 'a' ); - return
value is { value }
; - } ); - - render( ); - expect( screen.getByText( /^value is/ ) ).toHaveTextContent( - 'value is 1' - ); - expect( MapUI ).toHaveBeenCalledTimes( 1 ); - - act( () => { - map.set( 'a', 2 ); - } ); - expect( screen.getByText( /^value is/ ) ).toHaveTextContent( - 'value is 2' - ); - expect( MapUI ).toHaveBeenCalledTimes( 2 ); - - // check that setting unobserved map key doesn't trigger a render at all - act( () => { - map.set( 'b', 1 ); - } ); - expect( MapUI ).toHaveBeenCalledTimes( 2 ); - } ); -} );