Block Editor: Consider received blocks state change as ignored (#14916)

* Block Editor: Avoid creating new state reference on unchanging isPersistentChange

* Block Editor: Consider received blocks state change as ignored

* fixup a35fa82

* Block Editor: Regenerate documentation

docs/designers-developers/developers/data/data-core-block-editor.md

@@ -775,6 +775,20 @@ via its `onChange` callback, in addition to `onInput`.
 
 Whether the most recent block change was persistent.
 
+### __unstableIsLastBlockChangeIgnored
+
+Returns true if the most recent block change is be considered ignored, or
+false otherwise. An ignored change is one not to be committed by
+BlockEditorProvider, neither via `onChange` nor `onInput`.
+
+*Parameters*
+
+ * state: Block editor state.
+
+*Returns*
+
+Whether the most recent block change was ignored.
+
 ## Actions
 
 ### resetBlocks

packages/block-editor/CHANGELOG.md

@@ -15,6 +15,11 @@
 
 - `CopyHandler` will now only catch cut/copy events coming from its `props.children`, instead of from anywhere in the `document`.
 
+### Internal
+
+- Improved handling of blocks state references for unchanging states.
+- Updated handling of blocks state to effectively ignored programmatically-received blocks data (e.g. reusable blocks received from editor).
+
 ## 1.0.0 (2019-03-06)
 
 ### New Features

packages/block-editor/src/components/provider/index.js

@@ -69,6 +69,7 @@ class BlockEditorProvider extends Component {
 		const {
 			getBlocks,
 			isLastBlockChangePersistent,
+			__unstableIsLastBlockChangeIgnored,
 		} = registry.select( 'core/block-editor' );
 
 		let blocks = getBlocks();
@@ -81,7 +82,12 @@ class BlockEditorProvider extends Component {
 			} = this.props;
 			const newBlocks = getBlocks();
 			const newIsPersistent = isLastBlockChangePersistent();
-			if ( newBlocks !== blocks && this.isSyncingIncomingValue ) {
+			if (
+				newBlocks !== blocks && (
+					this.isSyncingIncomingValue ||
+					__unstableIsLastBlockChangeIgnored()
+				)
+			) {
 				this.isSyncingIncomingValue = false;
 				blocks = newBlocks;
 				isPersistent = newIsPersistent;

packages/block-editor/src/store/reducer.js

@@ -188,16 +188,6 @@ export function isUpdatingSameBlockAttribute( action, lastAction ) {
 function withPersistentBlockChange( reducer ) {
 	let lastAction;
 
-	/**
-	 * Set of action types for which a blocks state change should be considered
-	 * non-persistent.
-	 *
-	 * @type {Set}
-	 */
-	const IGNORED_ACTION_TYPES = new Set( [
-		'RECEIVE_BLOCKS',
-	] );
-
 	return ( state, action ) => {
 		let nextState = reducer( state, action );
 
@@ -206,19 +196,14 @@ function withPersistentBlockChange( reducer ) {
 		// Defer to previous state value (or default) unless changing or
 		// explicitly marking as persistent.
 		if ( state === nextState && ! isExplicitPersistentChange ) {
-			return {
-				...nextState,
-				isPersistentChange: get( state, [ 'isPersistentChange' ], true ),
-			};
-		}
+			const nextIsPersistentChange = get( state, [ 'isPersistentChange' ], true );
+			if ( state.isPersistentChange === nextIsPersistentChange ) {
+				return state;
+			}
 
-		// Some state changes should not be considered persistent, namely those
-		// which are not a direct result of user interaction.
-		const isIgnoredActionType = IGNORED_ACTION_TYPES.has( action.type );
-		if ( isIgnoredActionType ) {
 			return {
 				...nextState,
-				isPersistentChange: false,
+				isPersistentChange: nextIsPersistentChange,
 			};
 		}
 
@@ -239,6 +224,37 @@ function withPersistentBlockChange( reducer ) {
 	};
 }
 
+/**
+ * Higher-order reducer intended to augment the blocks reducer, assigning an
+ * `isIgnoredChange` property value corresponding to whether a change in state
+ * can be considered as ignored. A change is considered ignored when the result
+ * of an action not incurred by direct user interaction.
+ *
+ * @param {Function} reducer Original reducer function.
+ *
+ * @return {Function} Enhanced reducer function.
+ */
+function withIgnoredBlockChange( reducer ) {
+	/**
+	 * Set of action types for which a blocks state change should be ignored.
+	 *
+	 * @type {Set}
+	 */
+	const IGNORED_ACTION_TYPES = new Set( [
+		'RECEIVE_BLOCKS',
+	] );
+
+	return ( state, action ) => {
+		const nextState = reducer( state, action );
+
+		if ( nextState !== state ) {
+			nextState.isIgnoredChange = IGNORED_ACTION_TYPES.has( action.type );
+		}
+
+		return nextState;
+	};
+}
+
 /**
  * Higher-order reducer targeting the combined blocks reducer, augmenting
  * block client IDs in remove action to include cascade of inner blocks.
@@ -380,6 +396,7 @@ export const blocks = flow(
 	withBlockReset,
 	withSaveReusableBlock,
 	withPersistentBlockChange,
+	withIgnoredBlockChange,
 )( {
 	byClientId( state = {}, action ) {
 		switch ( action.type ) {

packages/block-editor/src/store/selectors.js

@@ -1387,6 +1387,24 @@ export function isLastBlockChangePersistent( state ) {
 	return state.blocks.isPersistentChange;
 }
 
+/**
+ * Returns true if the most recent block change is be considered ignored, or
+ * false otherwise. An ignored change is one not to be committed by
+ * BlockEditorProvider, neither via `onChange` nor `onInput`.
+ *
+ * @param {Object} state Block editor state.
+ *
+ * @return {boolean} Whether the most recent block change was ignored.
+ */
+export function __unstableIsLastBlockChangeIgnored( state ) {
+	// TODO: Removal Plan: Changes incurred by RECEIVE_BLOCKS should not be
+	// ignored if in-fact they result in a change in blocks state. The current
+	// need to ignore changes not a result of user interaction should be
+	// accounted for in the refactoring of reusable blocks as occurring within
+	// their own separate block editor / state (#7119).
+	return state.blocks.isIgnoredChange;
+}
+
 /**
  * Returns the value of a post meta from the editor settings.
  *

packages/block-editor/src/store/test/reducer.js

@@ -233,7 +233,8 @@ describe( 'state', () => {
 				const state = blocks( existingState, action );
 
 				expect( state ).toEqual( {
-					isPersistentChange: expect.anything(),
+					isPersistentChange: true,
+					isIgnoredChange: false,
 					byClientId: {
 						clicken: {
 							clientId: 'chicken',
@@ -295,7 +296,8 @@ describe( 'state', () => {
 				const state = blocks( existingState, action );
 
 				expect( state ).toEqual( {
-					isPersistentChange: expect.anything(),
+					isPersistentChange: true,
+					isIgnoredChange: false,
 					byClientId: {
 						clicken: {
 							clientId: 'chicken',
@@ -386,7 +388,8 @@ describe( 'state', () => {
 				const state = blocks( existingState, action );
 
 				expect( state ).toEqual( {
-					isPersistentChange: expect.anything(),
+					isPersistentChange: true,
+					isIgnoredChange: false,
 					byClientId: {
 						clicken: {
 							clientId: 'chicken',
@@ -478,7 +481,8 @@ describe( 'state', () => {
 				const state = blocks( existingState, action );
 
 				expect( state ).toEqual( {
-					isPersistentChange: expect.anything(),
+					isPersistentChange: true,
+					isIgnoredChange: false,
 					byClientId: {
 						clicken: {
 							clientId: 'chicken',
@@ -512,6 +516,7 @@ describe( 'state', () => {
 				attributes: {},
 				order: {},
 				isPersistentChange: true,
+				isIgnoredChange: false,
 			} );
 		} );
 
@@ -1500,7 +1505,22 @@ describe( 'state', () => {
 					expect( state.isPersistentChange ).toBe( true );
 				} );
 
-				it( 'should not consider received blocks as persistent change', () => {
+				it( 'should retain reference for same state, same persistence', () => {
+					const original = deepFreeze( blocks( undefined, {
+						type: 'RESET_BLOCKS',
+						blocks: [],
+					} ) );
+
+					const state = blocks( original, {
+						type: '__INERT__',
+					} );
+
+					expect( state ).toBe( original );
+				} );
+			} );
+
+			describe( 'isIgnoredChange', () => {
+				it( 'should consider received blocks as ignored change', () => {
 					const state = blocks( undefined, {
 						type: 'RECEIVE_BLOCKS',
 						blocks: [ {
@@ -1510,7 +1530,7 @@ describe( 'state', () => {
 						} ],
 					} );
 
-					expect( state.isPersistentChange ).toBe( false );
+					expect( state.isIgnoredChange ).toBe( true );
 				} );
 			} );
 		} );

packages/e2e-tests/specs/change-detection.test.js

@@ -288,4 +288,21 @@ describe( 'Change detection', () => {
 
 		await assertIsDirty( true );
 	} );
+
+	it( 'should not prompt when receiving reusable blocks', async () => {
+		// Regression Test: Verify that non-modifying behaviors does not incur
+		// dirtiness. Previously, this could occur as a result of either (a)
+		// selecting a block, (b) opening the inserter, or (c) editing a post
+		// which contained a reusable block. The root issue was changes in
+		// block editor state as a result of reusable blocks data having been
+		// received, reflected here in this test.
+		//
+		// TODO: This should be considered a temporary test, existing only so
+		// long as the experimental reusable blocks fetching data flow exists.
+		//
+		// See: https://github.com/WordPress/gutenberg/issues/14766
+		await page.evaluate( () => window.wp.data.dispatch( 'core/editor' ).__experimentalReceiveReusableBlocks( [] ) );
+
+		await assertIsDirty( false );
+	} );
 } );