Update the behavior of the cached undo/redo stack (#51644)

Co-authored-by: Ella van Durpe <ella@vandurpe.com>

docs/explanations/architecture/entities.md

@@ -56,8 +56,14 @@ For example, let's say a user edits the title of a post, followed by a modificat
 
 The store also keep tracks of a "pointer" to the current "undo/redo" step. By default, the pointer always points to the last item in the stack. This pointer is updated when the user performs an undo or redo operation.
 
-### Transient changes
+### Cached changes
 
-The undo/redo core behavior also supports what we call "transient modifications". These are modifications that are not stored in the undo/redo stack right away. For instance, when a user starts typing in a text field, the value of the field is modified in the store, but this modification is not stored in the undo/redo stack until after the user moves to the next word or after a few milliseconds. This is done to avoid creating a new undo/redo step for each character typed by the user.
+The undo/redo core behavior also supports what we call "cached modifications". These are modifications that are not stored in the undo/redo stack right away. For instance, when a user starts typing in a text field, the value of the field is modified in the store, but this modification is not stored in the undo/redo stack until after the user moves to the next word or after a few milliseconds. This is done to avoid creating a new undo/redo step for each character typed by the user.
 
-So by default, `core-data` store considers all modifications to properties that are marked as "transient" (like the `blocks` property in the post entity) as transient modifications. It keeps these modifications outside the undo/redo stack in what is called a "cache" of modifications and these modifications are only stored in the undo/redo stack when we explicitely call `__unstableCreateUndoLevel` or when the next non-transient modification is performed.
+Cached changes are kept outside the undo/redo stack in what is called a "cache" of modifications and these modifications are only stored in the undo/redo stack when we explicitely call `__unstableCreateUndoLevel` or when the next modification is not a cached one.
+
+By default all calls to `editEntityRecord` are considered "non-cached" unless the `isCached` option is passed as true. Example:
+
+```js
+wp.data.dispatch( 'core' ).editEntityRecord( 'postType', 'post', 1, { title: 'Hello World' }, { isCached: true } );
+```

packages/core-data/src/actions.js

@@ -357,7 +357,7 @@ export const editEntityRecord =
 				`The entity being edited (${ kind }, ${ name }) does not have a loaded config.`
 			);
 		}
-		const { transientEdits = {}, mergedEdits = {} } = entityConfig;
+		const { mergedEdits = {} } = entityConfig;
 		const record = select.getRawEntityRecord( kind, name, recordId );
 		const editedRecord = select.getEditedEntityRecord(
 			kind,
@@ -382,7 +382,6 @@ export const editEntityRecord =
 					: value;
 				return acc;
 			}, {} ),
-			transientEdits,
 		};
 		dispatch( {
 			type: 'EDIT_ENTITY_RECORD',
@@ -395,6 +394,7 @@ export const editEntityRecord =
 						acc[ key ] = editedRecord[ key ];
 						return acc;
 					}, {} ),
+					isCached: options.isCached,
 				},
 			},
 		} );

packages/core-data/src/entity-provider.js

@@ -7,7 +7,7 @@ import {
 	useCallback,
 	useEffect,
 } from '@wordpress/element';
-import { useSelect, useDispatch, useRegistry } from '@wordpress/data';
+import { useSelect, useDispatch } from '@wordpress/data';
 import { parse, __unstableSerializeAndClean } from '@wordpress/blocks';
 import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor';
 
@@ -154,17 +154,16 @@ export function useEntityProp( kind, name, prop, _id ) {
  * @return {[WPBlock[], Function, Function]} The block array and setters.
  */
 export function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
-	const [ meta, updateMeta ] = useEntityProp( kind, name, 'meta', _id );
-	const registry = useRegistry();
 	const providerId = useEntityId( kind, name );
 	const id = _id ?? providerId;
-	const { content, blocks } = useSelect(
+	const { content, blocks, meta } = useSelect(
 		( select ) => {
 			const { getEditedEntityRecord } = select( STORE_NAME );
 			const editedRecord = getEditedEntityRecord( kind, name, id );
 			return {
 				blocks: editedRecord.blocks,
 				content: editedRecord.content,
+				meta: editedRecord.meta,
 			};
 		},
 		[ kind, name, id ]
@@ -194,7 +193,7 @@ export function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
 		( _blocks ) => {
 			if ( ! meta ) return;
 			// If meta.footnotes is empty, it means the meta is not registered.
-			if ( meta.footnotes === undefined ) return;
+			if ( meta.footnotes === undefined ) return {};
 
 			const { getRichTextValues } = unlock( blockEditorPrivateApis );
 			const _content = getRichTextValues( _blocks ).join( '' ) || '';
@@ -237,48 +236,57 @@ export function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
 				}, {} ),
 			};
 
-			updateMeta( {
-				...meta,
-				footnotes: JSON.stringify( newFootnotes ),
-			} );
+			return {
+				meta: {
+					...meta,
+					footnotes: JSON.stringify( newFootnotes ),
+				},
+			};
 		},
-		[ meta, updateMeta ]
+		[ meta ]
 	);
 
 	const onChange = useCallback(
 		( newBlocks, options ) => {
-			const { selection } = options;
-			const edits = { blocks: newBlocks, selection };
-
-			const noChange = blocks === edits.blocks;
+			const noChange = blocks === newBlocks;
 			if ( noChange ) {
 				return __unstableCreateUndoLevel( kind, name, id );
 			}
+			const { selection } = options;
 
 			// We create a new function here on every persistent edit
 			// to make sure the edit makes the post dirty and creates
 			// a new undo level.
-			edits.content = ( { blocks: blocksForSerialization = [] } ) =>
-				__unstableSerializeAndClean( blocksForSerialization );
+			const edits = {
+				blocks: newBlocks,
+				selection,
+				content: ( { blocks: blocksForSerialization = [] } ) =>
+					__unstableSerializeAndClean( blocksForSerialization ),
+				...updateFootnotes( newBlocks ),
+			};
 
-			registry.batch( () => {
-				updateFootnotes( edits.blocks );
-				editEntityRecord( kind, name, id, edits );
-			} );
+			editEntityRecord( kind, name, id, edits, { isCached: false } );
 		},
-		[ kind, name, id, blocks, updateFootnotes ]
+		[
+			kind,
+			name,
+			id,
+			blocks,
+			updateFootnotes,
+			__unstableCreateUndoLevel,
+			editEntityRecord,
+		]
 	);
 
 	const onInput = useCallback(
 		( newBlocks, options ) => {
 			const { selection } = options;
-			const edits = { blocks: newBlocks, selection };
-			registry.batch( () => {
-				updateFootnotes( edits.blocks );
-				editEntityRecord( kind, name, id, edits );
-			} );
+			const footnotesChanges = updateFootnotes( newBlocks );
+			const edits = { blocks: newBlocks, selection, ...footnotesChanges };
+
+			editEntityRecord( kind, name, id, edits, { isCached: true } );
 		},
-		[ kind, name, id, updateFootnotes ]
+		[ kind, name, id, updateFootnotes, editEntityRecord ]
 	);
 
 	return [ blocks ?? EMPTY_ARRAY, onInput, onChange ];

packages/core-data/src/reducer.js

@@ -439,7 +439,7 @@ export const entities = ( state = {}, action ) => {
  *
  * @property {number} list   The undo stack.
  * @property {number} offset Where in the undo stack we are.
- * @property {Object} cache  Cache of unpersisted transient edits.
+ * @property {Object} cache  Cache of unpersisted edits.
  */
 
 /** @typedef {Array<Object> & UndoStateMeta} UndoState */
@@ -543,10 +543,6 @@ export function undo( state = UNDO_INITIAL_STATE, action ) {
 				return state;
 			}
 
-			const isCachedChange = Object.keys( action.edits ).every(
-				( key ) => action.transientEdits[ key ]
-			);
-
 			const edits = Object.keys( action.edits ).map( ( key ) => {
 				return {
 					kind: action.kind,
@@ -558,7 +554,7 @@ export function undo( state = UNDO_INITIAL_STATE, action ) {
 				};
 			} );
 
-			if ( isCachedChange ) {
+			if ( action.meta.undo.isCached ) {
 				return {
 					...state,
 					cache: edits.reduce( appendEditToStack, state.cache ),

packages/core-data/src/test/reducer.js

@@ -155,19 +155,21 @@ describe( 'undo', () => {
 		from,
 		to,
 	} );
-	const createNextEditAction = ( edits, transientEdits = {} ) => {
+	const createNextEditAction = ( edits, isCached ) => {
 		let action = {
 			kind: 'someKind',
 			name: 'someName',
 			recordId: 'someRecordId',
 			edits,
-			transientEdits,
 		};
 		action = {
 			type: 'EDIT_ENTITY_RECORD',
 			...action,
 			meta: {
-				undo: { edits: lastValues },
+				undo: {
+					isCached,
+					edits: lastValues,
+				},
 			},
 		};
 		lastValues = { ...lastValues, ...edits };
@@ -303,10 +305,7 @@ describe( 'undo', () => {
 	it( 'handles flattened undos/redos', () => {
 		undoState = createNextUndoState();
 		undoState = createNextUndoState( { value: 1 } );
-		undoState = createNextUndoState(
-			{ transientValue: 2 },
-			{ transientValue: true }
-		);
+		undoState = createNextUndoState( { transientValue: 2 }, true );
 		undoState = createNextUndoState( { value: 3 } );
 		expectedUndoState.list.push(
 			[
@@ -335,10 +334,7 @@ describe( 'undo', () => {
 
 		// Check that transient edits are merged into the last
 		// edits.
-		undoState = createNextUndoState(
-			{ transientValue: 2 },
-			{ transientValue: true }
-		);
+		undoState = createNextUndoState( { transientValue: 2 }, true );
 		undoState = createNextUndoState( 'isCreate' );
 		expectedUndoState.list[ expectedUndoState.list.length - 1 ].push(
 			createExpectedDiff( 'transientValue', { from: undefined, to: 2 } )
@@ -359,10 +355,7 @@ describe( 'undo', () => {
 	it( 'explicitly creates an undo level when undoing while there are pending transient edits', () => {
 		undoState = createNextUndoState();
 		undoState = createNextUndoState( { value: 1 } );
-		undoState = createNextUndoState(
-			{ transientValue: 2 },
-			{ transientValue: true }
-		);
+		undoState = createNextUndoState( { transientValue: 2 }, true );
 		undoState = createNextUndoState( 'isUndo' );
 		expectedUndoState.list.push( [
 			createExpectedDiff( 'value', { from: undefined, to: 1 } ),

packages/data/src/registry.js

@@ -314,6 +314,12 @@ export function createRegistry( storeConfigs = {}, parent = null ) {
 	}
 
 	function batch( callback ) {
+		// If we're already batching, just call the callback.
+		if ( emitter.isPaused ) {
+			callback();
+			return;
+		}
+
 		emitter.pause();
 		Object.values( stores ).forEach( ( store ) => store.emitter.pause() );
 		callback();

packages/data/src/test/registry.js

@@ -734,6 +734,27 @@ describe( 'createRegistry', () => {
 			unsubscribe();
 			expect( listener2 ).toHaveBeenCalledTimes( 1 );
 		} );
+
+		it( 'should support nested batches', () => {
+			const store = registry.registerStore( 'myAwesomeReducer', {
+				reducer: ( state = 0 ) => state + 1,
+			} );
+			const listener = jest.fn();
+			subscribeWithUnsubscribe( listener );
+
+			registry.batch( () => {} );
+			expect( listener ).not.toHaveBeenCalled();
+
+			registry.batch( () => {
+				store.dispatch( { type: 'dummy' } );
+				registry.batch( () => {
+					store.dispatch( { type: 'dummy' } );
+					store.dispatch( { type: 'dummy' } );
+				} );
+				store.dispatch( { type: 'dummy' } );
+			} );
+			expect( listener ).toHaveBeenCalledTimes( 1 );
+		} );
 	} );
 
 	describe( 'use', () => {