Skip to content

[@wordpress/data] Add types to the useSelect function #37239

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.

Sign up for GitHub

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

Open
dmsnell wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

[@wordpress/data] Add types to the useSelect function #37239

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89c7885
Add types to the useSelect function
dmsnell Dec 8, 2021
ab676a3
Assert type cast to preserve existing behaviors
dmsnell Dec 22, 2021
1db052f
Update JSDoc commments to appease docgen
dmsnell Jan 12, 2022
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
Next Next commit
Add types to the useSelect function
  • Loading branch information
@dmsnell
dmsnell committed Mar 18, 2022
commit 89c7885eb1e471cba5bf01604795433585c7a8f7
2 changes: 1 addition & 1 deletion ...ponents/registry-provider/use-registry.js → ...ponents/registry-provider/use-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { Context } from './context';
* };
* ```
*
* @return {Function} A custom react hook exposing the registry context value.
* @return A custom react hook exposing the registry context value.
*/
export default function useRegistry() {
return useContext( Context );
Expand Down
67 changes: 37 additions & 30 deletions ...s/data/src/components/use-select/index.js → ...s/data/src/components/use-select/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,31 @@ import { useIsomorphicLayoutEffect } from '@wordpress/compose';
import useRegistry from '../registry-provider/use-registry';
import useAsyncMode from '../async-mode-provider/use-async-mode';

import type { MapOf, SelectChooser, SelectMapper } from '../../types';

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

/** @typedef {import('../../types').StoreDescriptor} StoreDescriptor */

/**
* Custom react hook for retrieving props from registered selectors.
*
* 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 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 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 @@ -87,9 +87,12 @@ const renderQueue = createQueue();
* }
* ```
*
* @return {Function} A custom react hook.
* @return A custom react hook.
*/
export default function useSelect( mapSelect, deps ) {
export default function useSelect(
mapSelect: SelectChooser,
deps?: unknown[]
): SelectChooser extends SelectMapper ? any : MapOf< Function > {
const hasMappingFunction = 'function' === typeof mapSelect;

// If we're recalling a store by its name or by
Expand All @@ -106,7 +109,7 @@ export default function useSelect( mapSelect, deps ) {
// `_mapSelect` if we can.
const callbackMapper = useCallback(
hasMappingFunction ? mapSelect : noop,
deps
deps ?? []
Copy link
Contributor

@ockham ockham Dec 14, 2021

Choose a reason for hiding this comment

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

This change is (weirdly) causing unit tests to fail 😅

Copy link
Member Author

@dmsnell dmsnell Jan 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed by casting deps

);
const _mapSelect = hasMappingFunction ? callbackMapper : null;

Expand All @@ -118,11 +121,11 @@ export default function useSelect( mapSelect, deps ) {
const queueContext = useMemoOne( () => ( { queue: true } ), [ registry ] );
const [ , forceRender ] = useReducer( ( s ) => s + 1, 0 );

const latestMapSelect = useRef();
const latestMapSelect = useRef< SelectMapper >();
const latestIsAsync = useRef( isAsync );
const latestMapOutput = useRef();
const latestMapOutputError = useRef();
const isMountedAndNotUnsubscribing = useRef();
const latestMapOutput = useRef< unknown >();
const latestMapOutputError = useRef< Error >();
const isMountedAndNotUnsubscribing = useRef( false );

// Keep track of the stores being selected in the _mapSelect function,
// and only subscribe to those stores later.
Expand All @@ -141,7 +144,7 @@ export default function useSelect( mapSelect, deps ) {
// in that case, we would still want to memoize it.
const depsChangedFlag = useMemo( () => ( {} ), deps || [] );

let mapOutput;
let mapOutput: unknown;

if ( _mapSelect ) {
mapOutput = latestMapOutput.current;
Expand All @@ -154,7 +157,9 @@ export default function useSelect( mapSelect, deps ) {
_mapSelect( registry.select, registry )
);
} catch ( error ) {
let errorMessage = `An error occurred while running 'mapSelect': ${ error.message }`;
let errorMessage = `An error occurred while running 'mapSelect': ${
( error as Error ).message
}`;

if ( latestMapOutputError.current ) {
errorMessage += `\nThe error may be correlated with this previous error:\n`;
Expand All @@ -169,7 +174,7 @@ export default function useSelect( mapSelect, deps ) {
}

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

Expand All @@ -189,15 +194,17 @@ export default function useSelect( mapSelect, deps ) {
} );

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

const onStoreChange = () => {
if ( isMountedAndNotUnsubscribing.current ) {
const currentMapSelect = latestMapSelect.current;

if ( isMountedAndNotUnsubscribing.current && currentMapSelect ) {
try {
const newMapOutput = trapSelect( () =>
latestMapSelect.current( registry.select, registry )
currentMapSelect( registry.select, registry )
);

if (
Expand All @@ -207,7 +214,7 @@ export default function useSelect( mapSelect, deps ) {
}
latestMapOutput.current = newMapOutput;
} catch ( error ) {
latestMapOutputError.current = error;
latestMapOutputError.current = error as Error;
}
forceRender();
}
Expand Down
23 changes: 2 additions & 21 deletions packages/data/src/registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,7 @@ import coreDataStore from './store';
import { createEmitter } from './utils/emitter';

/** @typedef {import('./types').StoreDescriptor} StoreDescriptor */

/**
* @typedef {Object} WPDataRegistry An isolated orchestrator of store registrations.
*
* @property {Function} registerGenericStore Given a namespace key and settings
* object, registers a new generic
* store.
* @property {Function} registerStore Given a namespace key and settings
* object, registers a new namespace
* store.
* @property {Function} subscribe Given a function callback, invokes
* the callback on any change to state
* within any registered store.
* @property {Function} select Given a namespace key, returns an
* object of the store's registered
* selectors.
* @property {Function} dispatch Given a namespace key, returns an
* object of the store's registered
* action dispatchers.
*/
/** @typedef {import('./types').DataRegistry} DataRegistry */

/**
* @typedef {Object} WPDataPlugin An object of registry function overrides.
Expand All @@ -50,7 +31,7 @@ import { createEmitter } from './utils/emitter';
* @param {Object} storeConfigs Initial store configurations.
* @param {Object?} parent Parent registry.
*
* @return {WPDataRegistry} Data registry.
* @return {DataRegistry} Data registry.
*/
export function createRegistry( storeConfigs = {}, parent = null ) {
const stores = {};
Expand Down
27 changes: 26 additions & 1 deletion packages/data/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
type MapOf< T > = { [ name: string ]: T };
/**
* External dependencies
*/
// eslint-disable-next-line no-restricted-imports
import type { MutableRefObject } from 'react';

export type MapOf< T > = { [ name: string ]: T };

export type ActionCreator = Function | Generator;
export type Resolver = Function | Generator;
export type Selector = Function;

export type AnyConfig = ReduxStoreConfig< any, any, any >;

export interface SelectMapper {
(
select: ( reference: StoreReference ) => MapOf< Function >,
registry: DataRegistry
): unknown;
}
export type SelectChooser = SelectMapper | StoreReference;
Copy link
Contributor

@youknowriad youknowriad Jan 12, 2022

Choose a reason for hiding this comment

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

This looks like something very specific to the useSelect hook and not a "meaningful" type. It's just a nitpick and I don't really have experience with typescript much but I'd have avoided a named typed for this one and just inlined it personally.

Copy link
Member Author

@dmsnell dmsnell Jan 19, 2022

Choose a reason for hiding this comment

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

this appears again in the type for DataRegistry for the select function. I was going to move it but forgot about that one. in time I'd like to come up with some better descriptions and names for these but I think for now at least we need to keep it here or duplicate the definition (which I'd rather avoid) so we don't cause false negatives on registry objects.

export type StoreReference = StoreDescriptor< any > | string;

export interface StoreInstance< Config extends AnyConfig > {
getSelectors: () => SelectorsOf< Config >;
getActions: () => ActionCreatorsOf< Config >;
Expand Down Expand Up @@ -38,7 +53,17 @@ export interface ReduxStoreConfig<
}

export interface DataRegistry {
__experimentalMarkListeningStores: (
callback: ( this: DataRegistry ) => unknown,
listeningStores: MutableRefObject< unknown >
) => unknown;
__experimentalSubscribeStore: (
storeName: string,
listener: () => void
) => ReturnType< DataEmitter[ 'subscribe' ] >;

register: ( store: StoreDescriptor< any > ) => void;
select: ( chooser: SelectChooser, deps?: unknown[] ) => any;
}

export interface DataEmitter {
Expand Down
4 changes: 2 additions & 2 deletions packages/is-shallow-equal/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export { default as isShallowEqualArrays } from './arrays';
* Returns true if the two arrays or objects are shallow equal, or false
* otherwise.
*
* @param {any[]|ComparableObject} a First object or array to compare.
* @param {any[]|ComparableObject} b Second object or array to compare.
* @param {unknown[]|ComparableObject|any} a First object or array to compare.
* @param {unknown[]|ComparableObject|any} b Second object or array to compare.
*
* @return {boolean} Whether the two values are shallow equal.
*/
Expand Down