From a40911ae095883743d4d743a38413588a5e427c0 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Date: Tue, 20 Apr 2021 16:14:39 +0200 Subject: [PATCH 1/2] Add batched variants for start/finish resolution actions --- packages/core-data/src/resolvers.js | 28 +- packages/core-data/src/test/resolvers.js | 8 +- .../data/src/redux-store/metadata/actions.js | 36 +++ .../data/src/redux-store/metadata/reducer.js | 11 + .../src/redux-store/metadata/test/reducer.js | 279 +++++++++++++----- 5 files changed, 262 insertions(+), 100 deletions(-) diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index df7228676b05e9..e02fb7a8cddeb5 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -219,20 +219,20 @@ export function* getEntityRecords( kind, name, query = {} ) { // See https://github.com/WordPress/gutenberg/pull/26575 if ( ! query?._fields ) { const key = entity.key || DEFAULT_ENTITY_KEY; - for ( const record of records ) { - if ( record[ key ] ) { - yield { - type: 'START_RESOLUTION', - selectorName: 'getEntityRecord', - args: [ kind, name, record[ key ] ], - }; - yield { - type: 'FINISH_RESOLUTION', - selectorName: 'getEntityRecord', - args: [ kind, name, record[ key ] ], - }; - } - } + const resolutionsArgs = records + .filter( ( record ) => record[ key ] ) + .map( ( record ) => [ kind, name, record[ key ] ] ); + + yield { + type: 'START_RESOLUTIONS', + selectorName: 'getEntityRecord', + args: resolutionsArgs, + }; + yield { + type: 'FINISH_RESOLUTIONS', + selectorName: 'getEntityRecord', + args: resolutionsArgs, + }; } } finally { yield* __unstableReleaseStoreLock( lock ); diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index 706a6ed8302022..098a399a6f5bf6 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -136,14 +136,14 @@ describe( 'getEntityRecords', () => { // It should mark the entity record that has an ID as resolved expect( fulfillment.next().value ).toEqual( { - type: 'START_RESOLUTION', + type: 'START_RESOLUTIONS', selectorName: 'getEntityRecord', - args: [ ENTITIES[ 1 ].kind, ENTITIES[ 1 ].name, 2 ], + args: [ [ ENTITIES[ 1 ].kind, ENTITIES[ 1 ].name, 2 ] ], } ); expect( fulfillment.next().value ).toEqual( { - type: 'FINISH_RESOLUTION', + type: 'FINISH_RESOLUTIONS', selectorName: 'getEntityRecord', - args: [ ENTITIES[ 1 ].kind, ENTITIES[ 1 ].name, 2 ], + args: [ [ ENTITIES[ 1 ].kind, ENTITIES[ 1 ].name, 2 ] ], } ); } ); } ); diff --git a/packages/data/src/redux-store/metadata/actions.js b/packages/data/src/redux-store/metadata/actions.js index 528cce18cd39f1..2b4cd072530a53 100644 --- a/packages/data/src/redux-store/metadata/actions.js +++ b/packages/data/src/redux-store/metadata/actions.js @@ -32,6 +32,42 @@ export function finishResolution( selectorName, args ) { }; } +/** + * Returns an action object used in signalling that a batch of selector resolutions has + * started. + * + * @param {string} selectorName Name of selector for which resolver triggered. + * @param {...*} args Array of arguments to associate for uniqueness, each item + * is associated to a resolution. + * + * @return {Object} Action object. + */ +export function startResolutions( selectorName, args ) { + return { + type: 'START_RESOLUTIONS', + selectorName, + args, + }; +} + +/** + * Returns an action object used in signalling that a batch of selector resolutions has + * completed. + * + * @param {string} selectorName Name of selector for which resolver triggered. + * @param {...*} args Array of arguments to associate for uniqueness, each item + * is associated to a resolution. + * + * @return {Object} Action object. + */ +export function finishResolutions( selectorName, args ) { + return { + type: 'FINISH_RESOLUTIONS', + selectorName, + args, + }; +} + /** * Returns an action object used in signalling that we should invalidate the resolution cache. * diff --git a/packages/data/src/redux-store/metadata/reducer.js b/packages/data/src/redux-store/metadata/reducer.js index 5f3e9e3f7f8d5f..b762c1a022e39e 100644 --- a/packages/data/src/redux-store/metadata/reducer.js +++ b/packages/data/src/redux-store/metadata/reducer.js @@ -30,6 +30,15 @@ const subKeysIsResolved = onSubKey( 'selectorName' )( nextState.set( action.args, isStarting ); return nextState; } + case 'START_RESOLUTIONS': + case 'FINISH_RESOLUTIONS': { + const isStarting = action.type === 'START_RESOLUTIONS'; + const nextState = new EquivalentKeyMap( state ); + for ( const resolutionArgs of action.args ) { + nextState.set( resolutionArgs, isStarting ); + } + return nextState; + } case 'INVALIDATE_RESOLUTION': { const nextState = new EquivalentKeyMap( state ); nextState.delete( action.args ); @@ -60,6 +69,8 @@ const isResolved = ( state = {}, action ) => { : state; case 'START_RESOLUTION': case 'FINISH_RESOLUTION': + case 'START_RESOLUTIONS': + case 'FINISH_RESOLUTIONS': case 'INVALIDATE_RESOLUTION': return subKeysIsResolved( state, action ); } diff --git a/packages/data/src/redux-store/metadata/test/reducer.js b/packages/data/src/redux-store/metadata/test/reducer.js index 41708eb29176d8..49074b2ea33ce0 100644 --- a/packages/data/src/redux-store/metadata/test/reducer.js +++ b/packages/data/src/redux-store/metadata/test/reducer.js @@ -15,115 +15,230 @@ describe( 'reducer', () => { expect( state ).toEqual( {} ); } ); - it( 'should return with started resolution', () => { - const state = reducer( undefined, { - type: 'START_RESOLUTION', - selectorName: 'getFoo', - args: [], + describe( 'single resolution', () => { + it( 'should return with started resolution', () => { + const state = reducer( undefined, { + type: 'START_RESOLUTION', + selectorName: 'getFoo', + args: [], + } ); + + // { test: { getFoo: EquivalentKeyMap( [] => true ) } } + expect( state.getFoo.get( [] ) ).toBe( true ); } ); - // { test: { getFoo: EquivalentKeyMap( [] => true ) } } - expect( state.getFoo.get( [] ) ).toBe( true ); - } ); + it( 'should return with finished resolution', () => { + const original = reducer( undefined, { + type: 'START_RESOLUTION', + selectorName: 'getFoo', + args: [], + } ); + const state = reducer( deepFreeze( original ), { + type: 'FINISH_RESOLUTION', + selectorName: 'getFoo', + args: [], + } ); - it( 'should return with finished resolution', () => { - const original = reducer( undefined, { - type: 'START_RESOLUTION', - selectorName: 'getFoo', - args: [], - } ); - const state = reducer( deepFreeze( original ), { - type: 'FINISH_RESOLUTION', - selectorName: 'getFoo', - args: [], + // { test: { getFoo: EquivalentKeyMap( [] => false ) } } + expect( state.getFoo.get( [] ) ).toBe( false ); } ); - // { test: { getFoo: EquivalentKeyMap( [] => false ) } } - expect( state.getFoo.get( [] ) ).toBe( false ); - } ); + it( 'should remove invalidations', () => { + let state = reducer( undefined, { + type: 'START_RESOLUTION', + selectorName: 'getFoo', + args: [], + } ); + state = reducer( deepFreeze( state ), { + type: 'FINISH_RESOLUTION', + selectorName: 'getFoo', + args: [], + } ); + state = reducer( deepFreeze( state ), { + type: 'INVALIDATE_RESOLUTION', + selectorName: 'getFoo', + args: [], + } ); - it( 'should remove invalidations', () => { - let state = reducer( undefined, { - type: 'START_RESOLUTION', - selectorName: 'getFoo', - args: [], - } ); - state = reducer( deepFreeze( state ), { - type: 'FINISH_RESOLUTION', - selectorName: 'getFoo', - args: [], - } ); - state = reducer( deepFreeze( state ), { - type: 'INVALIDATE_RESOLUTION', - selectorName: 'getFoo', - args: [], + // { getFoo: EquivalentKeyMap( [] => undefined ) } + expect( state.getFoo.get( [] ) ).toBe( undefined ); } ); - // { getFoo: EquivalentKeyMap( [] => undefined ) } - expect( state.getFoo.get( [] ) ).toBe( undefined ); - } ); + it( 'different arguments should not conflict', () => { + const original = reducer( undefined, { + type: 'START_RESOLUTION', + selectorName: 'getFoo', + args: [ 'post' ], + } ); + let state = reducer( deepFreeze( original ), { + type: 'FINISH_RESOLUTION', + selectorName: 'getFoo', + args: [ 'post' ], + } ); + state = reducer( deepFreeze( state ), { + type: 'START_RESOLUTION', + selectorName: 'getFoo', + args: [ 'block' ], + } ); - it( 'different arguments should not conflict', () => { - const original = reducer( undefined, { - type: 'START_RESOLUTION', - selectorName: 'getFoo', - args: [ 'post' ], - } ); - let state = reducer( deepFreeze( original ), { - type: 'FINISH_RESOLUTION', - selectorName: 'getFoo', - args: [ 'post' ], - } ); - state = reducer( deepFreeze( state ), { - type: 'START_RESOLUTION', - selectorName: 'getFoo', - args: [ 'block' ], + // { getFoo: EquivalentKeyMap( [] => false ) } + expect( state.getFoo.get( [ 'post' ] ) ).toBe( false ); + expect( state.getFoo.get( [ 'block' ] ) ).toBe( true ); } ); - // { getFoo: EquivalentKeyMap( [] => false ) } - expect( state.getFoo.get( [ 'post' ] ) ).toBe( false ); - expect( state.getFoo.get( [ 'block' ] ) ).toBe( true ); + it( + 'should remove invalidation for store level and leave others ' + + 'intact', + () => { + const original = reducer( undefined, { + type: 'FINISH_RESOLUTION', + selectorName: 'getFoo', + args: [ 'post' ], + } ); + const state = reducer( deepFreeze( original ), { + type: 'INVALIDATE_RESOLUTION_FOR_STORE', + } ); + + expect( state ).toEqual( {} ); + } + ); + + it( + 'should remove invalidation for store and selector name level and ' + + 'leave other selectors at store level intact', + () => { + const original = reducer( undefined, { + type: 'FINISH_RESOLUTION', + selectorName: 'getFoo', + args: [ 'post' ], + } ); + let state = reducer( deepFreeze( original ), { + type: 'FINISH_RESOLUTION', + selectorName: 'getBar', + args: [ 'postBar' ], + } ); + state = reducer( deepFreeze( state ), { + type: 'INVALIDATE_RESOLUTION_FOR_STORE_SELECTOR', + selectorName: 'getBar', + } ); + + expect( state.getBar ).toBeUndefined(); + // { getFoo: EquivalentKeyMap( [] => false ) } + expect( state.getFoo.get( [ 'post' ] ) ).toBe( false ); + } + ); } ); - it( - 'should remove invalidation for store level and leave others ' + - 'intact', - () => { + describe( 'resolution batch', () => { + it( 'should return with started resolutions', () => { + const state = reducer( undefined, { + type: 'START_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'post' ], [ 'block' ] ], + } ); + + expect( state.getFoo.get( [ 'post' ] ) ).toBe( true ); + expect( state.getFoo.get( [ 'block' ] ) ).toBe( true ); + } ); + + it( 'should return with finished resolutions', () => { const original = reducer( undefined, { - type: 'FINISH_RESOLUTION', + type: 'START_RESOLUTIONS', selectorName: 'getFoo', - args: [ 'post' ], + args: [ [ 'post' ], [ 'block' ] ], } ); const state = reducer( deepFreeze( original ), { - type: 'INVALIDATE_RESOLUTION_FOR_STORE', + type: 'FINISH_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'post' ], [ 'block' ] ], } ); - expect( state ).toEqual( {} ); - } - ); + expect( state.getFoo.get( [ 'post' ] ) ).toBe( false ); + expect( state.getFoo.get( [ 'block' ] ) ).toBe( false ); + } ); - it( - 'should remove invalidation for store and selector name level and ' + - 'leave other selectors at store level intact', - () => { - const original = reducer( undefined, { - type: 'FINISH_RESOLUTION', + it( 'should remove invalidations', () => { + let state = reducer( undefined, { + type: 'START_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'post' ], [ 'block' ] ], + } ); + state = reducer( deepFreeze( state ), { + type: 'FINISH_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'post' ], [ 'block' ] ], + } ); + state = reducer( deepFreeze( state ), { + type: 'INVALIDATE_RESOLUTION', selectorName: 'getFoo', args: [ 'post' ], } ); + + expect( state.getFoo.get( [ 'post' ] ) ).toBe( undefined ); + expect( state.getFoo.get( [ 'block' ] ) ).toBe( false ); + } ); + + it( 'different arguments should not conflict', () => { + const original = reducer( undefined, { + type: 'START_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'post' ] ], + } ); let state = reducer( deepFreeze( original ), { - type: 'FINISH_RESOLUTION', - selectorName: 'getBar', - args: [ 'postBar' ], + type: 'FINISH_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'post' ] ], } ); state = reducer( deepFreeze( state ), { - type: 'INVALIDATE_RESOLUTION_FOR_STORE_SELECTOR', - selectorName: 'getBar', + type: 'START_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'block' ] ], } ); - expect( state.getBar ).toBeUndefined(); - // { getFoo: EquivalentKeyMap( [] => false ) } expect( state.getFoo.get( [ 'post' ] ) ).toBe( false ); - } - ); + expect( state.getFoo.get( [ 'block' ] ) ).toBe( true ); + } ); + + it( + 'should remove invalidation for store level and leave others ' + + 'intact', + () => { + const original = reducer( undefined, { + type: 'FINISH_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'post' ], [ 'block' ] ], + } ); + const state = reducer( deepFreeze( original ), { + type: 'INVALIDATE_RESOLUTION_FOR_STORE', + } ); + + expect( state ).toEqual( {} ); + } + ); + + it( + 'should remove invalidation for store and selector name level and ' + + 'leave other selectors at store level intact', + () => { + const original = reducer( undefined, { + type: 'FINISH_RESOLUTIONS', + selectorName: 'getFoo', + args: [ [ 'post' ], [ 'block' ] ], + } ); + let state = reducer( deepFreeze( original ), { + type: 'FINISH_RESOLUTIONS', + selectorName: 'getBar', + args: [ [ 'postBar' ] ], + } ); + state = reducer( deepFreeze( state ), { + type: 'INVALIDATE_RESOLUTION_FOR_STORE_SELECTOR', + selectorName: 'getBar', + } ); + + expect( state.getBar ).toBeUndefined(); + expect( state.getFoo.get( [ 'post' ] ) ).toBe( false ); + expect( state.getFoo.get( [ 'block' ] ) ).toBe( false ); + } + ); + } ); } ); From 9e19ef18e456a5cded767a7c00f543511f3828bb Mon Sep 17 00:00:00 2001 From: Carlos Garcia Date: Tue, 20 Apr 2021 17:30:48 +0200 Subject: [PATCH 2/2] CHANGELOG files of core-data and data packages updated --- packages/core-data/CHANGELOG.md | 1 + packages/data/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/core-data/CHANGELOG.md b/packages/core-data/CHANGELOG.md index 9e629d47e155a3..3b5b0030eba3c8 100644 --- a/packages/core-data/CHANGELOG.md +++ b/packages/core-data/CHANGELOG.md @@ -1,6 +1,7 @@ ## Unreleased +- The `getEntityRecords` resolver has been updated and now uses the batched variants of start and finish resolution actions. ## 2.26.0 (2021-03-17) diff --git a/packages/data/CHANGELOG.md b/packages/data/CHANGELOG.md index 2a17415457f191..27ed36f133da2e 100644 --- a/packages/data/CHANGELOG.md +++ b/packages/data/CHANGELOG.md @@ -1,6 +1,7 @@ ## Unreleased +- Added new `startResolutions` and `finishResolutions` actions as batched variants of `startResolution` and `finishResolutions` actions. ## 4.27.0 (2021-03-17)