From d05a9f4af210a0d84944eff5d0de4cf47c6c7223 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Thu, 31 Mar 2022 12:03:51 +0200 Subject: [PATCH 1/3] Data: add useSuspenseSelect hook --- packages/data/README.md | 28 ++++ .../data/src/components/use-select/index.js | 128 ++++++++++++++++++ packages/data/src/index.js | 17 ++- packages/data/src/redux-store/index.js | 39 ++++++ packages/data/src/registry.js | 34 ++++- 5 files changed, 241 insertions(+), 5 deletions(-) diff --git a/packages/data/README.md b/packages/data/README.md index 2e3e07a13ecc1b..757b4e8c383ae4 100644 --- a/packages/data/README.md +++ b/packages/data/README.md @@ -662,6 +662,20 @@ _Parameters_ - _listener_ `Function`: Callback function. +### suspendSelect + +Given the name of a registered store, returns an object containing the store's +selectors pre-bound to state so that you only need to supply additional arguments, +and modified so that they throw promises in case the selector is not resolved yet. + +_Parameters_ + +- _storeNameOrDescriptor_ `string|StoreDescriptor`: Unique namespace identifier for the store or the store descriptor. + +_Returns_ + +- `Object`: Object containing the store's suspense-wrapped selectors. + ### use Extends a registry to inherit functionality provided by a given plugin. A @@ -827,6 +841,20 @@ _Returns_ - `Function`: A custom react hook. +### useSuspenseSelect + +A variant of the `useSelect` hook that has the same API, but will throw a +suspense Promise if any of the called selectors is in an unresolved state. + +_Parameters_ + +- _mapSelect_ `Function`: Function called on every state change. The returned value is exposed to the component using this hook. The function receives the `registry.suspendSelect` method as the first argument and the `registry` as the second one. +- _deps_ `Array`: A dependency array used to memoize the `mapSelect` so that the same `mapSelect` is invoked on every state change unless the dependencies change. + +_Returns_ + +- `Object`: Data object returned by the `mapSelect` function. + ### withDispatch Higher-order component used to add dispatch props using registered action diff --git a/packages/data/src/components/use-select/index.js b/packages/data/src/components/use-select/index.js index 78fd9fc6b3c74c..ec07ca9d3e804a 100644 --- a/packages/data/src/components/use-select/index.js +++ b/packages/data/src/components/use-select/index.js @@ -241,3 +241,131 @@ export default function useSelect( mapSelect, deps ) { return hasMappingFunction ? mapOutput : registry.select( mapSelect ); } + +/** + * A variant of the `useSelect` hook that has the same API, but will throw a + * suspense Promise if any of the called selectors is in an unresolved state. + * + * @param {Function} mapSelect Function called on every state change. The + * returned value is exposed to the component + * using this hook. The function receives the + * `registry.suspendSelect` method as the first + * argument and the `registry` as the second one. + * @param {Array} deps A dependency array used to memoize the `mapSelect` + * so that the same `mapSelect` is invoked on every + * state change unless the dependencies change. + * + * @return {Object} Data object returned by the `mapSelect` function. + */ +export function useSuspenseSelect( mapSelect, deps ) { + const _mapSelect = useCallback( mapSelect, deps ); + + const registry = useRegistry(); + const isAsync = useAsyncMode(); + // React can sometimes clear the `useMemo` cache. + // We use the cache-stable `useMemoOne` to avoid + // losing queues. + const queueContext = useMemoOne( () => ( { queue: true } ), [ registry ] ); + const [ , forceRender ] = useReducer( ( s ) => s + 1, 0 ); + + const latestMapSelect = useRef(); + const latestIsAsync = useRef( isAsync ); + const latestMapOutput = useRef(); + const latestMapOutputError = useRef(); + const isMountedAndNotUnsubscribing = useRef(); + + // Keep track of the stores being selected in the `mapSelect` function, + // and only subscribe to those stores later. + const listeningStores = useRef( [] ); + const trapSelect = useCallback( + ( callback ) => + registry.__experimentalMarkListeningStores( + () => callback( registry.suspendSelect, registry ), + listeningStores + ), + [ registry ] + ); + + let mapOutput = latestMapOutput.current; + let mapOutputError = latestMapOutputError.current; + + if ( latestMapSelect.current !== _mapSelect ) { + try { + mapOutput = trapSelect( _mapSelect ); + } catch ( error ) { + mapOutputError = error; + } + } + + useIsomorphicLayoutEffect( () => { + latestMapSelect.current = _mapSelect; + latestMapOutput.current = mapOutput; + latestMapOutputError.current = mapOutputError; + isMountedAndNotUnsubscribing.current = true; + + // This has to run after the other ref updates + // to avoid using stale values in the flushed + // callbacks or potentially overwriting a + // changed `latestMapOutput.current`. + if ( latestIsAsync.current !== isAsync ) { + latestIsAsync.current = isAsync; + renderQueue.flush( queueContext ); + } + } ); + + // Generate a "flag" for used in the effect dependency array. + // It's different than just using `mapSelect` since deps could be undefined, + // in that case, we would still want to memoize it. + const depsChangedFlag = useMemo( () => ( {} ), deps || [] ); + + useIsomorphicLayoutEffect( () => { + const onStoreChange = () => { + if ( ! isMountedAndNotUnsubscribing.current ) { + return; + } + + try { + const newMapOutput = trapSelect( latestMapSelect.current ); + + if ( isShallowEqual( latestMapOutput.current, newMapOutput ) ) { + return; + } + + latestMapOutput.current = newMapOutput; + } catch ( error ) { + latestMapOutputError.current = error; + } + + forceRender(); + }; + + const onChange = () => { + if ( latestIsAsync.current ) { + renderQueue.add( queueContext, onStoreChange ); + } else { + onStoreChange(); + } + }; + + // catch any possible state changes during mount before the subscription + // could be set. + onChange(); + + const unsubscribers = listeningStores.current.map( ( storeName ) => + registry.__experimentalSubscribeStore( storeName, onChange ) + ); + + return () => { + isMountedAndNotUnsubscribing.current = false; + // The return value of the subscribe function could be undefined if the store is a custom generic store. + unsubscribers.forEach( ( unsubscribe ) => unsubscribe?.() ); + renderQueue.flush( queueContext ); + }; + }, [ registry, trapSelect, depsChangedFlag ] ); + + if ( mapOutputError ) { + throw mapOutputError; + } + + return mapOutput; +} diff --git a/packages/data/src/index.js b/packages/data/src/index.js index 0dc98955f212c2..fe7106cf44fca4 100644 --- a/packages/data/src/index.js +++ b/packages/data/src/index.js @@ -19,7 +19,10 @@ export { RegistryConsumer, useRegistry, } from './components/registry-provider'; -export { default as useSelect } from './components/use-select'; +export { + default as useSelect, + useSuspenseSelect, +} from './components/use-select'; export { useDispatch } from './components/use-dispatch'; export { AsyncModeProvider } from './components/async-mode-provider'; export { createRegistry } from './registry'; @@ -115,6 +118,18 @@ export const select = defaultRegistry.select; */ export const resolveSelect = defaultRegistry.resolveSelect; +/** + * Given the name of a registered store, returns an object containing the store's + * selectors pre-bound to state so that you only need to supply additional arguments, + * and modified so that they throw promises in case the selector is not resolved yet. + * + * @param {string|StoreDescriptor} storeNameOrDescriptor Unique namespace identifier for the store + * or the store descriptor. + * + * @return {Object} Object containing the store's suspense-wrapped selectors. + */ +export const suspendSelect = defaultRegistry.suspendSelect; + /** * Given the name of a registered store, returns an object of the store's action creators. * Calling an action creator will cause it to be dispatched, updating the state value accordingly. diff --git a/packages/data/src/redux-store/index.js b/packages/data/src/redux-store/index.js index 15ee6c84d012c8..64be2a70aed93e 100644 --- a/packages/data/src/redux-store/index.js +++ b/packages/data/src/redux-store/index.js @@ -162,10 +162,12 @@ export default function createReduxStore( key, options ) { } const resolveSelectors = mapResolveSelectors( selectors, store ); + const suspendSelectors = mapSuspendSelectors( selectors, store ); const getSelectors = () => selectors; const getActions = () => actions; const getResolveSelectors = () => resolveSelectors; + const getSuspendSelectors = () => suspendSelectors; // We have some modules monkey-patching the store object // It's wrong to do so but until we refactor all of our effects to controls @@ -200,6 +202,7 @@ export default function createReduxStore( key, options ) { resolvers, getSelectors, getResolveSelectors, + getSuspendSelectors, getActions, subscribe, }; @@ -376,6 +379,42 @@ function mapResolveSelectors( selectors, store ) { } ); } +/** + * Maps selectors to functions that throw a suspense promise if not yet resolved. + * + * @param {Object} selectors Selectors to map. + * @param {Object} store The redux store the selectors select from. + * + * @return {Object} Selectors mapped to their suspense functions. + */ +function mapSuspendSelectors( selectors, store ) { + return mapValues( selectors, ( selector, selectorName ) => { + // Selector without a resolver doesn't have any extra suspense behavior. + if ( ! selector.hasResolver ) { + return selector; + } + + return ( ...args ) => { + const result = selector.apply( null, args ); + + if ( selectors.hasFinishedResolution( selectorName, args ) ) { + return result; + } + + throw new Promise( ( resolve ) => { + const unsubscribe = store.subscribe( () => { + if ( + selectors.hasFinishedResolution( selectorName, args ) + ) { + resolve(); + unsubscribe(); + } + } ); + } ); + }; + } ); +} + /** * Returns resolvers with matched selectors for a given namespace. * Resolvers are side effects invoked once per argument set of a given selector call, diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js index 32150fbbc5d455..31a1ccd34288f9 100644 --- a/packages/data/src/registry.js +++ b/packages/data/src/registry.js @@ -93,14 +93,16 @@ export function createRegistry( storeConfigs = {}, parent = null ) { return store.getSelectors(); } - return parent && parent.select( storeName ); + return parent?.select( storeName ); } function __unstableMarkListeningStores( callback, ref ) { listeningStores.clear(); - const result = callback.call( this ); - ref.current = Array.from( listeningStores ); - return result; + try { + return callback.call( this ); + } finally { + ref.current = Array.from( listeningStores ); + } } /** @@ -127,6 +129,29 @@ export function createRegistry( storeConfigs = {}, parent = null ) { return parent && parent.resolveSelect( storeName ); } + /** + * Given the name of a registered store, returns an object containing the store's + * selectors pre-bound to state so that you only need to supply additional arguments, + * and modified so that they throw promises in case the selector is not resolved yet. + * + * @param {string|StoreDescriptor} storeNameOrDescriptor Unique namespace identifier for the store + * or the store descriptor. + * + * @return {Object} Object containing the store's suspense-wrapped selectors. + */ + function suspendSelect( storeNameOrDescriptor ) { + const storeName = isObject( storeNameOrDescriptor ) + ? storeNameOrDescriptor.name + : storeNameOrDescriptor; + __experimentalListeningStores.add( storeName ); + const store = stores[ storeName ]; + if ( store ) { + return store.getSuspendSelectors(); + } + + return parent && parent.suspendSelect( storeName ); + } + /** * Returns the available actions for a part of the state. * @@ -276,6 +301,7 @@ export function createRegistry( storeConfigs = {}, parent = null ) { subscribe, select, resolveSelect, + suspendSelect, dispatch, use, register, From 1ccfadcf76a93d7030cdbd0ab4137d5457d76bb6 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 1 Apr 2022 13:40:37 +0200 Subject: [PATCH 2/3] useSuspenseSelect: throw resolution error, add unit test suite --- .../components/use-select/test/suspense.js | 160 ++++++++++++++++++ packages/data/src/redux-store/index.js | 4 + 2 files changed, 164 insertions(+) create mode 100644 packages/data/src/components/use-select/test/suspense.js diff --git a/packages/data/src/components/use-select/test/suspense.js b/packages/data/src/components/use-select/test/suspense.js new file mode 100644 index 00000000000000..b96f10b07a4981 --- /dev/null +++ b/packages/data/src/components/use-select/test/suspense.js @@ -0,0 +1,160 @@ +/** + * External dependencies + */ +import { render, waitFor } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import { + createRegistry, + createReduxStore, + useSuspenseSelect, + RegistryProvider, +} from '@wordpress/data'; +import { Component, Suspense } from '@wordpress/element'; + +jest.useRealTimers(); + +function createRegistryWithStore() { + const initialState = { + prefix: 'pre-', + token: null, + data: null, + fails: true, + }; + + const reducer = ( state = initialState, action ) => { + switch ( action.type ) { + case 'RECEIVE_TOKEN': + return { ...state, token: action.token }; + case 'RECEIVE_DATA': + return { ...state, data: action.data }; + default: + return state; + } + }; + + const selectors = { + getPrefix: ( state ) => state.prefix, + getToken: ( state ) => state.token, + getData: ( state, token ) => { + if ( ! token ) { + throw 'missing token in selector'; + } + return state.data; + }, + getThatFails: ( state ) => state.fails, + }; + + const sleep = ( ms ) => new Promise( ( r ) => setTimeout( () => r(), ms ) ); + + const resolvers = { + getToken: () => async ( { dispatch } ) => { + await sleep( 10 ); + dispatch( { type: 'RECEIVE_TOKEN', token: 'token' } ); + }, + getData: ( token ) => async ( { dispatch } ) => { + await sleep( 10 ); + if ( ! token ) { + throw 'missing token in resolver'; + } + dispatch( { type: 'RECEIVE_DATA', data: 'therealdata' } ); + }, + getThatFails: () => async () => { + await sleep( 10 ); + throw 'resolution failed'; + }, + }; + + const store = createReduxStore( 'test', { + reducer, + selectors, + resolvers, + } ); + + const registry = createRegistry(); + registry.register( store ); + + return { registry, store }; +} + +describe( 'useSuspenseSelect', () => { + it( 'renders after suspending a few times', async () => { + const { registry, store } = createRegistryWithStore(); + let attempts = 0; + let renders = 0; + + const UI = () => { + attempts++; + const { result } = useSuspenseSelect( ( select ) => { + const prefix = select( store ).getPrefix(); + const token = select( store ).getToken(); + const data = select( store ).getData( token ); + return { result: prefix + data }; + }, [] ); + renders++; + return
{ result }
; + }; + + const App = () => ( + + + + + + ); + + const rendered = render( ); + await waitFor( () => rendered.getByLabelText( 'loaded' ) ); + + // Verify there were 3 attempts to render. Suspended twice because of + // `getToken` and `getData` selectors not being resolved, and then finally + // rendered after all data got loaded. + expect( attempts ).toBe( 3 ); + expect( renders ).toBe( 1 ); + } ); + + it( 'shows error when resolution fails', async () => { + const { registry, store } = createRegistryWithStore(); + + const UI = () => { + const { token } = useSuspenseSelect( ( select ) => { + // Call a selector whose resolution fails. The `useSuspenseSelect` + // is then supposed to throw the resolution error. + return { token: select( store ).getThatFails() }; + }, [] ); + return
{ token }
; + }; + + class Error extends Component { + state = { error: null }; + + static getDerivedStateFromError( error ) { + return { error }; + } + + render() { + if ( this.state.error ) { + return
{ this.state.error }
; + } + return this.props.children; + } + } + + const App = () => ( + + + + + + + + ); + + const rendered = render( ); + const label = await waitFor( () => rendered.getByLabelText( 'error' ) ); + expect( label.textContent ).toBe( 'resolution failed' ); + expect( console ).toHaveErrored(); + } ); +} ); diff --git a/packages/data/src/redux-store/index.js b/packages/data/src/redux-store/index.js index 64be2a70aed93e..08b36435d7fdc7 100644 --- a/packages/data/src/redux-store/index.js +++ b/packages/data/src/redux-store/index.js @@ -398,6 +398,10 @@ function mapSuspendSelectors( selectors, store ) { const result = selector.apply( null, args ); if ( selectors.hasFinishedResolution( selectorName, args ) ) { + if ( selectors.hasResolutionFailed( selectorName, args ) ) { + throw selectors.getResolutionError( selectorName, args ); + } + return result; } From 23ecd5668889de9bfa2e406d870caba1ce1d1a3d Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 26 Apr 2022 11:11:32 +0200 Subject: [PATCH 3/3] Sync with recent useSelect changes --- .../data/src/components/use-select/index.js | 69 +++++++++---------- packages/data/src/registry.js | 2 +- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/packages/data/src/components/use-select/index.js b/packages/data/src/components/use-select/index.js index ec07ca9d3e804a..575af0369ed090 100644 --- a/packages/data/src/components/use-select/index.js +++ b/packages/data/src/components/use-select/index.js @@ -262,75 +262,68 @@ export function useSuspenseSelect( mapSelect, deps ) { const registry = useRegistry(); const isAsync = useAsyncMode(); - // React can sometimes clear the `useMemo` cache. - // We use the cache-stable `useMemoOne` to avoid - // losing queues. - const queueContext = useMemoOne( () => ( { queue: true } ), [ registry ] ); - const [ , forceRender ] = useReducer( ( s ) => s + 1, 0 ); + const latestRegistry = useRef( registry ); const latestMapSelect = useRef(); const latestIsAsync = useRef( isAsync ); const latestMapOutput = useRef(); const latestMapOutputError = useRef(); - const isMountedAndNotUnsubscribing = useRef(); // Keep track of the stores being selected in the `mapSelect` function, // and only subscribe to those stores later. const listeningStores = useRef( [] ); - const trapSelect = useCallback( + const wrapSelect = useCallback( ( callback ) => - registry.__experimentalMarkListeningStores( + registry.__unstableMarkListeningStores( () => callback( registry.suspendSelect, registry ), listeningStores ), [ registry ] ); + // Generate a "flag" for used in the effect dependency array. + // It's different than just using `mapSelect` since deps could be undefined, + // in that case, we would still want to memoize it. + const depsChangedFlag = useMemo( () => ( {} ), deps || [] ); + let mapOutput = latestMapOutput.current; let mapOutputError = latestMapOutputError.current; - if ( latestMapSelect.current !== _mapSelect ) { + const hasReplacedRegistry = latestRegistry.current !== registry; + const hasReplacedMapSelect = latestMapSelect.current !== _mapSelect; + const hasLeftAsyncMode = latestIsAsync.current && ! isAsync; + + if ( hasReplacedRegistry || hasReplacedMapSelect || hasLeftAsyncMode ) { try { - mapOutput = trapSelect( _mapSelect ); + mapOutput = wrapSelect( _mapSelect ); } catch ( error ) { mapOutputError = error; } } useIsomorphicLayoutEffect( () => { + latestRegistry.current = registry; latestMapSelect.current = _mapSelect; + latestIsAsync.current = isAsync; latestMapOutput.current = mapOutput; latestMapOutputError.current = mapOutputError; - isMountedAndNotUnsubscribing.current = true; - - // This has to run after the other ref updates - // to avoid using stale values in the flushed - // callbacks or potentially overwriting a - // changed `latestMapOutput.current`. - if ( latestIsAsync.current !== isAsync ) { - latestIsAsync.current = isAsync; - renderQueue.flush( queueContext ); - } } ); - // Generate a "flag" for used in the effect dependency array. - // It's different than just using `mapSelect` since deps could be undefined, - // in that case, we would still want to memoize it. - const depsChangedFlag = useMemo( () => ( {} ), deps || [] ); + // React can sometimes clear the `useMemo` cache. + // We use the cache-stable `useMemoOne` to avoid + // losing queues. + const queueContext = useMemoOne( () => ( { queue: true } ), [ registry ] ); + const [ , forceRender ] = useReducer( ( s ) => s + 1, 0 ); + const isMounted = useRef( false ); useIsomorphicLayoutEffect( () => { const onStoreChange = () => { - if ( ! isMountedAndNotUnsubscribing.current ) { - return; - } - try { - const newMapOutput = trapSelect( latestMapSelect.current ); + const newMapOutput = wrapSelect( latestMapSelect.current ); if ( isShallowEqual( latestMapOutput.current, newMapOutput ) ) { return; } - latestMapOutput.current = newMapOutput; } catch ( error ) { latestMapOutputError.current = error; @@ -340,6 +333,10 @@ export function useSuspenseSelect( mapSelect, deps ) { }; const onChange = () => { + if ( ! isMounted.current ) { + return; + } + if ( latestIsAsync.current ) { renderQueue.add( queueContext, onStoreChange ); } else { @@ -349,19 +346,21 @@ export function useSuspenseSelect( mapSelect, deps ) { // catch any possible state changes during mount before the subscription // could be set. - onChange(); + onStoreChange(); const unsubscribers = listeningStores.current.map( ( storeName ) => - registry.__experimentalSubscribeStore( storeName, onChange ) + registry.__unstableSubscribeStore( storeName, onChange ) ); + isMounted.current = true; + return () => { - isMountedAndNotUnsubscribing.current = false; // The return value of the subscribe function could be undefined if the store is a custom generic store. unsubscribers.forEach( ( unsubscribe ) => unsubscribe?.() ); - renderQueue.flush( queueContext ); + renderQueue.cancel( queueContext ); + isMounted.current = false; }; - }, [ registry, trapSelect, depsChangedFlag ] ); + }, [ registry, wrapSelect, depsChangedFlag ] ); if ( mapOutputError ) { throw mapOutputError; diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js index 31a1ccd34288f9..a929a1680d2329 100644 --- a/packages/data/src/registry.js +++ b/packages/data/src/registry.js @@ -143,7 +143,7 @@ export function createRegistry( storeConfigs = {}, parent = null ) { const storeName = isObject( storeNameOrDescriptor ) ? storeNameOrDescriptor.name : storeNameOrDescriptor; - __experimentalListeningStores.add( storeName ); + listeningStores.add( storeName ); const store = stores[ storeName ]; if ( store ) { return store.getSuspendSelectors();