From 8d63492cdeeec748b78c043d18143606cde685fb Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Mon, 7 Aug 2023 19:09:14 +0300 Subject: [PATCH 1/4] Data: Introduce `countSelectorsByStatus` redux metadata selector --- packages/data/src/redux-store/index.js | 1 + .../src/redux-store/metadata/selectors.js | 30 +++++++++ .../redux-store/metadata/test/selectors.js | 63 +++++++++++++++++++ 3 files changed, 94 insertions(+) diff --git a/packages/data/src/redux-store/index.js b/packages/data/src/redux-store/index.js index 43357b766f618e..d9614a7ba35baf 100644 --- a/packages/data/src/redux-store/index.js +++ b/packages/data/src/redux-store/index.js @@ -430,6 +430,7 @@ function mapResolveSelectors( selectors, store ) { getResolutionState, getResolutionError, hasResolvingSelectors, + countSelectorsByStatus, ...storeSelectors } = selectors; diff --git a/packages/data/src/redux-store/metadata/selectors.js b/packages/data/src/redux-store/metadata/selectors.js index 6cde6ec54b24e1..4fe535e7dbb51e 100644 --- a/packages/data/src/redux-store/metadata/selectors.js +++ b/packages/data/src/redux-store/metadata/selectors.js @@ -153,3 +153,33 @@ export function hasResolvingSelectors( state ) { ) ); } + +/** + * Retrieves the total number of selectors, grouped per status. + * + * @param {State} state Data state. + * + * @return {Object} Object, containing selector totals by status. + */ +export function countSelectorsByStatus( state ) { + const selectorsByStatus = {}; + + Object.values( state ).forEach( ( selectorState ) => + /** + * This uses the internal `_map` property of `EquivalentKeyMap` for + * optimization purposes, since the `EquivalentKeyMap` implementation + * does not support a `.values()` implementation. + * + * @see https://github.com/aduth/equivalent-key-map + */ + Array.from( selectorState._map.values() ).forEach( ( resolution ) => { + const currentStatus = resolution[ 1 ]?.status ?? 'error'; + if ( ! selectorsByStatus[ currentStatus ] ) { + selectorsByStatus[ currentStatus ] = 0; + } + selectorsByStatus[ currentStatus ]++; + } ) + ); + + return selectorsByStatus; +} diff --git a/packages/data/src/redux-store/metadata/test/selectors.js b/packages/data/src/redux-store/metadata/test/selectors.js index 2eea14a7b6059f..3baa5d4ff9b713 100644 --- a/packages/data/src/redux-store/metadata/test/selectors.js +++ b/packages/data/src/redux-store/metadata/test/selectors.js @@ -356,3 +356,66 @@ describe( 'hasResolvingSelectors', () => { expect( result ).toBe( true ); } ); } ); + +describe( 'countSelectorsByStatus', () => { + let registry; + + beforeEach( () => { + registry = createRegistry(); + registry.registerStore( 'store', { + reducer: ( state = null, action ) => { + if ( action.type === 'RECEIVE' ) { + return action.items; + } + + return state; + }, + selectors: { + getFoo: ( state ) => state, + getBar: ( state ) => state, + getBaz: ( state ) => state, + getFailingFoo: ( state ) => state, + getFailingBar: ( state ) => state, + }, + resolvers: { + getFailingFoo: () => { + throw new Error( 'error fetching' ); + }, + getFailingBar: () => { + throw new Error( 'error fetching' ); + }, + }, + } ); + } ); + + it( 'counts selectors properly by status, excluding missing statuses', () => { + registry.dispatch( 'store' ).startResolution( 'getFoo', [] ); + registry.dispatch( 'store' ).startResolution( 'getBar', [] ); + registry.dispatch( 'store' ).startResolution( 'getBaz', [] ); + registry.dispatch( 'store' ).finishResolution( 'getFoo', [] ); + registry.dispatch( 'store' ).finishResolution( 'getBaz', [] ); + + const { countSelectorsByStatus } = registry.select( 'store' ); + const result = countSelectorsByStatus(); + + expect( result ).toEqual( { + finished: 2, + resolving: 1, + } ); + } ); + + it( 'counts errors properly', async () => { + registry.dispatch( 'store' ).startResolution( 'getFoo', [] ); + await resolve( registry, 'getFailingFoo' ); + await resolve( registry, 'getFailingBar' ); + registry.dispatch( 'store' ).finishResolution( 'getFoo', [] ); + + const { countSelectorsByStatus } = registry.select( 'store' ); + const result = countSelectorsByStatus(); + + expect( result ).toEqual( { + finished: 1, + error: 2, + } ); + } ); +} ); From 4104960e2cacc843f5f9e26d402b5299d9df6602 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Thu, 7 Sep 2023 09:20:55 +0300 Subject: [PATCH 2/4] Add rememo dependency --- package-lock.json | 2 ++ packages/data/package.json | 1 + 2 files changed, 3 insertions(+) diff --git a/package-lock.json b/package-lock.json index 911ece7ee68c00..9769d333d49638 100644 --- a/package-lock.json +++ b/package-lock.json @@ -55117,6 +55117,7 @@ "is-plain-object": "^5.0.0", "is-promise": "^4.0.0", "redux": "^4.1.2", + "rememo": "^4.0.2", "turbo-combine-reducers": "^1.0.2", "use-memo-one": "^1.1.1" }, @@ -67969,6 +67970,7 @@ "is-plain-object": "^5.0.0", "is-promise": "^4.0.0", "redux": "^4.1.2", + "rememo": "^4.0.2", "turbo-combine-reducers": "^1.0.2", "use-memo-one": "^1.1.1" } diff --git a/packages/data/package.json b/packages/data/package.json index b5c7737c2ba767..149c4e18493acc 100644 --- a/packages/data/package.json +++ b/packages/data/package.json @@ -41,6 +41,7 @@ "is-plain-object": "^5.0.0", "is-promise": "^4.0.0", "redux": "^4.1.2", + "rememo": "^4.0.2", "turbo-combine-reducers": "^1.0.2", "use-memo-one": "^1.1.1" }, From 7a1076bef919213eba590ce0e50068fccbbddf12 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Thu, 7 Sep 2023 09:21:43 +0300 Subject: [PATCH 3/4] Add memoization and tests --- .../src/redux-store/metadata/selectors.js | 9 +++++-- .../redux-store/metadata/test/selectors.js | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/data/src/redux-store/metadata/selectors.js b/packages/data/src/redux-store/metadata/selectors.js index 4fe535e7dbb51e..1ac81a5034041e 100644 --- a/packages/data/src/redux-store/metadata/selectors.js +++ b/packages/data/src/redux-store/metadata/selectors.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import createSelector from 'rememo'; + /** * Internal dependencies */ @@ -161,7 +166,7 @@ export function hasResolvingSelectors( state ) { * * @return {Object} Object, containing selector totals by status. */ -export function countSelectorsByStatus( state ) { +export const countSelectorsByStatus = createSelector( ( state ) => { const selectorsByStatus = {}; Object.values( state ).forEach( ( selectorState ) => @@ -182,4 +187,4 @@ export function countSelectorsByStatus( state ) { ); return selectorsByStatus; -} +} ); diff --git a/packages/data/src/redux-store/metadata/test/selectors.js b/packages/data/src/redux-store/metadata/test/selectors.js index 3baa5d4ff9b713..11eba0301e8c55 100644 --- a/packages/data/src/redux-store/metadata/test/selectors.js +++ b/packages/data/src/redux-store/metadata/test/selectors.js @@ -418,4 +418,28 @@ describe( 'countSelectorsByStatus', () => { error: 2, } ); } ); + + it( 'applies memoization and returns the same object for the same state', () => { + const { countSelectorsByStatus } = registry.select( 'store' ); + + expect( countSelectorsByStatus() ).toBe( countSelectorsByStatus() ); + + registry.dispatch( 'store' ).startResolution( 'getFoo', [] ); + registry.dispatch( 'store' ).finishResolution( 'getFoo', [] ); + + expect( countSelectorsByStatus() ).toBe( countSelectorsByStatus() ); + } ); + + it( 'returns a new object when different state is provided', () => { + const { countSelectorsByStatus } = registry.select( 'store' ); + + const result1 = countSelectorsByStatus(); + + registry.dispatch( 'store' ).startResolution( 'getFoo', [] ); + registry.dispatch( 'store' ).finishResolution( 'getFoo', [] ); + + const result2 = countSelectorsByStatus(); + + expect( result1 ).not.toBe( result2 ); + } ); } ); From 9a70aa5fada76676209a691d3b92e61d4d0c9762 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Thu, 7 Sep 2023 10:30:51 +0300 Subject: [PATCH 4/4] Specify createSelector's getDependants explicitly --- .../src/redux-store/metadata/selectors.js | 49 ++++++++++--------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/packages/data/src/redux-store/metadata/selectors.js b/packages/data/src/redux-store/metadata/selectors.js index 1ac81a5034041e..884ec714efd574 100644 --- a/packages/data/src/redux-store/metadata/selectors.js +++ b/packages/data/src/redux-store/metadata/selectors.js @@ -166,25 +166,30 @@ export function hasResolvingSelectors( state ) { * * @return {Object} Object, containing selector totals by status. */ -export const countSelectorsByStatus = createSelector( ( state ) => { - const selectorsByStatus = {}; - - Object.values( state ).forEach( ( selectorState ) => - /** - * This uses the internal `_map` property of `EquivalentKeyMap` for - * optimization purposes, since the `EquivalentKeyMap` implementation - * does not support a `.values()` implementation. - * - * @see https://github.com/aduth/equivalent-key-map - */ - Array.from( selectorState._map.values() ).forEach( ( resolution ) => { - const currentStatus = resolution[ 1 ]?.status ?? 'error'; - if ( ! selectorsByStatus[ currentStatus ] ) { - selectorsByStatus[ currentStatus ] = 0; - } - selectorsByStatus[ currentStatus ]++; - } ) - ); - - return selectorsByStatus; -} ); +export const countSelectorsByStatus = createSelector( + ( state ) => { + const selectorsByStatus = {}; + + Object.values( state ).forEach( ( selectorState ) => + /** + * This uses the internal `_map` property of `EquivalentKeyMap` for + * optimization purposes, since the `EquivalentKeyMap` implementation + * does not support a `.values()` implementation. + * + * @see https://github.com/aduth/equivalent-key-map + */ + Array.from( selectorState._map.values() ).forEach( + ( resolution ) => { + const currentStatus = resolution[ 1 ]?.status ?? 'error'; + if ( ! selectorsByStatus[ currentStatus ] ) { + selectorsByStatus[ currentStatus ] = 0; + } + selectorsByStatus[ currentStatus ]++; + } + ) + ); + + return selectorsByStatus; + }, + ( state ) => [ state ] +);