Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor saveEntityRecord from redux-rungen to async thunks #33201

Merged
merged 14 commits into from
Aug 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/reference-guides/data/data-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion packages/core-data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
130 changes: 48 additions & 82 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -357,43 +358,38 @@ 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;
}
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.
// (Function edits that should be evaluated on save to avoid expensive computations on every edit.)
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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My version doesn't do await here, because editEntityRecord is a synchronous action. It merely writes things to memory (updating state) and never does any request or anything else async.

kind,
name,
recordId,
Expand All @@ -406,22 +402,20 @@ export function* saveEntityRecord(
}
}

yield {
await dispatch( {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This action is also purely synchronous.

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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -521,15 +505,18 @@ export function* saveEntityRecord(
},
{}
);
yield receiveEntityRecords(
await dispatch.receiveEntityRecords(
kind,
name,
newRecord,
undefined,
true
);
} else {
yield receiveAutosaves( persistedRecord.id, updatedRecord );
await dispatch.receiveAutosaves(
persistedRecord.id,
updatedRecord
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both receiveEntityRecords and receiveAutosaves are synchronous: they merely write the new/updated records to state.

}
} else {
let edits = record;
Expand All @@ -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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sync.

kind,
name,
updatedRecord,
Expand All @@ -566,20 +547,20 @@ export function* saveEntityRecord(
} catch ( _error ) {
error = _error;
}
yield {
dispatch( {
type: 'SAVE_ENTITY_RECORD_FINISH',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also sync.

kind,
name,
recordId,
error,
isAutosave,
};
} );

return updatedRecord;
} finally {
yield* __unstableReleaseStoreLock( lock );
await dispatch( __unstableReleaseStoreLock( lock ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlocking is sync.

}
}
};

/**
* Runs multiple core-data actions at the same time using one API request.
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand Down
1 change: 1 addition & 0 deletions packages/core-data/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const storeConfig = {
actions: { ...actions, ...entityActions, ...locksActions },
selectors: { ...selectors, ...entitySelectors, ...locksSelectors },
resolvers: { ...resolvers, ...entityResolvers },
__experimentalUseThunks: true,
};

/**
Expand Down
Loading