-
Notifications
You must be signed in to change notification settings - Fork 144
Try: Collect async resolver calls #4444
Changes from 6 commits
1de7647
c689874
e5c3573
47f149e
c01969f
6c19881
f24f603
a34115d
5f1316d
1f603d9
88bf237
bd5043b
a3cd9b8
985e476
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 ); | ||
} ); | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
/** | ||
* External Dependencies | ||
*/ | ||
|
||
import { apiFetch } from '@wordpress/data-controls'; | ||
import { getOptionsToRequest } from './controls'; | ||
|
||
/** | ||
* Internal dependencies | ||
|
@@ -23,3 +23,23 @@ export function* getOptionsWithRequest( names ) { | |
return error; | ||
} | ||
} | ||
|
||
const fetches = {}; | ||
|
||
export function* getOption( name ) { | ||
yield setIsRequesting( name ); | ||
const names = yield getOptionsToRequest( name ); | ||
|
||
const fetchInProgress = fetches[ names ]; | ||
if ( fetchInProgress ) { | ||
return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a heads up that I added this back in since we need to return early here. Returning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As Paul pointed out, this will cause the built in cached resolver for |
||
} | ||
|
||
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 ]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is a bit ambiguous. Are you talking about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Up for rewording this comment, but I believe it's accurate. We need to delete the fetch from our local variable to allow (keyword) wp data to handle invalidation. It does not invalidate cache directly, just allows it. If we don't remove this fetch and we want to invalidate this cache using |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could probably just eliminate the declaration of
fetchInProgress
here and usefetches[names]
directly in the condition.