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

@@ -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.

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 },

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;

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 );
 }

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' );
 }
 
 /**

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', () => {

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 ) );
 	} );

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 );
 	} );
 } );

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();