From 1de764720cda2ea5c002c58860f5f38401dbd53a Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Wed, 27 May 2020 12:15:24 +1200 Subject: [PATCH 01/13] try --- client/homepage/index.js | 20 ++++++++++++++++-- packages/data/src/options/resolvers.js | 21 +++++++++++++++++++ packages/data/src/options/selectors.js | 4 ++++ .../src/options/with-options-hydration.js | 18 +++++++++------- 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/client/homepage/index.js b/client/homepage/index.js index 5cd4517a3fa..9f7b5c3eac5 100644 --- a/client/homepage/index.js +++ b/client/homepage/index.js @@ -3,11 +3,16 @@ */ import { compose } from '@wordpress/compose'; import { Suspense, lazy } from '@wordpress/element'; +import { identity } from 'lodash'; /** * WooCommerce dependencies */ import { Spinner } from '@woocommerce/components'; +import { + withOptionsHydration, + OPTIONS_STORE_NAME, +} from '@woocommerce/data'; /** * Internal dependencies @@ -19,7 +24,9 @@ const ProfileWizard = lazy( () => import( /* webpackChunkName: "profile-wizard" */ '../profile-wizard' ) ); -const Homepage = ( { profileItems, query } ) => { +const Homepage = ( { profileItems, query, blogname, blogdescription } ) => { + console.log(blogname); + console.log(blogdescription); if ( isOnboardingEnabled() && ! profileItems.completed ) { return ( }> @@ -32,6 +39,11 @@ const Homepage = ( { profileItems, query } ) => { }; export default compose( + window.wcSettings.preloadOptions + ? withOptionsHydration( { + ...window.wcSettings.preloadOptions, + } ) + : identity, withSelect( ( select ) => { if ( ! isOnboardingEnabled() ) { return; @@ -40,6 +52,10 @@ export default compose( const { getProfileItems } = select( 'wc-api' ); const profileItems = getProfileItems(); - return { profileItems }; + const { getOption } = select( OPTIONS_STORE_NAME ); + const blogname = getOption( 'blogname' ); + const blogdescription = getOption( 'blogdescription' ); + + return { profileItems, blogname, blogdescription }; } ) )( Homepage ); diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 04735747f4c..8dcae2cb381 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -23,3 +23,24 @@ export function* getOptionsWithRequest( names ) { return error; } } + +const optionsToRequest = []; +const fetches = {}; + +export function* getOption( name ) { + yield setIsRequesting( true ); + optionsToRequest.push( name ); + + setTimeout( function* () { + const names = optionsToRequest.join(','); + const fetchPromise = fetches[ names ]; + if ( fetchPromise ) { + const results = yield fetchPromise; + return results; + } + + const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; + fetches[ names ] = yield apiFetch( { path: url } ); + return fetches[ names ]; + }, 1 ); +} diff --git a/packages/data/src/options/selectors.js b/packages/data/src/options/selectors.js index 1cd138eff6c..4cfc1da5e67 100644 --- a/packages/data/src/options/selectors.js +++ b/packages/data/src/options/selectors.js @@ -27,6 +27,10 @@ export const getOptions = ( state, names ) => { }, {} ); }; +export const getOption = ( state, name ) => { + return state[ name ]; +}; + /** * Get options from state tree or make a request if unresolved. * diff --git a/packages/data/src/options/with-options-hydration.js b/packages/data/src/options/with-options-hydration.js index 7d465baa693..6c27bddd3f6 100644 --- a/packages/data/src/options/with-options-hydration.js +++ b/packages/data/src/options/with-options-hydration.js @@ -26,14 +26,16 @@ export const withOptionsHydration = ( data ) => ( OriginalComponent ) => { } = registry.dispatch( STORE_NAME ); const names = Object.keys( dataRef.current ); - if ( - ! isResolving( 'getOptionsWithRequest', names ) && - ! hasFinishedResolution( 'getOptionsWithRequest', names ) - ) { - startResolution( 'getOptionsWithRequest', names ); - receiveOptions( dataRef.current ); - finishResolution( 'getOptionsWithRequest', names ); - } + names.forEach( ( name ) => { + if ( + ! isResolving( 'getOption', [ name ] ) && + ! hasFinishedResolution( 'getOption', [ name ] ) + ) { + startResolution( 'getOption', [ name ] ); + receiveOptions( { [ name ]: dataRef.current[ name ] } ); + finishResolution( 'getOption', [ name ] ); + } + } ); }, [] ); return ; From c689874248a4ddfa3eb3b4aa215c80dd445050e3 Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Wed, 27 May 2020 12:36:15 +1200 Subject: [PATCH 02/13] receiveOptions --- packages/data/src/options/resolvers.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 8dcae2cb381..25279c58e69 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -41,6 +41,7 @@ export function* getOption( name ) { const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; fetches[ names ] = yield apiFetch( { path: url } ); + yield receiveOptions( fetches[ names ] ); return fetches[ names ]; }, 1 ); } From e5c3573bdca7ae3f50d0f9cb0d76d2b7d27e2f2f Mon Sep 17 00:00:00 2001 From: Joshua Flowers Date: Wed, 27 May 2020 13:23:29 +0300 Subject: [PATCH 03/13] Add control to handle checking requested options --- packages/data/src/options/controls.js | 31 ++++++++++++++++++++++++++ packages/data/src/options/index.js | 2 +- packages/data/src/options/resolvers.js | 25 +++++++++------------ 3 files changed, 42 insertions(+), 16 deletions(-) create mode 100644 packages/data/src/options/controls.js diff --git a/packages/data/src/options/controls.js b/packages/data/src/options/controls.js new file mode 100644 index 00000000000..891de67a7d4 --- /dev/null +++ b/packages/data/src/options/controls.js @@ -0,0 +1,31 @@ +/** + * External dependencies + */ +import { controls as dataControls } from '@wordpress/data-controls'; + +let optionNames = []; + +export const getOptionsToRequest = ( optionName ) => { + return { + type: 'GET_OPTIONS_TO_REQUEST', + optionName, + }; +}; + +export const controls = { + ...dataControls, + GET_OPTIONS_TO_REQUEST( { optionName } ) { + optionNames.push( optionName ); + + return new Promise( resolve => { + setTimeout( () => { + resolve( optionNames.join(',') ); + + // Clear option names after all resolved; + setTimeout( () => { + optionNames = []; + }, 1 ) + }, 1 ); + } ); + }, +}; diff --git a/packages/data/src/options/index.js b/packages/data/src/options/index.js index 5e03c0bb99a..8e017be45b3 100644 --- a/packages/data/src/options/index.js +++ b/packages/data/src/options/index.js @@ -3,7 +3,6 @@ */ import { registerStore } from '@wordpress/data'; -import { controls } from '@wordpress/data-controls'; /** * Internal dependencies @@ -12,6 +11,7 @@ import { STORE_NAME } from './constants'; import * as selectors from './selectors'; import * as actions from './actions'; import * as resolvers from './resolvers'; +import { controls } from './controls'; import reducer from './reducer'; registerStore( STORE_NAME, { diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 25279c58e69..beffbf56608 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -1,8 +1,8 @@ /** * External Dependencies */ - import { apiFetch } from '@wordpress/data-controls'; +import { getOptionsToRequest } from './controls'; /** * Internal dependencies @@ -24,24 +24,19 @@ export function* getOptionsWithRequest( names ) { } } -const optionsToRequest = []; const fetches = {}; export function* getOption( name ) { yield setIsRequesting( true ); - optionsToRequest.push( name ); + const names = yield getOptionsToRequest( name ); - setTimeout( function* () { - const names = optionsToRequest.join(','); - const fetchPromise = fetches[ names ]; - if ( fetchPromise ) { - const results = yield fetchPromise; - return results; - } + const fetchInProgress = fetches[ names ]; + if ( fetchInProgress ) { + return; + } - const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; - fetches[ names ] = yield apiFetch( { path: url } ); - yield receiveOptions( fetches[ names ] ); - return fetches[ names ]; - }, 1 ); + const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; + fetches[ names ] = true; + const result = yield apiFetch( { path: url } ); + yield receiveOptions( result ); } From 47f149e07f819d6a3b1d6380a2e4b7e4eea3987e Mon Sep 17 00:00:00 2001 From: Joshua Flowers Date: Wed, 27 May 2020 13:43:55 +0300 Subject: [PATCH 04/13] Delete fetch request after completion --- packages/data/src/options/resolvers.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index beffbf56608..c7661b290e6 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -37,6 +37,9 @@ export function* getOption( name ) { const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; fetches[ names ] = true; - const result = yield apiFetch( { path: url } ); + const result = yield apiFetch( { path: url } ); yield receiveOptions( result ); + + // Delete the fetch after to allow wp data to handle cache invalidation. + delete fetches[ names ]; } From c01969fadd8702e31b6b29880418c0623d1f4930 Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Thu, 28 May 2020 12:45:14 +1200 Subject: [PATCH 05/13] wait for in-flight result before returning --- client/homepage/index.js | 20 +++++++++++++------- packages/data/src/options/resolvers.js | 7 +++---- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/client/homepage/index.js b/client/homepage/index.js index 9f7b5c3eac5..386c18c6527 100644 --- a/client/homepage/index.js +++ b/client/homepage/index.js @@ -9,10 +9,7 @@ import { identity } from 'lodash'; * WooCommerce dependencies */ import { Spinner } from '@woocommerce/components'; -import { - withOptionsHydration, - OPTIONS_STORE_NAME, -} from '@woocommerce/data'; +import { withOptionsHydration, OPTIONS_STORE_NAME } from '@woocommerce/data'; /** * Internal dependencies @@ -25,8 +22,8 @@ const ProfileWizard = lazy( () => ); const Homepage = ( { profileItems, query, blogname, blogdescription } ) => { - console.log(blogname); - console.log(blogdescription); + console.log( blogname ); + console.log( blogdescription ); if ( isOnboardingEnabled() && ! profileItems.completed ) { return ( }> @@ -52,10 +49,19 @@ export default compose( const { getProfileItems } = select( 'wc-api' ); const profileItems = getProfileItems(); - const { getOption } = select( OPTIONS_STORE_NAME ); + const { getOption, isResolving } = select( OPTIONS_STORE_NAME ); const blogname = getOption( 'blogname' ); const blogdescription = getOption( 'blogdescription' ); + console.log( + 'isResolving blogname', + isResolving( 'getOption', [ 'blogname' ] ) + ); + console.log( + 'isResolving blogdescription', + isResolving( 'getOption', [ 'blogdescription' ] ) + ); + return { profileItems, blogname, blogdescription }; } ) )( Homepage ); diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index c7661b290e6..06b28593f76 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -32,13 +32,12 @@ export function* getOption( name ) { const fetchInProgress = fetches[ names ]; if ( fetchInProgress ) { - return; + return yield fetches[ names ]; } const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; - fetches[ names ] = true; - const result = yield apiFetch( { path: url } ); - yield receiveOptions( result ); + fetches[ names ] = yield apiFetch( { path: url } ); + yield receiveOptions( fetches[ names ] ); // Delete the fetch after to allow wp data to handle cache invalidation. delete fetches[ names ]; From 6c19881ce424918c16b5e8cf835016d1d908de2f Mon Sep 17 00:00:00 2001 From: Joshua Flowers Date: Thu, 28 May 2020 13:19:21 +0300 Subject: [PATCH 06/13] Manually set requesting status of options --- .../task-list/tasks/payments/square.js | 6 ++---- .../task-list/tasks/payments/wcpay.js | 6 ++---- client/homepage/index.js | 10 +++++----- packages/data/src/options/actions.js | 4 ++-- packages/data/src/options/reducer.js | 18 ++++++++++++++---- packages/data/src/options/resolvers.js | 9 +++++---- packages/data/src/options/selectors.js | 5 +++-- 7 files changed, 33 insertions(+), 25 deletions(-) diff --git a/client/dashboard/task-list/tasks/payments/square.js b/client/dashboard/task-list/tasks/payments/square.js index c562c489dd4..639bcce7737 100644 --- a/client/dashboard/task-list/tasks/payments/square.js +++ b/client/dashboard/task-list/tasks/payments/square.js @@ -145,16 +145,14 @@ class Square extends Component { export default compose( withSelect( ( select ) => { - const { getOptions, isGetOptionsRequesting } = select( + const { getOptions, isRequesting } = select( OPTIONS_STORE_NAME ); const options = getOptions( [ 'woocommerce_square_credit_card_settings', ] ); const optionsIsRequesting = Boolean( - isGetOptionsRequesting( [ - 'woocommerce_square_credit_card_settings', - ] ) + isRequesting( 'woocommerce_square_credit_card_settings' ) ); return { diff --git a/client/dashboard/task-list/tasks/payments/wcpay.js b/client/dashboard/task-list/tasks/payments/wcpay.js index 41286961d66..d1b52fb5e37 100644 --- a/client/dashboard/task-list/tasks/payments/wcpay.js +++ b/client/dashboard/task-list/tasks/payments/wcpay.js @@ -127,16 +127,14 @@ class WCPay extends Component { export default compose( withSelect( ( select ) => { - const { getOptions, isGetOptionsRequesting } = select( + const { getOptions, isRequesting } = select( OPTIONS_STORE_NAME ); const options = getOptions( [ 'woocommerce_woocommerce_payments_settings', ] ); const optionsIsRequesting = Boolean( - isGetOptionsRequesting( [ - 'woocommerce_woocommerce_payments_settings', - ] ) + isRequesting( 'woocommerce_woocommerce_payments_settings' ) ); return { diff --git a/client/homepage/index.js b/client/homepage/index.js index 386c18c6527..5221487a50a 100644 --- a/client/homepage/index.js +++ b/client/homepage/index.js @@ -49,17 +49,17 @@ export default compose( const { getProfileItems } = select( 'wc-api' ); const profileItems = getProfileItems(); - const { getOption, isResolving } = select( OPTIONS_STORE_NAME ); + const { getOption, isRequesting } = select( OPTIONS_STORE_NAME ); const blogname = getOption( 'blogname' ); const blogdescription = getOption( 'blogdescription' ); console.log( - 'isResolving blogname', - isResolving( 'getOption', [ 'blogname' ] ) + 'isRequesting blogname', + isRequesting( 'blogname' ) ); console.log( - 'isResolving blogdescription', - isResolving( 'getOption', [ 'blogdescription' ] ) + 'isRequesting blogdescription', + isRequesting( 'blogdescription' ) ); return { profileItems, blogname, blogdescription }; diff --git a/packages/data/src/options/actions.js b/packages/data/src/options/actions.js index b6b6b269b9b..d3b5c2db0b5 100644 --- a/packages/data/src/options/actions.js +++ b/packages/data/src/options/actions.js @@ -17,10 +17,10 @@ export function receiveOptions( options ) { }; } -export function setIsRequesting( isRequesting ) { +export function setIsRequesting( optionName ) { return { type: TYPES.SET_IS_REQUESTING, - isRequesting, + optionName }; } diff --git a/packages/data/src/options/reducer.js b/packages/data/src/options/reducer.js index 71f707c5f4e..2c59f6d187e 100644 --- a/packages/data/src/options/reducer.js +++ b/packages/data/src/options/reducer.js @@ -4,21 +4,31 @@ import TYPES from './action-types'; const optionsReducer = ( - state = { isRequesting: false, isUpdating: false }, - { type, options, isRequesting, error, isUpdating } + state = { isRequesting: {}, isUpdating: false }, + { type, options, error, isUpdating, optionName } ) => { switch ( type ) { case TYPES.RECEIVE_OPTIONS: + const resolvedOptions = {}; + Object.keys( options ).forEach( name => { + resolvedOptions[ name ] = false; + } ); state = { ...state, ...options, - isRequesting: false, + isRequesting: { + ...state.isRequesting, + ...resolvedOptions + }, }; break; case TYPES.SET_IS_REQUESTING: state = { ...state, - isRequesting, + isRequesting: { + ...state.isRequesting, + [ optionName ]: true, + }, }; break; case TYPES.SET_IS_UPDATING: diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 06b28593f76..7a1a73e27d3 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -27,17 +27,18 @@ export function* getOptionsWithRequest( names ) { const fetches = {}; export function* getOption( name ) { - yield setIsRequesting( true ); + yield setIsRequesting( name ); const names = yield getOptionsToRequest( name ); const fetchInProgress = fetches[ names ]; if ( fetchInProgress ) { - return yield fetches[ names ]; + return; } const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; - fetches[ names ] = yield apiFetch( { path: url } ); - yield receiveOptions( fetches[ names ] ); + fetches[ names ] = true; + const result = yield apiFetch( { path: url } ); + yield receiveOptions( result ); // Delete the fetch after to allow wp data to handle cache invalidation. delete fetches[ names ]; diff --git a/packages/data/src/options/selectors.js b/packages/data/src/options/selectors.js index 4cfc1da5e67..fde266ddfb4 100644 --- a/packages/data/src/options/selectors.js +++ b/packages/data/src/options/selectors.js @@ -48,9 +48,10 @@ export const getOptionsWithRequest = ( state, names ) => { * Determine if options are being requested. * * @param {Object} state - Reducer state + * @param {string} optionName - Option name */ -export const isGetOptionsRequesting = ( state ) => { - return state.isRequesting || false; +export const isRequesting = ( state, optionName ) => { + return state.isRequesting[ optionName ] || false; }; /** From f24f6039e7fe3b78fd5323c4e3989c12b42b2d1f Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Tue, 2 Jun 2020 09:29:34 +1200 Subject: [PATCH 07/13] Add error handling --- packages/data/src/options/actions.js | 3 ++- packages/data/src/options/reducer.js | 18 ++++++++++++------ packages/data/src/options/resolvers.js | 11 ++++++++--- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/data/src/options/actions.js b/packages/data/src/options/actions.js index d3b5c2db0b5..5c30c7dbddc 100644 --- a/packages/data/src/options/actions.js +++ b/packages/data/src/options/actions.js @@ -24,10 +24,11 @@ export function setIsRequesting( optionName ) { }; } -export function setRequestingError( error ) { +export function setRequestingError( error, options ) { return { type: TYPES.SET_REQUESTING_ERROR, error, + options }; } diff --git a/packages/data/src/options/reducer.js b/packages/data/src/options/reducer.js index 2c59f6d187e..b947fd00146 100644 --- a/packages/data/src/options/reducer.js +++ b/packages/data/src/options/reducer.js @@ -3,22 +3,25 @@ */ import TYPES from './action-types'; +const getIsRequestingObject = ( options, value ) => { + return Object.keys( options ).reduce( ( result, name ) => { + result[ name ] = value; + return result; + }, {} ); +}; + const optionsReducer = ( state = { isRequesting: {}, isUpdating: false }, { type, options, error, isUpdating, optionName } ) => { switch ( type ) { case TYPES.RECEIVE_OPTIONS: - const resolvedOptions = {}; - Object.keys( options ).forEach( name => { - resolvedOptions[ name ] = false; - } ); state = { ...state, ...options, isRequesting: { ...state.isRequesting, - ...resolvedOptions + ...getIsRequestingObject( options, false ), }, }; break; @@ -41,7 +44,10 @@ const optionsReducer = ( state = { ...state, requestingError: error, - isRequesting: false, + isRequesting: { + ...state.isRequesting, + ...getIsRequestingObject( options, false ), + }, }; break; case TYPES.SET_UPDATING_ERROR: diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 7a1a73e27d3..8c445c4490c 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -19,7 +19,7 @@ export function* getOptionsWithRequest( names ) { yield receiveOptions( results ); return results; } catch ( error ) { - yield setRequestingError( error ); + yield setRequestingError( error, names ); return error; } } @@ -37,8 +37,13 @@ export function* getOption( name ) { const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; fetches[ names ] = true; - const result = yield apiFetch( { path: url } ); - yield receiveOptions( result ); + + try { + const result = yield apiFetch( { path: url } ); + yield receiveOptions( result ); + } catch ( error ) { + yield setRequestingError( error, names ); + } // Delete the fetch after to allow wp data to handle cache invalidation. delete fetches[ names ]; From a34115de72f790e3c6dc89acbaa5d084e3bf9eac Mon Sep 17 00:00:00 2001 From: Joshua Flowers Date: Tue, 2 Jun 2020 18:48:50 +0300 Subject: [PATCH 08/13] Move fetch into batch fetch control --- client/homepage/index.js | 10 ++++----- packages/data/src/options/controls.js | 31 +++++++++++++++++++++----- packages/data/src/options/reducer.js | 8 ------- packages/data/src/options/resolvers.js | 21 +++-------------- 4 files changed, 34 insertions(+), 36 deletions(-) diff --git a/client/homepage/index.js b/client/homepage/index.js index 5221487a50a..386c18c6527 100644 --- a/client/homepage/index.js +++ b/client/homepage/index.js @@ -49,17 +49,17 @@ export default compose( const { getProfileItems } = select( 'wc-api' ); const profileItems = getProfileItems(); - const { getOption, isRequesting } = select( OPTIONS_STORE_NAME ); + const { getOption, isResolving } = select( OPTIONS_STORE_NAME ); const blogname = getOption( 'blogname' ); const blogdescription = getOption( 'blogdescription' ); console.log( - 'isRequesting blogname', - isRequesting( 'blogname' ) + 'isResolving blogname', + isResolving( 'getOption', [ 'blogname' ] ) ); console.log( - 'isRequesting blogdescription', - isRequesting( 'blogdescription' ) + 'isResolving blogdescription', + isResolving( 'getOption', [ 'blogdescription' ] ) ); return { profileItems, blogname, blogdescription }; diff --git a/packages/data/src/options/controls.js b/packages/data/src/options/controls.js index 891de67a7d4..e473f905866 100644 --- a/packages/data/src/options/controls.js +++ b/packages/data/src/options/controls.js @@ -2,29 +2,50 @@ * External dependencies */ import { controls as dataControls } from '@wordpress/data-controls'; +import apiFetch from '@wordpress/api-fetch'; + +/** + * Internal dependencies + */ +import { WC_ADMIN_NAMESPACE } from '../constants'; let optionNames = []; +const fetches = {}; -export const getOptionsToRequest = ( optionName ) => { +export const batchFetch = ( optionName ) => { return { - type: 'GET_OPTIONS_TO_REQUEST', + type: 'BATCH_FETCH', optionName, }; }; export const controls = { ...dataControls, - GET_OPTIONS_TO_REQUEST( { optionName } ) { + BATCH_FETCH( { optionName } ) { optionNames.push( optionName ); return new Promise( resolve => { - setTimeout( () => { - resolve( optionNames.join(',') ); + setTimeout( function() { + const names = optionNames.join(','); + if ( fetches[ names ] ) { + return fetches[ names ].then( ( result ) => { + resolve( result[ optionName ] ); + } ); + } + + const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; + fetches[ names ] = apiFetch( { path: url } ); + fetches[names].then( ( result ) => { + resolve( result[ optionName ] ); + } ) // Clear option names after all resolved; setTimeout( () => { optionNames = []; + // Delete the fetch after to allow wp data to handle cache invalidation. + delete fetches[ names ]; }, 1 ) + }, 1 ); } ); }, diff --git a/packages/data/src/options/reducer.js b/packages/data/src/options/reducer.js index 2c59f6d187e..a6e548c4a94 100644 --- a/packages/data/src/options/reducer.js +++ b/packages/data/src/options/reducer.js @@ -9,17 +9,9 @@ const optionsReducer = ( ) => { switch ( type ) { case TYPES.RECEIVE_OPTIONS: - const resolvedOptions = {}; - Object.keys( options ).forEach( name => { - resolvedOptions[ name ] = false; - } ); state = { ...state, ...options, - isRequesting: { - ...state.isRequesting, - ...resolvedOptions - }, }; break; case TYPES.SET_IS_REQUESTING: diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 7a1a73e27d3..9154d2fc544 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -2,7 +2,7 @@ * External Dependencies */ import { apiFetch } from '@wordpress/data-controls'; -import { getOptionsToRequest } from './controls'; +import { batchFetch } from './controls'; /** * Internal dependencies @@ -24,22 +24,7 @@ export function* getOptionsWithRequest( names ) { } } -const fetches = {}; - export function* getOption( name ) { - yield setIsRequesting( name ); - const names = yield getOptionsToRequest( name ); - - const fetchInProgress = fetches[ names ]; - if ( fetchInProgress ) { - return; - } - - const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; - fetches[ names ] = true; - const result = yield apiFetch( { path: url } ); - yield receiveOptions( result ); - - // Delete the fetch after to allow wp data to handle cache invalidation. - delete fetches[ names ]; + const result = yield batchFetch( name ); + yield receiveOptions( { [ name ]: result } ); } From 1f603d9913f1a0d0bbd58f07edb0c9e3f89f8345 Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Thu, 4 Jun 2020 11:56:44 +1200 Subject: [PATCH 09/13] reset --- client/dashboard/task-list/tasks/payments/square.js | 6 ++++-- client/dashboard/task-list/tasks/payments/wcpay.js | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/client/dashboard/task-list/tasks/payments/square.js b/client/dashboard/task-list/tasks/payments/square.js index 639bcce7737..c562c489dd4 100644 --- a/client/dashboard/task-list/tasks/payments/square.js +++ b/client/dashboard/task-list/tasks/payments/square.js @@ -145,14 +145,16 @@ class Square extends Component { export default compose( withSelect( ( select ) => { - const { getOptions, isRequesting } = select( + const { getOptions, isGetOptionsRequesting } = select( OPTIONS_STORE_NAME ); const options = getOptions( [ 'woocommerce_square_credit_card_settings', ] ); const optionsIsRequesting = Boolean( - isRequesting( 'woocommerce_square_credit_card_settings' ) + isGetOptionsRequesting( [ + 'woocommerce_square_credit_card_settings', + ] ) ); return { diff --git a/client/dashboard/task-list/tasks/payments/wcpay.js b/client/dashboard/task-list/tasks/payments/wcpay.js index d1b52fb5e37..41286961d66 100644 --- a/client/dashboard/task-list/tasks/payments/wcpay.js +++ b/client/dashboard/task-list/tasks/payments/wcpay.js @@ -127,14 +127,16 @@ class WCPay extends Component { export default compose( withSelect( ( select ) => { - const { getOptions, isRequesting } = select( + const { getOptions, isGetOptionsRequesting } = select( OPTIONS_STORE_NAME ); const options = getOptions( [ 'woocommerce_woocommerce_payments_settings', ] ); const optionsIsRequesting = Boolean( - isRequesting( 'woocommerce_woocommerce_payments_settings' ) + isGetOptionsRequesting( [ + 'woocommerce_woocommerce_payments_settings', + ] ) ); return { From 88bf2374af68ac171fe97452b2f9e1797f167c88 Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Thu, 4 Jun 2020 12:21:57 +1200 Subject: [PATCH 10/13] reducer tests --- packages/data/src/options/actions.js | 11 +---- packages/data/src/options/reducer.js | 26 ++-------- packages/data/src/options/resolvers.js | 5 +- packages/data/src/options/selectors.js | 5 +- packages/data/src/options/test/reducer.js | 60 +++++++++++++++++++++++ 5 files changed, 71 insertions(+), 36 deletions(-) create mode 100644 packages/data/src/options/test/reducer.js diff --git a/packages/data/src/options/actions.js b/packages/data/src/options/actions.js index dd8eddd499e..62f5fc012bb 100644 --- a/packages/data/src/options/actions.js +++ b/packages/data/src/options/actions.js @@ -17,18 +17,11 @@ export function receiveOptions( options ) { }; } -export function setIsRequesting( optionName ) { - return { - type: TYPES.SET_IS_REQUESTING, - optionName, - }; -} - -export function setRequestingError( error, options ) { +export function setRequestingError( error, name ) { return { type: TYPES.SET_REQUESTING_ERROR, error, - options, + name, }; } diff --git a/packages/data/src/options/reducer.js b/packages/data/src/options/reducer.js index ffd2dce54b8..759f3ab32ec 100644 --- a/packages/data/src/options/reducer.js +++ b/packages/data/src/options/reducer.js @@ -3,16 +3,9 @@ */ import TYPES from './action-types'; -const getIsRequestingObject = ( options, value ) => { - return Object.keys( options ).reduce( ( result, name ) => { - result[ name ] = value; - return result; - }, {} ); -}; - const optionsReducer = ( - state = { isRequesting: {}, isUpdating: false }, - { type, options, error, isUpdating, optionName } + state = { isUpdating: false, requestingErrors: {} }, + { type, options, error, isUpdating, name } ) => { switch ( type ) { case TYPES.RECEIVE_OPTIONS: @@ -21,15 +14,6 @@ const optionsReducer = ( ...options, }; break; - case TYPES.SET_IS_REQUESTING: - state = { - ...state, - isRequesting: { - ...state.isRequesting, - [ optionName ]: true, - }, - }; - break; case TYPES.SET_IS_UPDATING: state = { ...state, @@ -39,10 +23,8 @@ const optionsReducer = ( case TYPES.SET_REQUESTING_ERROR: state = { ...state, - requestingError: error, - isRequesting: { - ...state.isRequesting, - ...getIsRequestingObject( options, false ), + requestingErrors: { + [ name ]: error, }, }; break; diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 1b6d148a44f..623b6da88de 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -8,10 +8,9 @@ import { batchFetch } from './controls'; * Internal dependencies */ import { WC_ADMIN_NAMESPACE } from '../constants'; -import { receiveOptions, setIsRequesting, setRequestingError } from './actions'; +import { receiveOptions, setRequestingError } from './actions'; export function* getOptionsWithRequest( names ) { - yield setIsRequesting( true ); const url = WC_ADMIN_NAMESPACE + '/options?options=' + names.join( ',' ); try { @@ -29,6 +28,6 @@ export function* getOption( name ) { const result = yield batchFetch( name ); yield receiveOptions( { [ name ]: result } ); } catch ( error ) { - yield setRequestingError( error, [ name ] ); + yield setRequestingError( error, name ); } } diff --git a/packages/data/src/options/selectors.js b/packages/data/src/options/selectors.js index fde266ddfb4..dca2234f015 100644 --- a/packages/data/src/options/selectors.js +++ b/packages/data/src/options/selectors.js @@ -58,9 +58,10 @@ export const isRequesting = ( state, optionName ) => { * Determine if an options request resulted in an error. * * @param {Object} state - Reducer state + * @param {string} name - Option name */ -export const getOptionsRequestingError = ( state ) => { - return state.requestingError || false; +export const getOptionsRequestingError = ( state, name ) => { + return state.requestingErrors[ name ] || false; }; /** diff --git a/packages/data/src/options/test/reducer.js b/packages/data/src/options/test/reducer.js new file mode 100644 index 00000000000..27b38089d41 --- /dev/null +++ b/packages/data/src/options/test/reducer.js @@ -0,0 +1,60 @@ +/** + * Internal dependencies + */ +import reducer from '../reducer'; +import TYPES from '../action-types'; + +const defaultState = { isUpdating: false, requestingErrors: {} }; + +describe( 'options reducer', () => { + it( 'should return a default state', () => { + const state = reducer( undefined, {} ); + expect( state ).toEqual( defaultState ); + expect( state ).not.toBe( defaultState ); + } ); + + it( 'should handle RECEIVE_OPTIONS', () => { + const state = reducer( defaultState, { + type: TYPES.RECEIVE_OPTIONS, + options: { test_option: 'abc' }, + } ); + + /* eslint-disable dot-notation */ + expect( state.requestingErrors[ 'test_option' ] ).toBeUndefined(); + expect( state[ 'test_option' ] ).toBe( 'abc' ); + /* eslint-enable dot-notation */ + } ); + + it( 'should handle SET_REQUESTING_ERROR', () => { + const state = reducer( defaultState, { + type: TYPES.SET_REQUESTING_ERROR, + error: 'My bad', + name: 'test_option' + } ); + + /* eslint-disable dot-notation */ + expect( state.requestingErrors[ 'test_option' ] ).toBe( 'My bad' ); + expect( state[ 'test_option' ] ).toBeUndefined(); + /* eslint-enable dot-notation */ + } ); + + it( 'should handle SET_UPDATING_ERROR', () => { + const state = reducer( defaultState, { + type: TYPES.SET_UPDATING_ERROR, + error: 'My bad', + } ); + + expect( state.updatingError ).toBe( 'My bad' ); + expect( state.isUpdating ).toBe( false ); + } ); + + it( 'should handle SET_IS_UPDATING', () => { + const state = reducer( defaultState, { + type: TYPES.SET_IS_UPDATING, + isUpdating: true, + } ); + + expect( state.isUpdating ).toBe( true ); + } ); + +} ); \ No newline at end of file From bd5043b1bd7e41e2c3a603f6b9cac0ef64654fbf Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Thu, 4 Jun 2020 12:29:56 +1200 Subject: [PATCH 11/13] cleanup --- packages/data/src/options/controls.js | 4 +--- packages/data/src/options/resolvers.js | 2 +- packages/data/src/options/selectors.js | 16 ++++++---------- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/data/src/options/controls.js b/packages/data/src/options/controls.js index e473f905866..957150d28d2 100644 --- a/packages/data/src/options/controls.js +++ b/packages/data/src/options/controls.js @@ -35,9 +35,7 @@ export const controls = { const url = WC_ADMIN_NAMESPACE + '/options?options=' + names; fetches[ names ] = apiFetch( { path: url } ); - fetches[names].then( ( result ) => { - resolve( result[ optionName ] ); - } ) + fetches[names].then( ( result ) => resolve( result ) ) // Clear option names after all resolved; setTimeout( () => { diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 623b6da88de..4fcfb606652 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -26,7 +26,7 @@ export function* getOptionsWithRequest( names ) { export function* getOption( name ) { try { const result = yield batchFetch( name ); - yield receiveOptions( { [ name ]: result } ); + yield receiveOptions( result ); } catch ( error ) { yield setRequestingError( error, name ); } diff --git a/packages/data/src/options/selectors.js b/packages/data/src/options/selectors.js index dca2234f015..ab4ab814041 100644 --- a/packages/data/src/options/selectors.js +++ b/packages/data/src/options/selectors.js @@ -27,6 +27,12 @@ export const getOptions = ( state, names ) => { }, {} ); }; +/** + * Get option from state tree. + * + * @param {Object} state - Reducer state + * @param {Array} name - Option name + */ export const getOption = ( state, name ) => { return state[ name ]; }; @@ -44,16 +50,6 @@ export const getOptionsWithRequest = ( state, names ) => { }, {} ); }; -/** - * Determine if options are being requested. - * - * @param {Object} state - Reducer state - * @param {string} optionName - Option name - */ -export const isRequesting = ( state, optionName ) => { - return state.isRequesting[ optionName ] || false; -}; - /** * Determine if an options request resulted in an error. * From a3cd9b8160f17326bfe45304efb6be59011c0c4f Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Thu, 4 Jun 2020 12:32:54 +1200 Subject: [PATCH 12/13] add comment --- packages/data/src/options/resolvers.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/data/src/options/resolvers.js b/packages/data/src/options/resolvers.js index 4fcfb606652..87aa9860896 100644 --- a/packages/data/src/options/resolvers.js +++ b/packages/data/src/options/resolvers.js @@ -23,6 +23,11 @@ export function* getOptionsWithRequest( names ) { } } +/** + * Request an option value. + * + * @param {string} name - Option name + */ export function* getOption( name ) { try { const result = yield batchFetch( name ); From 985e4767373aa13171f2782030169d78a9947e5f Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Thu, 4 Jun 2020 12:35:32 +1200 Subject: [PATCH 13/13] return Homepage to its original --- client/homepage/index.js | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/client/homepage/index.js b/client/homepage/index.js index 386c18c6527..5cd4517a3fa 100644 --- a/client/homepage/index.js +++ b/client/homepage/index.js @@ -3,13 +3,11 @@ */ import { compose } from '@wordpress/compose'; import { Suspense, lazy } from '@wordpress/element'; -import { identity } from 'lodash'; /** * WooCommerce dependencies */ import { Spinner } from '@woocommerce/components'; -import { withOptionsHydration, OPTIONS_STORE_NAME } from '@woocommerce/data'; /** * Internal dependencies @@ -21,9 +19,7 @@ const ProfileWizard = lazy( () => import( /* webpackChunkName: "profile-wizard" */ '../profile-wizard' ) ); -const Homepage = ( { profileItems, query, blogname, blogdescription } ) => { - console.log( blogname ); - console.log( blogdescription ); +const Homepage = ( { profileItems, query } ) => { if ( isOnboardingEnabled() && ! profileItems.completed ) { return ( }> @@ -36,11 +32,6 @@ const Homepage = ( { profileItems, query, blogname, blogdescription } ) => { }; export default compose( - window.wcSettings.preloadOptions - ? withOptionsHydration( { - ...window.wcSettings.preloadOptions, - } ) - : identity, withSelect( ( select ) => { if ( ! isOnboardingEnabled() ) { return; @@ -49,19 +40,6 @@ export default compose( const { getProfileItems } = select( 'wc-api' ); const profileItems = getProfileItems(); - const { getOption, isResolving } = select( OPTIONS_STORE_NAME ); - const blogname = getOption( 'blogname' ); - const blogdescription = getOption( 'blogdescription' ); - - console.log( - 'isResolving blogname', - isResolving( 'getOption', [ 'blogname' ] ) - ); - console.log( - 'isResolving blogdescription', - isResolving( 'getOption', [ 'blogdescription' ] ) - ); - - return { profileItems, blogname, blogdescription }; + return { profileItems }; } ) )( Homepage );