From 69f12f853c46deda9d107cd6f080e6cc0f8c7fee Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 18 May 2018 03:10:34 -0400 Subject: [PATCH] Data: Refactor resolver fulfillment / request progress as data state (#6600) * Data: Refactor resolver fulfillment / request progress as data state * Data: Ensure resolution finishes for varied resolver shapes --- core-data/actions.js | 17 ----- core-data/index.js | 9 ++- core-data/reducer.js | 11 ---- core-data/resolvers.js | 2 - core-data/selectors.js | 29 ++++++++- core-data/test/reducer.js | 39 ------------ core-data/test/resolvers.js | 4 +- core-data/test/selectors.js | 54 +++++++++------- data/index.js | 46 +++++--------- data/store/actions.js | 37 +++++++++++ data/store/index.js | 16 +++++ data/store/reducer.js | 38 +++++++++++ data/store/selectors.js | 71 +++++++++++++++++++++ data/store/test/reducer.js | 47 ++++++++++++++ data/store/test/selectors.js | 120 +++++++++++++++++++++++++++++++++++ data/store/test/utils.js | 39 ++++++++++++ data/store/utils.js | 28 ++++++++ data/test/index.js | 117 +++++++++++++++++++--------------- package-lock.json | 6 +- package.json | 2 +- 20 files changed, 547 insertions(+), 185 deletions(-) create mode 100644 data/store/actions.js create mode 100644 data/store/index.js create mode 100644 data/store/reducer.js create mode 100644 data/store/selectors.js create mode 100644 data/store/test/reducer.js create mode 100644 data/store/test/selectors.js create mode 100644 data/store/test/utils.js create mode 100644 data/store/utils.js diff --git a/core-data/actions.js b/core-data/actions.js index 4730edd73dc33..94ba26f21581f 100644 --- a/core-data/actions.js +++ b/core-data/actions.js @@ -3,23 +3,6 @@ */ import { castArray } from 'lodash'; -/** - * Returns an action object used in signalling that the request for a given - * data type has been made. - * - * @param {string} dataType Data type requested. - * @param {?string} subType Optional data sub-type. - * - * @return {Object} Action object. - */ -export function setRequested( dataType, subType ) { - return { - type: 'SET_REQUESTED', - dataType, - subType, - }; -} - /** * Returns an action object used in signalling that terms have been received * for a given taxonomy. diff --git a/core-data/index.js b/core-data/index.js index 195e0db4e6efd..87f51853b0d4d 100644 --- a/core-data/index.js +++ b/core-data/index.js @@ -12,6 +12,13 @@ import * as actions from './actions'; import * as resolvers from './resolvers'; import { default as entities, getMethodName } from './entities'; +/** + * The reducer key used by core data in store registration. + * + * @type {string} + */ +export const REDUCER_KEY = 'core'; + const createEntityRecordGetter = ( source ) => entities.reduce( ( result, entity ) => { const { kind, name } = entity; const methodName = getMethodName( kind, name ); @@ -22,7 +29,7 @@ const createEntityRecordGetter = ( source ) => entities.reduce( ( result, entity const entityResolvers = createEntityRecordGetter( resolvers ); const entitySelectors = createEntityRecordGetter( selectors ); -const store = registerStore( 'core', { +const store = registerStore( REDUCER_KEY, { reducer, actions, selectors: { ...selectors, ...entitySelectors }, diff --git a/core-data/reducer.js b/core-data/reducer.js index ea86b96907f60..12c83545ffccb 100644 --- a/core-data/reducer.js +++ b/core-data/reducer.js @@ -31,17 +31,6 @@ export function terms( state = {}, action ) { ...state, [ action.taxonomy ]: action.terms, }; - - case 'SET_REQUESTED': - const { dataType, subType: taxonomy } = action; - if ( dataType !== 'terms' || state.hasOwnProperty( taxonomy ) ) { - return state; - } - - return { - ...state, - [ taxonomy ]: null, - }; } return state; diff --git a/core-data/resolvers.js b/core-data/resolvers.js index 96cc471c06e5b..8efe1eb9b777b 100644 --- a/core-data/resolvers.js +++ b/core-data/resolvers.js @@ -7,7 +7,6 @@ import apiRequest from '@wordpress/api-request'; * Internal dependencies */ import { - setRequested, receiveTerms, receiveUserQuery, receiveEntityRecords, @@ -21,7 +20,6 @@ import { getEntity } from './entities'; * progress. */ export async function* getCategories() { - yield setRequested( 'terms', 'categories' ); const categories = await apiRequest( { path: '/wp/v2/categories?per_page=-1' } ); yield receiveTerms( 'categories', categories ); } diff --git a/core-data/selectors.js b/core-data/selectors.js index 4cc78cb7c2321..c56c096ba5cc6 100644 --- a/core-data/selectors.js +++ b/core-data/selectors.js @@ -4,6 +4,29 @@ import createSelector from 'rememo'; import { map } from 'lodash'; +/** + * WordPress dependencies + */ +import { select } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { REDUCER_KEY } from './'; + +/** + * Returns true if resolution is in progress for the core selector of the given + * name and arguments. + * + * @param {string} selectorName Core data selector name. + * @param {...*} args Arguments passed to selector. + * + * @return {boolean} Whether resolution is in progress. + */ +function isResolving( selectorName, ...args ) { + return select( 'core/data' ).isResolving( REDUCER_KEY, selectorName, ...args ); +} + /** * Returns all the available terms for the given taxonomy. * @@ -37,7 +60,7 @@ export function getCategories( state ) { * @return {boolean} Whether a request is in progress for taxonomy's terms. */ export function isRequestingTerms( state, taxonomy ) { - return state.terms[ taxonomy ] === null; + return isResolving( 'getTerms', taxonomy ); } /** @@ -48,8 +71,8 @@ export function isRequestingTerms( state, taxonomy ) { * * @return {boolean} Whether a request is in progress for categories. */ -export function isRequestingCategories( state ) { - return isRequestingTerms( state, 'categories' ); +export function isRequestingCategories() { + return isResolving( 'getCategories' ); } /** diff --git a/core-data/test/reducer.js b/core-data/test/reducer.js index 60b4564d14f91..5d608b025910c 100644 --- a/core-data/test/reducer.js +++ b/core-data/test/reducer.js @@ -27,45 +27,6 @@ describe( 'terms()', () => { categories: [ { id: 1 } ], } ); } ); - - it( 'assigns requested taxonomy to null', () => { - const originalState = deepFreeze( {} ); - const state = terms( originalState, { - type: 'SET_REQUESTED', - dataType: 'terms', - subType: 'categories', - } ); - - expect( state ).toEqual( { - categories: null, - } ); - } ); - - it( 'does not assign requested taxonomy to null if received', () => { - const originalState = deepFreeze( { - categories: [ { id: 1 } ], - } ); - const state = terms( originalState, { - type: 'SET_REQUESTED', - dataType: 'terms', - subType: 'categories', - } ); - - expect( state ).toEqual( { - categories: [ { id: 1 } ], - } ); - } ); - - it( 'does not assign requested taxonomy if not terms data type', () => { - const originalState = deepFreeze( {} ); - const state = terms( originalState, { - type: 'SET_REQUESTED', - dataType: 'foo', - subType: 'categories', - } ); - - expect( state ).toEqual( {} ); - } ); } ); describe( 'entities', () => { diff --git a/core-data/test/resolvers.js b/core-data/test/resolvers.js index 3c4bcca1a7d19..fd996fc261cf5 100644 --- a/core-data/test/resolvers.js +++ b/core-data/test/resolvers.js @@ -7,7 +7,7 @@ import apiRequest from '@wordpress/api-request'; * Internal dependencies */ import { getCategories, getEntityRecord } from '../resolvers'; -import { setRequested, receiveTerms, receiveEntityRecords } from '../actions'; +import { receiveTerms, receiveEntityRecords } from '../actions'; jest.mock( '@wordpress/api-request' ); @@ -24,8 +24,6 @@ describe( 'getCategories', () => { it( 'yields with requested terms', async () => { const fulfillment = getCategories(); - const requested = ( await fulfillment.next() ).value; - expect( requested.type ).toBe( setRequested().type ); const received = ( await fulfillment.next() ).value; expect( received ).toEqual( receiveTerms( 'categories', CATEGORIES ) ); } ); diff --git a/core-data/test/selectors.js b/core-data/test/selectors.js index 12ab66bcea356..28aea5c40971a 100644 --- a/core-data/test/selectors.js +++ b/core-data/test/selectors.js @@ -6,7 +6,13 @@ import deepFreeze from 'deep-freeze'; /** * Internal dependencies */ -import { getTerms, isRequestingTerms, getEntityRecord } from '../selectors'; +import { getTerms, isRequestingCategories, getEntityRecord } from '../selectors'; +import { select } from '@wordpress/data'; + +jest.mock( '@wordpress/data', () => ( { + ...require.requireActual( '@wordpress/data' ), + select: jest.fn().mockReturnValue( {} ), +} ) ); describe( 'getTerms()', () => { it( 'returns value of terms by taxonomy', () => { @@ -24,35 +30,39 @@ describe( 'getTerms()', () => { } ); } ); -describe( 'isRequestingTerms()', () => { - it( 'returns false if never requested', () => { - const state = deepFreeze( { - terms: {}, - } ); +describe( 'isRequestingCategories()', () => { + beforeAll( () => { + select( 'core/data' ).isResolving = jest.fn().mockReturnValue( false ); + } ); - const result = isRequestingTerms( state, 'categories' ); - expect( result ).toBe( false ); + afterAll( () => { + select( 'core/data' ).isResolving.mockRestore(); } ); - it( 'returns false if terms received', () => { - const state = deepFreeze( { - terms: { - categories: [ { id: 1 } ], - }, - } ); + function setIsResolving( isResolving ) { + select( 'core/data' ).isResolving.mockImplementation( + ( reducerKey, selectorName ) => ( + isResolving && + reducerKey === 'core' && + selectorName === 'getCategories' + ) + ); + } - const result = isRequestingTerms( state, 'categories' ); + it( 'returns false if never requested', () => { + const result = isRequestingCategories(); expect( result ).toBe( false ); } ); - it( 'returns true if requesting', () => { - const state = deepFreeze( { - terms: { - categories: null, - }, - } ); + it( 'returns false if categories resolution finished', () => { + setIsResolving( false ); + const result = isRequestingCategories(); + expect( result ).toBe( false ); + } ); - const result = isRequestingTerms( state, 'categories' ); + it( 'returns true if categories resolution started', () => { + setIsResolving( true ); + const result = isRequestingCategories(); expect( result ).toBe( true ); } ); } ); diff --git a/data/index.js b/data/index.js index eaafa51cd4d3c..5b441a912bb87 100644 --- a/data/index.js +++ b/data/index.js @@ -3,7 +3,6 @@ */ import { combineReducers, createStore } from 'redux'; import { flowRight, without, mapValues, overEvery } from 'lodash'; -import EquivalentKeyMap from 'equivalent-key-map'; /** * WordPress dependencies @@ -14,6 +13,8 @@ import isShallowEqual from '@wordpress/is-shallow-equal'; /** * Internal dependencies */ +import registerDataStore from './store'; + export { loadAndPersist, withRehydratation } from './persist'; /** @@ -130,51 +131,30 @@ export function registerSelectors( reducerKey, newSelectors ) { * @param {Object} newResolvers Resolvers to register. */ export function registerResolvers( reducerKey, newResolvers ) { - const createResolver = ( selector, key ) => { + const { hasStartedResolution } = select( 'core/data' ); + const { startResolution, finishResolution } = dispatch( 'core/data' ); + + const createResolver = ( selector, selectorName ) => { // Don't modify selector behavior if no resolver exists. - if ( ! newResolvers.hasOwnProperty( key ) ) { + if ( ! newResolvers.hasOwnProperty( selectorName ) ) { return selector; } const store = stores[ reducerKey ]; // Normalize resolver shape to object. - let resolver = newResolvers[ key ]; + let resolver = newResolvers[ selectorName ]; if ( ! resolver.fulfill ) { resolver = { fulfill: resolver }; } - /** - * To ensure that fulfillment occurs only once per arguments set - * (even for deeply "equivalent" arguments), track calls. - * - * @type {EquivalentKeyMap} - */ - const fulfilledByEquivalentArgs = new EquivalentKeyMap(); - - /** - * Returns true if resolver fulfillment has already occurred for an - * equivalent set of arguments. Includes side effect when returning - * false to ensure the next invocation returns true. - * - * @param {Array} args Arguments set. - * - * @return {boolean} Whether fulfillment has already occurred. - */ - function hasBeenFulfilled( args ) { - const hasArguments = fulfilledByEquivalentArgs.has( args ); - if ( ! hasArguments ) { - fulfilledByEquivalentArgs.set( args, true ); - } - - return hasArguments; - } - async function fulfill( ...args ) { - if ( hasBeenFulfilled( args ) ) { + if ( hasStartedResolution( reducerKey, selectorName, args ) ) { return; } + startResolution( reducerKey, selectorName, args ); + // At this point, selectors have already been pre-bound to inject // state, it would not be otherwise provided to fulfill. const state = store.getState(); @@ -193,6 +173,8 @@ export function registerResolvers( reducerKey, newResolvers ) { store.dispatch( maybeAction ); } } + + finishResolution( reducerKey, selectorName, args ); } if ( typeof resolver.isFulfilled === 'function' ) { @@ -485,3 +467,5 @@ export function toAsyncIterable( object ) { } }() ); } + +registerDataStore(); diff --git a/data/store/actions.js b/data/store/actions.js new file mode 100644 index 0000000000000..a386aae4eca61 --- /dev/null +++ b/data/store/actions.js @@ -0,0 +1,37 @@ +/** + * Returns an action object used in signalling that selector resolution has + * started. + * + * @param {string} reducerKey Registered store reducer key. + * @param {string} selectorName Name of selector for which resolver triggered. + * @param {...*} args Arguments to associate for uniqueness. + * + * @return {Object} Action object. + */ +export function startResolution( reducerKey, selectorName, args ) { + return { + type: 'START_RESOLUTION', + reducerKey, + selectorName, + args, + }; +} + +/** + * Returns an action object used in signalling that selector resolution has + * completed. + * + * @param {string} reducerKey Registered store reducer key. + * @param {string} selectorName Name of selector for which resolver triggered. + * @param {...*} args Arguments to associate for uniqueness. + * + * @return {Object} Action object. + */ +export function finishResolution( reducerKey, selectorName, args ) { + return { + type: 'FINISH_RESOLUTION', + reducerKey, + selectorName, + args, + }; +} diff --git a/data/store/index.js b/data/store/index.js new file mode 100644 index 0000000000000..417babf5a51a8 --- /dev/null +++ b/data/store/index.js @@ -0,0 +1,16 @@ +/** + * Internal dependencies + */ +import { registerStore } from '../'; + +import reducer from './reducer'; +import * as selectors from './selectors'; +import * as actions from './actions'; + +export default function registerDataStore() { + registerStore( 'core/data', { + reducer, + actions, + selectors, + } ); +} diff --git a/data/store/reducer.js b/data/store/reducer.js new file mode 100644 index 0000000000000..378535ce44972 --- /dev/null +++ b/data/store/reducer.js @@ -0,0 +1,38 @@ +/** + * External dependencies + */ +import { flowRight } from 'lodash'; +import EquivalentKeyMap from 'equivalent-key-map'; + +/** + * Internal dependencies + */ +import { onSubKey } from './utils'; + +/** + * Reducer function returning next state for selector resolution, object form: + * + * reducerKey -> selectorName -> EquivalentKeyMap + * + * @param {Object} state Current state. + * @param {Object} action Dispatched action. + * + * @returns {Object} Next state. + */ +const isResolved = flowRight( [ + onSubKey( 'reducerKey' ), + onSubKey( 'selectorName' ), +] )( ( state = new EquivalentKeyMap(), action ) => { + switch ( action.type ) { + case 'START_RESOLUTION': + case 'FINISH_RESOLUTION': + const isStarting = action.type === 'START_RESOLUTION'; + const nextState = new EquivalentKeyMap( state ); + nextState.set( action.args, isStarting ); + return nextState; + } + + return state; +} ); + +export default isResolved; diff --git a/data/store/selectors.js b/data/store/selectors.js new file mode 100644 index 0000000000000..8f5c9d18de2fe --- /dev/null +++ b/data/store/selectors.js @@ -0,0 +1,71 @@ +/** + * External dependencies + */ +import { get } from 'lodash'; + +/** + * Returns the raw `isResolving` value for a given reducer key, selector name, + * and arguments set. May be undefined if the selector has never been resolved + * or not resolved for the given set of arguments, otherwise true or false for + * resolution started and completed respectively. + * + * @param {Object} state Data state. + * @param {string} reducerKey Registered store reducer key. + * @param {string} selectorName Selector name. + * @param {Array} args Arguments passed to selector. + * + * @return {?boolean} isResolving value. + */ +export function getIsResolving( state, reducerKey, selectorName, args ) { + const map = get( state, [ reducerKey, selectorName ] ); + if ( ! map ) { + return; + } + + return map.get( args ); +} + +/** + * Returns true if resolution has already been triggered for a given reducer + * key, selector name, and arguments set. + * + * @param {Object} state Data state. + * @param {string} reducerKey Registered store reducer key. + * @param {string} selectorName Selector name. + * @param {?Array} args Arguments passed to selector (default `[]`). + * + * @return {boolean} Whether resolution has been triggered. + */ +export function hasStartedResolution( state, reducerKey, selectorName, args = [] ) { + return getIsResolving( state, reducerKey, selectorName, args ) !== undefined; +} + +/** + * Returns true if resolution has completed for a given reducer key, selector + * name, and arguments set. + * + * @param {Object} state Data state. + * @param {string} reducerKey Registered store reducer key. + * @param {string} selectorName Selector name. + * @param {?Array} args Arguments passed to selector. + * + * @return {boolean} Whether resolution has completed. + */ +export function hasFinishedResolution( state, reducerKey, selectorName, args = [] ) { + return getIsResolving( state, reducerKey, selectorName, args ) === false; +} + +/** + * Returns true if resolution has been triggered but has not yet completed for + * a given reducer key, selector name, and arguments set. + * + * @param {Object} state Data state. + * @param {string} reducerKey Registered store reducer key. + * @param {string} selectorName Selector name. + * @param {?Array} args Arguments passed to selector. + * + * @return {boolean} Whether resolution is in progress. + */ +export function isResolving( state, reducerKey, selectorName, args = [] ) { + return getIsResolving( state, reducerKey, selectorName, args ) === true; +} diff --git a/data/store/test/reducer.js b/data/store/test/reducer.js new file mode 100644 index 0000000000000..4844420dab750 --- /dev/null +++ b/data/store/test/reducer.js @@ -0,0 +1,47 @@ +/** + * External dependencies + */ +import deepFreeze from 'deep-freeze'; + +/** + * Internal dependencies + */ +import reducer from '../reducer'; + +describe( 'reducer', () => { + it( 'should default to an empty object', () => { + const state = reducer( undefined, {} ); + + expect( state ).toEqual( {} ); + } ); + + it( 'should return with started resolution', () => { + const state = reducer( undefined, { + type: 'START_RESOLUTION', + reducerKey: 'test', + selectorName: 'getFoo', + args: [], + } ); + + // { test: { getFoo: EquivalentKeyMap( [] => true ) } } + expect( state.test.getFoo.get( [] ) ).toBe( true ); + } ); + + it( 'should return with finished resolution', () => { + const original = reducer( undefined, { + type: 'START_RESOLUTION', + reducerKey: 'test', + selectorName: 'getFoo', + args: [], + } ); + const state = reducer( deepFreeze( original ), { + type: 'FINISH_RESOLUTION', + reducerKey: 'test', + selectorName: 'getFoo', + args: [], + } ); + + // { test: { getFoo: EquivalentKeyMap( [] => false ) } } + expect( state.test.getFoo.get( [] ) ).toBe( false ); + } ); +} ); diff --git a/data/store/test/selectors.js b/data/store/test/selectors.js new file mode 100644 index 0000000000000..1af4bf9b8adf1 --- /dev/null +++ b/data/store/test/selectors.js @@ -0,0 +1,120 @@ +/** + * External dependencies + */ +import EquivalentKeyMap from 'equivalent-key-map'; + +/** + * Internal dependencies + */ +import { + getIsResolving, + hasStartedResolution, + hasFinishedResolution, + isResolving, +} from '../selectors'; + +describe( 'getIsResolving', () => { + it( 'should return undefined if no state by reducerKey, selectorName', () => { + const state = {}; + const result = getIsResolving( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( undefined ); + } ); + + it( 'should return undefined if state by reducerKey, selectorName, but not args', () => { + const state = { + test: { + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), + }, + }; + const result = getIsResolving( state, 'test', 'getFoo', [ 'bar' ] ); + + expect( result ).toBe( undefined ); + } ); + + it( 'should return value by reducerKey, selectorName', () => { + const state = { + test: { + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), + }, + }; + const result = getIsResolving( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( true ); + } ); +} ); + +describe( 'hasStartedResolution', () => { + it( 'returns false if not has started', () => { + const state = {}; + const result = hasStartedResolution( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( false ); + } ); + + it( 'returns true if has started', () => { + const state = { + test: { + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), + }, + }; + const result = hasStartedResolution( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( true ); + } ); +} ); + +describe( 'hasFinishedResolution', () => { + it( 'returns false if not has finished', () => { + const state = { + test: { + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), + }, + }; + const result = hasFinishedResolution( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( false ); + } ); + + it( 'returns true if has finished', () => { + const state = { + test: { + getFoo: new EquivalentKeyMap( [ [ [], false ] ] ), + }, + }; + const result = hasFinishedResolution( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( true ); + } ); +} ); + +describe( 'isResolving', () => { + it( 'returns false if not has started', () => { + const state = {}; + const result = isResolving( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( false ); + } ); + + it( 'returns false if has finished', () => { + const state = { + test: { + getFoo: new EquivalentKeyMap( [ [ [], false ] ] ), + }, + }; + const result = isResolving( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( false ); + } ); + + it( 'returns true if has started but not finished', () => { + const state = { + test: { + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), + }, + }; + const result = isResolving( state, 'test', 'getFoo', [] ); + + expect( result ).toBe( true ); + } ); +} ); diff --git a/data/store/test/utils.js b/data/store/test/utils.js new file mode 100644 index 0000000000000..97490250d4d3a --- /dev/null +++ b/data/store/test/utils.js @@ -0,0 +1,39 @@ +/** + * Internal dependencies + */ +import { onSubKey } from '../utils'; + +describe( 'onSubKey', () => { + function createEnhancedReducer( actionProperty ) { + const enhanceReducer = onSubKey( actionProperty ); + return enhanceReducer( ( state, action ) => 'Called by ' + action.caller ); + } + + it( 'should default to an empty object', () => { + const reducer = createEnhancedReducer( 'caller' ); + const nextState = reducer( undefined, { type: '@@INIT' } ); + + expect( nextState ).toEqual( {} ); + } ); + + it( 'should ignore actions where property not present', () => { + const state = {}; + const reducer = createEnhancedReducer( 'caller' ); + const nextState = reducer( state, { type: 'DO_FOO' } ); + + expect( nextState ).toBe( state ); + } ); + + it( 'should key by action property', () => { + const reducer = createEnhancedReducer( 'caller' ); + + let state = Object.freeze( {} ); + state = reducer( state, { type: 'DO_FOO', caller: 1 } ); + state = reducer( state, { type: 'DO_FOO', caller: 2 } ); + + expect( state ).toEqual( { + 1: 'Called by 1', + 2: 'Called by 2', + } ); + } ); +} ); diff --git a/data/store/utils.js b/data/store/utils.js new file mode 100644 index 0000000000000..d793855683d65 --- /dev/null +++ b/data/store/utils.js @@ -0,0 +1,28 @@ +/** + * Higher-order reducer creator which creates a combined reducer object, keyed + * by a property on the action object. + * + * @param {string} actionProperty Action property by which to key object. + * + * @return {Function} Higher-order reducer. + */ +export const onSubKey = ( actionProperty ) => ( reducer ) => ( state = {}, action ) => { + // Retrieve subkey from action. Do not track if undefined; useful for cases + // where reducer is scoped by action shape. + const key = action[ actionProperty ]; + if ( key === undefined ) { + return state; + } + + // Avoid updating state if unchanged. Note that this also accounts for a + // reducer which returns undefined on a key which is not yet tracked. + const nextKeyState = reducer( state[ key ], action ); + if ( nextKeyState === state[ key ] ) { + return state; + } + + return { + ...state, + [ key ]: nextKeyState, + }; +}; diff --git a/data/test/index.js b/data/test/index.js index d48bc13720544..7042011e9b006 100644 --- a/data/test/index.js +++ b/data/test/index.js @@ -2,6 +2,7 @@ * External dependencies */ import { mount } from 'enzyme'; +import { castArray } from 'lodash'; /** * WordPress dependencies @@ -32,6 +33,11 @@ jest.mock( '@wordpress/utils', () => ( { deprecated: jest.fn(), } ) ); +// Mock data store to prevent self-initialization, as it needs to be reset +// between tests of `registerResolvers` by replacement (new `registerStore`). +jest.mock( '../store', () => () => {} ); +const registerDataStore = require.requireActual( '../store' ).default; + describe( 'registerStore', () => { it( 'should be shorthand for reducer, actions, selectors registration', () => { const store = registerStore( 'butcher', { @@ -79,6 +85,10 @@ describe( 'registerReducer', () => { } ); describe( 'registerResolvers', () => { + beforeEach( () => { + registerDataStore(); + } ); + const unsubscribes = []; afterEach( () => { let unsubscribe; @@ -93,6 +103,18 @@ describe( 'registerResolvers', () => { return unsubscribe; } + function subscribeUntil( predicates ) { + predicates = castArray( predicates ); + + return new Promise( ( resolve ) => { + subscribeWithUnsubscribe( () => { + if ( predicates.every( ( predicate ) => predicate() ) ) { + resolve(); + } + } ); + } ); + } + it( 'should not do anything for selectors which do not have resolvers', () => { registerReducer( 'demo', ( state = 'OK' ) => state ); registerSelectors( 'demo', { @@ -197,7 +219,7 @@ describe( 'registerResolvers', () => { select( 'demo' ).getPage( 4, {} ); } ); - it( 'should resolve action to dispatch', ( done ) => { + it( 'should resolve action to dispatch', () => { registerReducer( 'demo', ( state = 'NOTOK', action ) => { return action.type === 'SET_OK' ? 'OK' : state; } ); @@ -208,19 +230,17 @@ describe( 'registerResolvers', () => { getValue: () => ( { type: 'SET_OK' } ), } ); - subscribeWithUnsubscribe( () => { - try { - expect( select( 'demo' ).getValue() ).toBe( 'OK' ); - done(); - } catch ( error ) { - done( error ); - } - } ); + const promise = subscribeUntil( [ + () => select( 'demo' ).getValue() === 'OK', + () => select( 'core/data' ).hasFinishedResolution( 'demo', 'getValue' ), + ] ); select( 'demo' ).getValue(); + + return promise; } ); - it( 'should resolve mixed type action array to dispatch', ( done ) => { + it( 'should resolve mixed type action array to dispatch', () => { registerReducer( 'counter', ( state = 0, action ) => { return action.type === 'INCREMENT' ? state + 1 : state; } ); @@ -234,16 +254,17 @@ describe( 'registerResolvers', () => { ], } ); - subscribeWithUnsubscribe( () => { - if ( select( 'counter' ).getCount() === 2 ) { - done(); - } - } ); + const promise = subscribeUntil( [ + () => select( 'counter' ).getCount() === 2, + () => select( 'core/data' ).hasFinishedResolution( 'counter', 'getCount' ), + ] ); select( 'counter' ).getCount(); + + return promise; } ); - it( 'should resolve generator action to dispatch', ( done ) => { + it( 'should resolve generator action to dispatch', () => { registerReducer( 'demo', ( state = 'NOTOK', action ) => { return action.type === 'SET_OK' ? 'OK' : state; } ); @@ -256,19 +277,17 @@ describe( 'registerResolvers', () => { }, } ); - subscribeWithUnsubscribe( () => { - try { - expect( select( 'demo' ).getValue() ).toBe( 'OK' ); - done(); - } catch ( error ) { - done( error ); - } - } ); + const promise = subscribeUntil( [ + () => select( 'demo' ).getValue() === 'OK', + () => select( 'core/data' ).hasFinishedResolution( 'demo', 'getValue' ), + ] ); select( 'demo' ).getValue(); + + return promise; } ); - it( 'should resolve promise action to dispatch', ( done ) => { + it( 'should resolve promise action to dispatch', () => { registerReducer( 'demo', ( state = 'NOTOK', action ) => { return action.type === 'SET_OK' ? 'OK' : state; } ); @@ -279,16 +298,14 @@ describe( 'registerResolvers', () => { getValue: () => Promise.resolve( { type: 'SET_OK' } ), } ); - subscribeWithUnsubscribe( () => { - try { - expect( select( 'demo' ).getValue() ).toBe( 'OK' ); - done(); - } catch ( error ) { - done( error ); - } - } ); + const promise = subscribeUntil( [ + () => select( 'demo' ).getValue() === 'OK', + () => select( 'core/data' ).hasFinishedResolution( 'demo', 'getValue' ), + ] ); select( 'demo' ).getValue(); + + return promise; } ); it( 'should resolve promise non-action to dispatch', ( done ) => { @@ -315,7 +332,7 @@ describe( 'registerResolvers', () => { } ); } ); - it( 'should resolve async iterator action to dispatch', ( done ) => { + it( 'should resolve async iterator action to dispatch', () => { registerReducer( 'counter', ( state = 0, action ) => { return action.type === 'INCREMENT' ? state + 1 : state; } ); @@ -329,16 +346,17 @@ describe( 'registerResolvers', () => { }, } ); - subscribeWithUnsubscribe( () => { - if ( select( 'counter' ).getCount() === 2 ) { - done(); - } - } ); + const promise = subscribeUntil( [ + () => select( 'counter' ).getCount() === 2, + () => select( 'core/data' ).hasFinishedResolution( 'counter', 'getCount' ), + ] ); select( 'counter' ).getCount(); + + return promise; } ); - it( 'should not dispatch resolved promise action on subsequent selector calls', ( done ) => { + it( 'should not dispatch resolved promise action on subsequent selector calls', () => { registerReducer( 'demo', ( state = 'NOTOK', action ) => { return action.type === 'SET_OK' && state === 'NOTOK' ? 'OK' : 'NOTOK'; } ); @@ -349,17 +367,12 @@ describe( 'registerResolvers', () => { getValue: () => Promise.resolve( { type: 'SET_OK' } ), } ); - subscribeWithUnsubscribe( () => { - try { - expect( select( 'demo' ).getValue() ).toBe( 'OK' ); - done(); - } catch ( error ) { - done( error ); - } - } ); + const promise = subscribeUntil( () => select( 'demo' ).getValue() === 'OK' ); select( 'demo' ).getValue(); select( 'demo' ).getValue(); + + return promise; } ); } ); @@ -383,7 +396,7 @@ describe( 'select', () => { } ); describe( 'withSelect', () => { - let wrapper; + let wrapper, store; const unsubscribes = []; afterEach( () => { @@ -399,7 +412,7 @@ describe( 'withSelect', () => { } ); function subscribeWithUnsubscribe( ...args ) { - const unsubscribe = subscribe( ...args ); + const unsubscribe = store.subscribe( ...args ); unsubscribes.push( unsubscribe ); return unsubscribe; } @@ -500,7 +513,7 @@ describe( 'withSelect', () => { // until after the current listener stack is called, we don't attempt // to setState on an unmounting `withSelect` component. It will fail if // an attempt is made to `setState` on an unmounted component. - const store = registerReducer( 'counter', ( state = 0, action ) => { + store = registerReducer( 'counter', ( state = 0, action ) => { if ( action.type === 'increment' ) { return state + 1; } @@ -526,7 +539,7 @@ describe( 'withSelect', () => { } ); it( 'should not rerun selection on unchanging state', () => { - const store = registerReducer( 'unchanging', ( state = {} ) => state ); + store = registerReducer( 'unchanging', ( state = {} ) => state ); registerSelectors( 'unchanging', { getState: ( state ) => state, diff --git a/package-lock.json b/package-lock.json index 2ca45302535ae..c1a6d6443329a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4589,9 +4589,9 @@ } }, "equivalent-key-map": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/equivalent-key-map/-/equivalent-key-map-0.1.1.tgz", - "integrity": "sha512-VfHxntFFcApMyX3TTEQg+nuDPoiGgs1WfYm1JN+d9HUM6Shxp2d7LrMrDmdiybYZVs7U8HM9mqAHpwE9/X8pSQ==" + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/equivalent-key-map/-/equivalent-key-map-0.2.0.tgz", + "integrity": "sha512-F6m+4Th/DEUexGY+OGZoMsq33u8CfLzW9kCuryIugZS4sO4niQIBjVUM3yvWy4oaBUB0hM7uVDBlZhreVAPfcg==" }, "errno": { "version": "0.1.7", diff --git a/package.json b/package.json index 54f009d8f9880..0e06701082eec 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "dom-react": "2.2.1", "dom-scroll-into-view": "1.2.1", "element-closest": "2.0.2", - "equivalent-key-map": "0.1.1", + "equivalent-key-map": "0.2.0", "escape-string-regexp": "1.0.5", "eslint-plugin-wordpress": "git://github.com/WordPress-Coding-Standards/eslint-plugin-wordpress.git#1774343f6226052a46b081e01db3fca8793cc9f1", "hpq": "1.2.0",