Skip to content

Commit

Permalink
[@wordpress/data]: Refactor use-select in preparation for adding types (
Browse files Browse the repository at this point in the history 
#37017)

In this patch we're making what should be a few minor changes to `use-select`
as we get ready to add types to the data package. Primarily:

 - Inverting a double-negative into a positive assertion
 - Creating an intermediate variable for the `useCallback` that
   might be given somethign that isn't a function.

There shouldn't be any impact on the behavior of the package.
  • Loading branch information
@dmsnell
dmsnell authored Dec 7, 2021
1 parent ac7b189 commit bd827c0
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 46 deletions.
2 changes: 1 addition & 1 deletion packages/data/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,7 @@ function Paste( { children } ) {

_Parameters_

- _\_mapSelect_ `Function|StoreDescriptor|string`: Function called on every state change. The returned value is exposed to the component implementing this hook. The function receives the `registry.select` method on the first argument and the `registry` on the second argument. When a store key is passed, all selectors for the store will be returned. This is only meant for usage of these selectors in event callbacks, not for data needed to create the element tree.
- _mapSelect_ `Function|StoreDescriptor|string`: Function called on every state change. The returned value is exposed to the component implementing this hook. The function receives the `registry.select` method on the first argument and the `registry` on the second argument. When a store key is passed, all selectors for the store will be returned. This is only meant for usage of these selectors in event callbacks, not for data needed to create the element tree.
- _deps_ `Array`: If provided, this memoizes the mapSelect so the same `mapSelect` is invoked on every state change unless the dependencies change.

_Returns_
Expand Down
105 changes: 60 additions & 45 deletions packages/data/src/components/use-select/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { useMemoOne } from 'use-memo-one';
* WordPress dependencies
*/
import { createQueue } from '@wordpress/priority-queue';
import { useRef, useCallback, useReducer, useMemo } from '@wordpress/element';
import { useRef, useCallback, useMemo, useReducer } from '@wordpress/element';
import isShallowEqual from '@wordpress/is-shallow-equal';
import { useIsomorphicLayoutEffect } from '@wordpress/compose';

Expand All @@ -17,6 +17,7 @@ import { useIsomorphicLayoutEffect } from '@wordpress/compose';
import useRegistry from '../registry-provider/use-registry';
import useAsyncMode from '../async-mode-provider/use-async-mode';

const noop = () => {};
const renderQueue = createQueue();

/** @typedef {import('../../types').StoreDescriptor} StoreDescriptor */
Expand All @@ -27,20 +28,20 @@ const renderQueue = createQueue();
* In general, this custom React hook follows the
* [rules of hooks](https://reactjs.org/docs/hooks-rules.html).
*
* @param {Function|StoreDescriptor|string} _mapSelect Function called on every state change. The
* returned value is exposed to the component
* implementing this hook. The function receives
* the `registry.select` method on the first
* argument and the `registry` on the second
* argument.
* When a store key is passed, all selectors for
* the store will be returned. This is only meant
* for usage of these selectors in event
* callbacks, not for data needed to create the
* element tree.
* @param {Array} deps If provided, this memoizes the mapSelect so the
* same `mapSelect` is invoked on every state
* change unless the dependencies change.
* @param {Function|StoreDescriptor|string} mapSelect Function called on every state change. The
* returned value is exposed to the component
* implementing this hook. The function receives
* the `registry.select` method on the first
* argument and the `registry` on the second
* argument.
* When a store key is passed, all selectors for
* the store will be returned. This is only meant
* for usage of these selectors in event
* callbacks, not for data needed to create the
* element tree.
* @param {Array} deps If provided, this memoizes the mapSelect so the
* same `mapSelect` is invoked on every state
* change unless the dependencies change.
*
* @example
* ```js
Expand Down Expand Up @@ -88,14 +89,27 @@ const renderQueue = createQueue();
*
* @return {Function} A custom react hook.
*/
export default function useSelect( _mapSelect, deps ) {
const isWithoutMapping = typeof _mapSelect !== 'function';
export default function useSelect( mapSelect, deps ) {
const hasMappingFunction = 'function' === typeof mapSelect;

if ( isWithoutMapping ) {
// If we're recalling a store by its name or by
// its descriptor then we won't be caching the
// calls to `mapSelect` because we won't be calling it.
if ( ! hasMappingFunction ) {
deps = [];
}

const mapSelect = useCallback( _mapSelect, deps );
// Because of the "rule of hooks" we have to call `useCallback`
// on every invocation whether or not we have a real function
// for `mapSelect`. we'll create this intermediate variable to
// fulfill that need and then reference it with our "real"
// `_mapSelect` if we can.
const callbackMapper = useCallback(
hasMappingFunction ? mapSelect : noop,
deps
);
const _mapSelect = hasMappingFunction ? callbackMapper : null;

const registry = useRegistry();
const isAsync = useAsyncMode();
// React can sometimes clear the `useMemo` cache.
Expand All @@ -110,7 +124,7 @@ export default function useSelect( _mapSelect, deps ) {
const latestMapOutputError = useRef();
const isMountedAndNotUnsubscribing = useRef();

// Keep track of the stores being selected in the mapSelect function,
// Keep track of the stores being selected in the _mapSelect function,
// and only subscribe to those stores later.
const listeningStores = useRef( [] );
const trapSelect = useCallback(
Expand All @@ -129,39 +143,37 @@ export default function useSelect( _mapSelect, deps ) {

let mapOutput;

if ( ! isWithoutMapping ) {
try {
if (
latestMapSelect.current !== mapSelect ||
latestMapOutputError.current
) {
if ( _mapSelect ) {
mapOutput = latestMapOutput.current;
const hasReplacedMapSelect = latestMapSelect.current !== _mapSelect;
const lastMapSelectFailed = !! latestMapOutputError.current;

if ( hasReplacedMapSelect || lastMapSelectFailed ) {
try {
mapOutput = trapSelect( () =>
mapSelect( registry.select, registry )
_mapSelect( registry.select, registry )
);
} else {
mapOutput = latestMapOutput.current;
}
} catch ( error ) {
let errorMessage = `An error occurred while running 'mapSelect': ${ error.message }`;
} catch ( error ) {
let errorMessage = `An error occurred while running 'mapSelect': ${ error.message }`;

if ( latestMapOutputError.current ) {
errorMessage += `\nThe error may be correlated with this previous error:\n`;
errorMessage += `${ latestMapOutputError.current.stack }\n\n`;
errorMessage += 'Original stack trace:';
}
if ( latestMapOutputError.current ) {
errorMessage += `\nThe error may be correlated with this previous error:\n`;
errorMessage += `${ latestMapOutputError.current.stack }\n\n`;
errorMessage += 'Original stack trace:';
}

// eslint-disable-next-line no-console
console.error( errorMessage );
mapOutput = latestMapOutput.current;
// eslint-disable-next-line no-console
console.error( errorMessage );
}
}
}

useIsomorphicLayoutEffect( () => {
if ( isWithoutMapping ) {
if ( ! hasMappingFunction ) {
return;
}

latestMapSelect.current = mapSelect;
latestMapSelect.current = _mapSelect;
latestMapOutput.current = mapOutput;
latestMapOutputError.current = undefined;
isMountedAndNotUnsubscribing.current = true;
Expand All @@ -177,7 +189,7 @@ export default function useSelect( _mapSelect, deps ) {
} );

useIsomorphicLayoutEffect( () => {
if ( isWithoutMapping ) {
if ( ! hasMappingFunction ) {
return;
}

Expand Down Expand Up @@ -227,7 +239,10 @@ export default function useSelect( _mapSelect, deps ) {
unsubscribers.forEach( ( unsubscribe ) => unsubscribe?.() );
renderQueue.flush( queueContext );
};
}, [ registry, trapSelect, depsChangedFlag, isWithoutMapping ] );
// If you're tempted to eliminate the spread dependencies below don't do it!
// We're passing these in from the calling function and want to make sure we're
// examining every individual value inside the `deps` array.
}, [ registry, trapSelect, hasMappingFunction, depsChangedFlag ] );

return isWithoutMapping ? registry.select( _mapSelect ) : mapOutput;
return hasMappingFunction ? mapOutput : registry.select( mapSelect );
}

0 comments on commit bd827c0

Please sign in to comment.