diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md index 7ad08fa7f9ff22..cfecc33470ab6c 100644 --- a/docs/reference-guides/data/data-core.md +++ b/docs/reference-guides/data/data-core.md @@ -702,7 +702,7 @@ _Parameters_ - _record_ `Object`: Record to be saved. - _options_ `Object`: Saving options. - _options.isAutosave_ `[boolean]`: Whether this is an autosave. -- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a control descriptor. +- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a promise. ### undo diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 96f6c1e300404e..4e906e79bb1be3 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -236,7 +236,7 @@ _Parameters_ - _record_ `Object`: Record to be saved. - _options_ `Object`: Saving options. - _options.isAutosave_ `[boolean]`: Whether this is an autosave. -- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a control descriptor. +- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a promise. ### undo diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index 6c3d392c31210b..fcc41bf5dd6cbf 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -9,6 +9,7 @@ import { v4 as uuid } from 'uuid'; */ import { controls } from '@wordpress/data'; import { apiFetch, __unstableAwaitPromise } from '@wordpress/data-controls'; +import triggerFetch from '@wordpress/api-fetch'; import { addQueryArgs } from '@wordpress/url'; /** @@ -357,16 +358,15 @@ export function __unstableCreateUndoLevel() { * @param {boolean} [options.isAutosave=false] Whether this is an autosave. * @param {Function} [options.__unstableFetch] Internal use only. Function to * call instead of `apiFetch()`. - * Must return a control - * descriptor. + * Must return a promise. */ -export function* saveEntityRecord( +export const saveEntityRecord = ( kind, name, record, - { isAutosave = false, __unstableFetch = null } = {} -) { - const entities = yield getKindEntities( kind ); + { isAutosave = false, __unstableFetch = triggerFetch } = {} +) => async ( { select, dispatch } ) => { + const entities = await dispatch( getKindEntities( kind ) ); const entity = find( entities, { kind, name } ); if ( ! entity ) { return; @@ -374,10 +374,12 @@ export function* saveEntityRecord( const entityIdKey = entity.key || DEFAULT_ENTITY_KEY; const recordId = record[ entityIdKey ]; - const lock = yield* __unstableAcquireStoreLock( - STORE_NAME, - [ 'entities', 'data', kind, name, recordId || uuid() ], - { exclusive: true } + const lock = await dispatch( + __unstableAcquireStoreLock( + STORE_NAME, + [ 'entities', 'data', kind, name, recordId || uuid() ], + { exclusive: true } + ) ); try { // Evaluate optimized edits. @@ -385,15 +387,9 @@ export function* saveEntityRecord( for ( const [ key, value ] of Object.entries( record ) ) { if ( typeof value === 'function' ) { const evaluatedValue = value( - yield controls.select( - STORE_NAME, - 'getEditedEntityRecord', - kind, - name, - recordId - ) + select.getEditedEntityRecord( kind, name, recordId ) ); - yield editEntityRecord( + await dispatch.editEntityRecord( kind, name, recordId, @@ -406,22 +402,20 @@ export function* saveEntityRecord( } } - yield { + await dispatch( { type: 'SAVE_ENTITY_RECORD_START', kind, name, recordId, isAutosave, - }; + } ); let updatedRecord; let error; try { const path = `${ entity.baseURL }${ recordId ? '/' + recordId : '' }`; - const persistedRecord = yield controls.select( - STORE_NAME, - 'getRawEntityRecord', + const persistedRecord = select.getRawEntityRecord( kind, name, recordId @@ -432,14 +426,9 @@ export function* saveEntityRecord( // This is fine for now as it is the only supported autosave, // but ideally this should all be handled in the back end, // so the client just sends and receives objects. - const currentUser = yield controls.select( - STORE_NAME, - 'getCurrentUser' - ); + const currentUser = select.getCurrentUser(); const currentUserId = currentUser ? currentUser.id : undefined; - const autosavePost = yield controls.select( - STORE_NAME, - 'getAutosave', + const autosavePost = select.getAutosave( persistedRecord.type, persistedRecord.id, currentUserId @@ -471,13 +460,8 @@ export function* saveEntityRecord( method: 'POST', data, }; - if ( __unstableFetch ) { - updatedRecord = yield __unstableAwaitPromise( - __unstableFetch( options ) - ); - } else { - updatedRecord = yield apiFetch( options ); - } + updatedRecord = await __unstableFetch( options ); + // An autosave may be processed by the server as a regular save // when its update is requested by the author and the post had // draft or auto-draft status. @@ -521,7 +505,7 @@ export function* saveEntityRecord( }, {} ); - yield receiveEntityRecords( + await dispatch.receiveEntityRecords( kind, name, newRecord, @@ -529,7 +513,10 @@ export function* saveEntityRecord( true ); } else { - yield receiveAutosaves( persistedRecord.id, updatedRecord ); + await dispatch.receiveAutosaves( + persistedRecord.id, + updatedRecord + ); } } else { let edits = record; @@ -547,14 +534,8 @@ export function* saveEntityRecord( method: recordId ? 'PUT' : 'POST', data: edits, }; - if ( __unstableFetch ) { - updatedRecord = yield __unstableAwaitPromise( - __unstableFetch( options ) - ); - } else { - updatedRecord = yield apiFetch( options ); - } - yield receiveEntityRecords( + updatedRecord = await __unstableFetch( options ); + await dispatch.receiveEntityRecords( kind, name, updatedRecord, @@ -566,20 +547,20 @@ export function* saveEntityRecord( } catch ( _error ) { error = _error; } - yield { + dispatch( { type: 'SAVE_ENTITY_RECORD_FINISH', kind, name, recordId, error, isAutosave, - }; + } ); return updatedRecord; } finally { - yield* __unstableReleaseStoreLock( lock ); + await dispatch( __unstableReleaseStoreLock( lock ) ); } -} +}; /** * Runs multiple core-data actions at the same time using one API request. @@ -658,28 +639,23 @@ export function* __experimentalBatch( requests ) { * @param {Object} recordId ID of the record. * @param {Object} options Saving options. */ -export function* saveEditedEntityRecord( kind, name, recordId, options ) { - if ( - ! ( yield controls.select( - STORE_NAME, - 'hasEditsForEntityRecord', - kind, - name, - recordId - ) ) - ) { +export const saveEditedEntityRecord = ( + kind, + name, + recordId, + options +) => async ( { select, dispatch } ) => { + if ( ! select.hasEditsForEntityRecord( kind, name, recordId ) ) { return; } - const edits = yield controls.select( - STORE_NAME, - 'getEntityRecordNonTransientEdits', + const edits = select.getEntityRecordNonTransientEdits( kind, name, recordId ); const record = { id: recordId, ...edits }; - return yield* saveEntityRecord( kind, name, record, options ); -} + return await dispatch.saveEntityRecord( kind, name, record, options ); +}; /** * Action triggered to save only specified properties for the entity. @@ -690,27 +666,17 @@ export function* saveEditedEntityRecord( kind, name, recordId, options ) { * @param {Array} itemsToSave List of entity properties to save. * @param {Object} options Saving options. */ -export function* __experimentalSaveSpecifiedEntityEdits( +export const __experimentalSaveSpecifiedEntityEdits = ( kind, name, recordId, itemsToSave, options -) { - if ( - ! ( yield controls.select( - STORE_NAME, - 'hasEditsForEntityRecord', - kind, - name, - recordId - ) ) - ) { +) => async ( { select, dispatch } ) => { + if ( ! select.hasEditsForEntityRecord( kind, name, recordId ) ) { return; } - const edits = yield controls.select( - STORE_NAME, - 'getEntityRecordNonTransientEdits', + const edits = select.getEntityRecordNonTransientEdits( kind, name, recordId @@ -721,8 +687,8 @@ export function* __experimentalSaveSpecifiedEntityEdits( editsToSave[ edit ] = edits[ edit ]; } } - return yield* saveEntityRecord( kind, name, editsToSave, options ); -} + return await dispatch.saveEntityRecord( kind, name, editsToSave, options ); +}; /** * Returns an action object used in signalling that Upload permissions have been received. diff --git a/packages/core-data/src/index.js b/packages/core-data/src/index.js index 4830da46ccc98f..43b9cadb1bc775 100644 --- a/packages/core-data/src/index.js +++ b/packages/core-data/src/index.js @@ -63,6 +63,7 @@ const storeConfig = { actions: { ...actions, ...entityActions, ...locksActions }, selectors: { ...selectors, ...entitySelectors, ...locksSelectors }, resolvers: { ...resolvers, ...entityResolvers }, + __experimentalUseThunks: true, }; /** diff --git a/packages/core-data/src/test/actions.js b/packages/core-data/src/test/actions.js index 6dfec991e2a03d..8ee805c489853c 100644 --- a/packages/core-data/src/test/actions.js +++ b/packages/core-data/src/test/actions.js @@ -2,6 +2,9 @@ * WordPress dependencies */ import { controls } from '@wordpress/data'; +import apiFetch from '@wordpress/api-fetch'; + +jest.mock( '@wordpress/api-fetch' ); /** * Internal dependencies @@ -10,7 +13,6 @@ import { editEntityRecord, saveEntityRecord, deleteEntityRecord, - receiveEntityRecords, receiveUserPermission, receiveAutosaves, receiveCurrentUser, @@ -106,54 +108,83 @@ describe( 'deleteEntityRecord', () => { } ); describe( 'saveEntityRecord', () => { + beforeEach( async () => { + apiFetch.mockReset(); + jest.useFakeTimers(); + } ); + it( 'triggers a POST request for a new record', async () => { const post = { title: 'new post' }; const entities = [ { name: 'post', kind: 'postType', baseURL: '/wp/v2/posts' }, ]; - const fulfillment = saveEntityRecord( 'postType', 'post', post ); - // Trigger generator - fulfillment.next(); + const select = { + getRawEntityRecord: () => post, + }; - // Provide entities and acquire lock - expect( fulfillment.next( entities ).value.type ).toBe( - 'MOCKED_ACQUIRE_LOCK' - ); + const dispatch = Object.assign( jest.fn(), { + receiveEntityRecords: jest.fn(), + } ); + // Provide entities + dispatch.mockReturnValueOnce( entities ); - // Trigger apiFetch - expect( fulfillment.next().value.type ).toEqual( - 'SAVE_ENTITY_RECORD_START' - ); + // Provide response + const updatedRecord = { ...post, id: 10 }; + apiFetch.mockImplementation( () => { + return updatedRecord; + } ); - expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' ); - const { value: apiFetchAction } = fulfillment.next( {} ); - expect( apiFetchAction.request ).toEqual( { + const result = await saveEntityRecord( + 'postType', + 'post', + post + )( { select, dispatch } ); + + expect( apiFetch ).toHaveBeenCalledTimes( 1 ); + expect( apiFetch ).toHaveBeenCalledWith( { path: '/wp/v2/posts', method: 'POST', data: post, } ); - // Provide response and trigger action - const updatedRecord = { ...post, id: 10 }; - const { value: received } = fulfillment.next( updatedRecord ); - expect( received ).toEqual( - receiveEntityRecords( - 'postType', - 'post', - updatedRecord, - undefined, - true, - { title: 'new post' } - ) - ); - expect( fulfillment.next().value.type ).toBe( - 'SAVE_ENTITY_RECORD_FINISH' - ); - // Release lock - expect( fulfillment.next().value.type ).toEqual( - 'MOCKED_RELEASE_LOCK' + + expect( dispatch ).toHaveBeenCalledTimes( 5 ); + expect( dispatch ).toHaveBeenCalledWith( { + type: 'SAVE_ENTITY_RECORD_START', + kind: 'postType', + name: 'post', + recordId: undefined, + isAutosave: false, + } ); + expect( dispatch ).toHaveBeenCalledWith( [ + { + type: 'MOCKED_ACQUIRE_LOCK', + }, + ] ); + expect( dispatch ).toHaveBeenCalledWith( { + type: 'SAVE_ENTITY_RECORD_FINISH', + kind: 'postType', + name: 'post', + recordId: undefined, + error: undefined, + isAutosave: false, + } ); + expect( dispatch ).toHaveBeenCalledWith( [ + { + type: 'MOCKED_RELEASE_LOCK', + }, + ] ); + + expect( dispatch.receiveEntityRecords ).toHaveBeenCalledTimes( 1 ); + expect( dispatch.receiveEntityRecords ).toHaveBeenCalledWith( + 'postType', + 'post', + updatedRecord, + undefined, + true, + post ); - expect( fulfillment.next().value ).toBe( updatedRecord ); + expect( result ).toBe( updatedRecord ); } ); it( 'triggers a PUT request for an existing record', async () => { @@ -161,41 +192,73 @@ describe( 'saveEntityRecord', () => { const entities = [ { name: 'post', kind: 'postType', baseURL: '/wp/v2/posts' }, ]; - const fulfillment = saveEntityRecord( 'postType', 'post', post ); - // Trigger generator - fulfillment.next(); + const select = { + getRawEntityRecord: () => post, + }; - // Provide entities and acquire lock - expect( fulfillment.next( entities ).value.type ).toBe( - 'MOCKED_ACQUIRE_LOCK' - ); + const dispatch = Object.assign( jest.fn(), { + receiveEntityRecords: jest.fn(), + } ); + // Provide entities + dispatch.mockReturnValueOnce( entities ); - // Trigger apiFetch - expect( fulfillment.next().value.type ).toEqual( - 'SAVE_ENTITY_RECORD_START' - ); - expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' ); - const { value: apiFetchAction } = fulfillment.next( {} ); - expect( apiFetchAction.request ).toEqual( { + // Provide response + const updatedRecord = { ...post, id: 10 }; + apiFetch.mockImplementation( () => { + return updatedRecord; + } ); + + const result = await saveEntityRecord( + 'postType', + 'post', + post + )( { select, dispatch } ); + + expect( apiFetch ).toHaveBeenCalledTimes( 1 ); + expect( apiFetch ).toHaveBeenCalledWith( { path: '/wp/v2/posts/10', method: 'PUT', data: post, } ); - // Provide response and trigger action - const { value: received } = fulfillment.next( post ); - expect( received ).toEqual( - receiveEntityRecords( 'postType', 'post', post, undefined, true, { - title: 'new post', - id: 10, - } ) - ); - expect( fulfillment.next().value.type ).toBe( - 'SAVE_ENTITY_RECORD_FINISH' - ); - // Release lock - expect( fulfillment.next().value.type ).toEqual( - 'MOCKED_RELEASE_LOCK' + + expect( dispatch ).toHaveBeenCalledTimes( 5 ); + expect( dispatch ).toHaveBeenCalledWith( { + type: 'SAVE_ENTITY_RECORD_START', + kind: 'postType', + name: 'post', + recordId: 10, + isAutosave: false, + } ); + expect( dispatch ).toHaveBeenCalledWith( [ + { + type: 'MOCKED_ACQUIRE_LOCK', + }, + ] ); + expect( dispatch ).toHaveBeenCalledWith( { + type: 'SAVE_ENTITY_RECORD_FINISH', + kind: 'postType', + name: 'post', + recordId: 10, + error: undefined, + isAutosave: false, + } ); + expect( dispatch ).toHaveBeenCalledWith( [ + { + type: 'MOCKED_RELEASE_LOCK', + }, + ] ); + + expect( dispatch.receiveEntityRecords ).toHaveBeenCalledTimes( 1 ); + expect( dispatch.receiveEntityRecords ).toHaveBeenCalledWith( + 'postType', + 'post', + updatedRecord, + undefined, + true, + post ); + + expect( result ).toBe( updatedRecord ); } ); it( 'triggers a PUT request for an existing record with a custom key', async () => { @@ -208,45 +271,70 @@ describe( 'saveEntityRecord', () => { key: 'slug', }, ]; - const fulfillment = saveEntityRecord( 'root', 'postType', postType ); - // Trigger generator - fulfillment.next(); + const select = { + getRawEntityRecord: () => ( {} ), + }; - // Provide entities and acquire lock - expect( fulfillment.next( entities ).value.type ).toBe( - 'MOCKED_ACQUIRE_LOCK' - ); + const dispatch = Object.assign( jest.fn(), { + receiveEntityRecords: jest.fn(), + } ); + // Provide entities + dispatch.mockReturnValueOnce( entities ); - // Trigger apiFetch - expect( fulfillment.next().value.type ).toEqual( - 'SAVE_ENTITY_RECORD_START' - ); - expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' ); - const { value: apiFetchAction } = fulfillment.next( {} ); - expect( apiFetchAction.request ).toEqual( { + // Provide response + apiFetch.mockImplementation( () => postType ); + + const result = await saveEntityRecord( + 'root', + 'postType', + postType + )( { select, dispatch } ); + + expect( apiFetch ).toHaveBeenCalledTimes( 1 ); + expect( apiFetch ).toHaveBeenCalledWith( { path: '/wp/v2/types/page', method: 'PUT', data: postType, } ); - // Provide response and trigger action - const { value: received } = fulfillment.next( postType ); - expect( received ).toEqual( - receiveEntityRecords( - 'root', - 'postType', - postType, - undefined, - true, - { slug: 'page', title: 'Pages' } - ) - ); - expect( fulfillment.next().value.type ).toBe( - 'SAVE_ENTITY_RECORD_FINISH' - ); - // Release lock - expect( fulfillment.next().value.type ).toEqual( - 'MOCKED_RELEASE_LOCK' + + expect( dispatch ).toHaveBeenCalledTimes( 5 ); + expect( dispatch ).toHaveBeenCalledWith( { + type: 'SAVE_ENTITY_RECORD_START', + kind: 'root', + name: 'postType', + recordId: 'page', + isAutosave: false, + } ); + expect( dispatch ).toHaveBeenCalledWith( [ + { + type: 'MOCKED_ACQUIRE_LOCK', + }, + ] ); + expect( dispatch ).toHaveBeenCalledWith( { + type: 'SAVE_ENTITY_RECORD_FINISH', + kind: 'root', + name: 'postType', + recordId: 'page', + error: undefined, + isAutosave: false, + } ); + expect( dispatch ).toHaveBeenCalledWith( [ + { + type: 'MOCKED_RELEASE_LOCK', + }, + ] ); + + expect( dispatch.receiveEntityRecords ).toHaveBeenCalledTimes( 1 ); + expect( dispatch.receiveEntityRecords ).toHaveBeenCalledWith( + 'root', + 'postType', + postType, + undefined, + true, + { slug: 'page', title: 'Pages' } ); + + expect( result ).toBe( postType ); } ); } ); diff --git a/packages/core-data/src/test/integration.js b/packages/core-data/src/test/integration.js index 43fbdaeb27bd97..6eb1645c506fc7 100644 --- a/packages/core-data/src/test/integration.js +++ b/packages/core-data/src/test/integration.js @@ -213,6 +213,11 @@ describe( 'saveEntityRecord', () => { slug: 'post-1', newField: 'a', } ); + + // Wait a few ticks – without rungen we have less control over the flow of things. + // @TODO: A better solution + await runPendingPromises(); + await runPendingPromises(); await runPendingPromises(); // There should ONLY be a single hanging API call (PUT) by this point. diff --git a/packages/core-data/src/utils/if-not-resolved.js b/packages/core-data/src/utils/if-not-resolved.js index 0baf36f0e5d8d9..653827cdfe6c02 100644 --- a/packages/core-data/src/utils/if-not-resolved.js +++ b/packages/core-data/src/utils/if-not-resolved.js @@ -1,13 +1,3 @@ -/** - * WordPress dependencies - */ -import { controls } from '@wordpress/data'; - -/** - * Internal dependencies - */ -import { STORE_NAME } from '../name'; - /** * Higher-order function which invokes the given resolver only if it has not * already been resolved with the arguments passed to the enhanced function. @@ -20,21 +10,13 @@ import { STORE_NAME } from '../name'; * * @return {Function} Enhanced resolver. */ -const ifNotResolved = ( resolver, selectorName ) => - /** - * @param {...any} args Original resolver arguments. - */ - function* resolveIfNotResolved( ...args ) { - const hasStartedResolution = yield controls.select( - STORE_NAME, - 'hasStartedResolution', - selectorName, - args - ); - - if ( ! hasStartedResolution ) { - yield* resolver( ...args ); - } - }; +const ifNotResolved = ( resolver, selectorName ) => ( ...args ) => async ( { + select, + dispatch, +} ) => { + if ( ! select.hasStartedResolution( selectorName, args ) ) { + await dispatch( resolver( ...args ) ); + } +}; export default ifNotResolved; diff --git a/packages/core-data/src/utils/test/if-not-resolved.js b/packages/core-data/src/utils/test/if-not-resolved.js index a773097d70514f..291e224b3833cd 100644 --- a/packages/core-data/src/utils/test/if-not-resolved.js +++ b/packages/core-data/src/utils/test/if-not-resolved.js @@ -27,49 +27,50 @@ describe( 'ifNotResolved', () => { expect( resolver ).toBeInstanceOf( Function ); } ); - it( 'triggers original resolver if not already resolved', () => { - controls.select.mockImplementation( ( _storeKey, selectorName ) => ( { - _nextValue: - selectorName === 'hasStartedResolution' ? false : undefined, - } ) ); + it( 'triggers original resolver if not already resolved', async () => { + const select = { hasStartedResolution: () => false }; + const dispatch = () => {}; const originalResolver = jest .fn() - .mockImplementation( function* () {} ); + .mockImplementation( async function () {} ); const resolver = ifNotResolved( originalResolver, 'originalResolver' ); - - const runResolver = resolver(); - - let next, nextValue; - do { - next = runResolver.next( nextValue ); - nextValue = next.value?._nextValue; - } while ( ! next.done ); + await resolver()( { select, dispatch } ); expect( originalResolver ).toHaveBeenCalledTimes( 1 ); } ); - it( 'does not trigger original resolver if already resolved', () => { - controls.select.mockImplementation( ( _storeKey, selectorName ) => ( { - _nextValue: - selectorName === 'hasStartedResolution' ? true : undefined, - } ) ); + it( 'does not trigger original resolver if already resolved', async () => { + const select = { hasStartedResolution: () => true }; + const dispatch = () => {}; const originalResolver = jest .fn() - .mockImplementation( function* () {} ); + .mockImplementation( async function () {} ); const resolver = ifNotResolved( originalResolver, 'originalResolver' ); + await resolver()( { select, dispatch } ); + + expect( originalResolver ).toHaveBeenCalledTimes( 0 ); + } ); - const runResolver = resolver(); + it( 'returns a promise when the resolver was not already resolved', async () => { + const select = { hasStartedResolution: () => false }; + let thunkRetval; + const dispatch = jest.fn( ( thunk ) => { + thunkRetval = thunk(); + return thunkRetval; + } ); - let next, nextValue; - do { - next = runResolver.next( nextValue ); - nextValue = next.value?._nextValue; - } while ( ! next.done ); + const originalResolver = jest.fn( () => () => + Promise.resolve( 'success!' ) + ); - expect( originalResolver ).toHaveBeenCalledTimes( 0 ); + const resolver = ifNotResolved( originalResolver, 'originalResolver' ); + const result = resolver()( { select, dispatch } ); + + await expect( result ).resolves.toBe( undefined ); + await expect( thunkRetval ).resolves.toBe( 'success!' ); } ); } );