Skip to content
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.

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

Data: Refactor resolver fulfillment / request progress as data state #6600

Merged
merged 2 commits into from
May 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 0 additions & 17 deletions core-data/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 8 additions & 1 deletion core-data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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 },
Expand Down
11 changes: 0 additions & 11 deletions core-data/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions core-data/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import apiRequest from '@wordpress/api-request';
* Internal dependencies
*/
import {
setRequested,
receiveTerms,
receiveUserQuery,
receiveEntityRecords,
Expand All @@ -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 );
}
Expand Down
29 changes: 26 additions & 3 deletions core-data/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,29 @@
*/
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 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's see if we want to autogenerate this one later on.

return select( 'core/data' ).isResolving( REDUCER_KEY, selectorName, ...args );
}

/**
* Returns all the available terms for the given taxonomy.
*
Expand Down Expand Up @@ -36,7 +59,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 );
}

/**
Expand All @@ -47,8 +70,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' );
}

/**
Expand Down
39 changes: 0 additions & 39 deletions core-data/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
4 changes: 1 addition & 3 deletions core-data/test/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );

Expand All @@ -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 ) );
} );
Expand Down
54 changes: 32 additions & 22 deletions core-data/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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 );
} );
} );
Expand Down
46 changes: 15 additions & 31 deletions data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import { combineReducers, createStore } from 'redux';
import { flowRight, without, mapValues, overEvery } from 'lodash';
import EquivalentKeyMap from 'equivalent-key-map';

/**
* WordPress dependencies
Expand All @@ -14,6 +13,8 @@ import isShallowEqual from '@wordpress/is-shallow-equal';
/**
* Internal dependencies
*/
import registerDataStore from './store';

export { loadAndPersist, withRehydratation } from './persist';

/**
Expand Down Expand Up @@ -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();
Expand All @@ -193,6 +173,8 @@ export function registerResolvers( reducerKey, newResolvers ) {
store.dispatch( maybeAction );
}
}

finishResolution( reducerKey, selectorName, args );
}

if ( typeof resolver.isFulfilled === 'function' ) {
Expand Down Expand Up @@ -485,3 +467,5 @@ export function toAsyncIterable( object ) {
}
}() );
}

registerDataStore();
Loading