Block: Avoid rerendering when the previous/next block updates (#5025)

editor/components/block-list/block.js

@@ -57,8 +57,8 @@ import {
 	isMultiSelecting,
 	getBlockIndex,
 	getEditedPostAttribute,
-	getNextBlock,
-	getPreviousBlock,
+	getNextBlockUid,
+	getPreviousBlockUid,
 	isBlockHovered,
 	isBlockMultiSelected,
 	isBlockSelected,
@@ -290,20 +290,20 @@ export class BlockListBlock extends Component {
 	}
 
 	mergeBlocks( forward = false ) {
-		const { block, previousBlock, nextBlock, onMerge } = this.props;
+		const { block, previousBlockUid, nextBlockUid, onMerge } = this.props;
 
 		// Do nothing when it's the first block.
 		if (
-			( ! forward && ! previousBlock ) ||
-			( forward && ! nextBlock )
+			( ! forward && ! previousBlockUid ) ||
+			( forward && ! nextBlockUid )
 		) {
 			return;
 		}
 
 		if ( forward ) {
-			onMerge( block, nextBlock );
+			onMerge( block.uid, nextBlockUid );
 		} else {
-			onMerge( previousBlock, block );
+			onMerge( previousBlockUid, block.uid );
 		}
 
 		// Manually trigger typing mode, since merging will remove this block and
@@ -612,8 +612,8 @@ export class BlockListBlock extends Component {
 const mapStateToProps = ( state, { uid, rootUID } ) => {
 	const isSelected = isBlockSelected( state, uid );
 	return {
-		previousBlock: getPreviousBlock( state, uid ),
-		nextBlock: getNextBlock( state, uid ),
+		previousBlockUid: getPreviousBlockUid( state, uid ),
+		nextBlockUid: getNextBlockUid( state, uid ),
 		block: getBlock( state, uid ),
 		isMultiSelected: isBlockMultiSelected( state, uid ),
 		isFirstMultiSelected: isFirstMultiSelectedBlock( state, uid ),

editor/components/writing-flow/index.js

@@ -24,8 +24,8 @@ import {
 	placeCaretAtVerticalEdge,
 } from '../../utils/dom';
 import {
-	getPreviousBlock,
-	getNextBlock,
+	getPreviousBlockUid,
+	getNextBlockUid,
 	getMultiSelectedBlocksStartUid,
 	getMultiSelectedBlocks,
 	getSelectedBlock,
@@ -155,20 +155,20 @@ class WritingFlow extends Component {
 	}
 
 	expandSelection( currentStartUid, isReverse ) {
-		const { previousBlock, nextBlock } = this.props;
+		const { previousBlockUid, nextBlockUid } = this.props;
 
-		const expandedBlock = isReverse ? previousBlock : nextBlock;
-		if ( expandedBlock ) {
-			this.props.onMultiSelect( currentStartUid, expandedBlock.uid );
+		const expandedBlockUid = isReverse ? previousBlockUid : nextBlockUid;
+		if ( expandedBlockUid ) {
+			this.props.onMultiSelect( currentStartUid, expandedBlockUid );
 		}
 	}
 
 	moveSelection( isReverse ) {
-		const { previousBlock, nextBlock } = this.props;
+		const { previousBlockUid, nextBlockUid } = this.props;
 
-		const focusedBlock = isReverse ? previousBlock : nextBlock;
-		if ( focusedBlock ) {
-			this.props.onSelectBlock( focusedBlock.uid );
+		const focusedBlockUid = isReverse ? previousBlockUid : nextBlockUid;
+		if ( focusedBlockUid ) {
+			this.props.onSelectBlock( focusedBlockUid );
 		}
 	}
 
@@ -299,8 +299,8 @@ class WritingFlow extends Component {
 
 export default connect(
 	( state ) => ( {
-		previousBlock: getPreviousBlock( state ),
-		nextBlock: getNextBlock( state ),
+		previousBlockUid: getPreviousBlockUid( state ),
+		nextBlockUid: getNextBlockUid( state ),
 		selectionStart: getMultiSelectedBlocksStartUid( state ),
 		hasMultiSelection: getMultiSelectedBlocks( state ).length > 1,
 		selectedBlock: getSelectedBlock( state ),

editor/store/actions.js

@@ -257,10 +257,18 @@ export function trashPost( postId, postType ) {
 	};
 }
 
-export function mergeBlocks( blockA, blockB ) {
+/**
+ * Returns an action object used in signalling that two blocks should be merged
+ *
+ * @param {string} blockAUid UID of the first block to merge.
+ * @param {string} blockBUid UID of the second block to merge.
+ *
+ * @return {Object} Action object.
+ */
+export function mergeBlocks( blockAUid, blockBUid ) {
 	return {
 		type: 'MERGE_BLOCKS',
-		blocks: [ blockA, blockB ],
+		blocks: [ blockAUid, blockBUid ],
 	};
 }
 

editor/store/effects.js

@@ -221,7 +221,10 @@ export default {
 	},
 	MERGE_BLOCKS( action, store ) {
 		const { dispatch } = store;
-		const [ blockA, blockB ] = action.blocks;
+		const state = store.getState();
+		const [ blockAUid, blockBUid ] = action.blocks;
+		const blockA = getBlock( state, blockAUid );
+		const blockB = getBlock( state, blockBUid );
 		const blockType = getBlockType( blockA.name );
 
 		// Only focus the previous block if it's not mergeable

editor/store/selectors.js

@@ -531,17 +531,17 @@ export function getBlockRootUID( state, uid ) {
 }
 
 /**
- * Returns the block adjacent one at the given reference startUID and modifier
+ * Returns the UID of the block adjacent one at the given reference startUID and modifier
  * directionality. Defaults start UID to the selected block, and direction as
  * next block. Returns null if there is no adjacent block.
  *
  * @param {Object}  state    Global application state.
  * @param {?string} startUID Optional UID of block from which to search.
  * @param {?number} modifier Directionality multiplier (1 next, -1 previous).
  *
- * @return {?Object} Adjacent block object, or null if none exists.
+ * @return {?string} Return the UID of the block, or null if none exists.
  */
-export function getAdjacentBlock( state, startUID, modifier = 1 ) {
+export function getAdjacentBlockUid( state, startUID, modifier = 1 ) {
 	// Default to selected block.
 	if ( startUID === undefined ) {
 		startUID = get( getSelectedBlock( state ), 'uid' );
@@ -584,33 +584,33 @@ export function getAdjacentBlock( state, startUID, modifier = 1 ) {
 	}
 
 	// Assume incremented index is within the set.
-	return getBlock( state, orderSet[ nextIndex ] );
+	return orderSet[ nextIndex ];
 }
 
 /**
- * Returns the previous block from the given reference startUID. Defaults start
+ * Returns the previous block's UID from the given reference startUID. Defaults start
  * UID to the selected block. Returns null if there is no previous block.
  *
  * @param {Object}  state    Global application state.
  * @param {?string} startUID Optional UID of block from which to search.
  *
- * @return {?Object} Adjacent block object, or null if none exists.
+ * @return {?string} Adjacent block's UID, or null if none exists.
  */
-export function getPreviousBlock( state, startUID ) {
-	return getAdjacentBlock( state, startUID, -1 );
+export function getPreviousBlockUid( state, startUID ) {
+	return getAdjacentBlockUid( state, startUID, -1 );
 }
 
 /**
- * Returns the next block from the given reference startUID. Defaults start UID
+ * Returns the next block's UID from the given reference startUID. Defaults start UID
  * to the selected block. Returns null if there is no next block.
  *
  * @param {Object}  state    Global application state.
  * @param {?string} startUID Optional UID of block from which to search.
  *
- * @return {?Object} Adjacent block object, or null if none exists.
+ * @return {?string} Adjacent block's UID, or null if none exists.
  */
-export function getNextBlock( state, startUID ) {
-	return getAdjacentBlock( state, startUID, 1 );
+export function getNextBlockUid( state, startUID ) {
+	return getAdjacentBlockUid( state, startUID, 1 );
 }
 
 /**

editor/store/test/actions.js

@@ -272,15 +272,11 @@ describe( 'actions', () => {
 
 	describe( 'mergeBlocks', () => {
 		it( 'should return MERGE_BLOCKS action', () => {
-			const blockA = {
-				uid: 'blockA',
-			};
-			const blockB = {
-				uid: 'blockB',
-			};
-			expect( mergeBlocks( blockA, blockB ) ).toEqual( {
+			const blockAUid = 'blockA';
+			const blockBUid = 'blockB';
+			expect( mergeBlocks( blockAUid, blockBUid ) ).toEqual( {
 				type: 'MERGE_BLOCKS',
-				blocks: [ blockA, blockB ],
+				blocks: [ blockAUid, blockBUid ],
 			} );
 		} );
 	} );

editor/store/test/effects.js

@@ -48,11 +48,13 @@ describe( 'effects', () => {
 
 	describe( '.MERGE_BLOCKS', () => {
 		const handler = effects.MERGE_BLOCKS;
+		const defaultGetBlock = selectors.getBlock;
 
 		afterEach( () => {
 			getBlockTypes().forEach( ( block ) => {
 				unregisterBlockType( block.name );
 			} );
+			selectors.getBlock = defaultGetBlock;
 		} );
 
 		it( 'should only focus the blockA if the blockA has no merge function', () => {
@@ -65,8 +67,13 @@ describe( 'effects', () => {
 				uid: 'ribs',
 				name: 'core/test-block',
 			};
+			selectors.getBlock = ( state, uid ) => {
+				return blockA.uid === uid ? blockA : blockB;
+			};
+
 			const dispatch = jest.fn();
-			handler( mergeBlocks( blockA, blockB ), { dispatch } );
+			const getState = () => ( {} );
+			handler( mergeBlocks( blockA.uid, blockB.uid ), { dispatch, getState } );
 
 			expect( dispatch ).toHaveBeenCalledTimes( 1 );
 			expect( dispatch ).toHaveBeenCalledWith( selectBlock( 'chicken' ) );
@@ -93,8 +100,12 @@ describe( 'effects', () => {
 				name: 'core/test-block',
 				attributes: { content: 'ribs' },
 			};
+			selectors.getBlock = ( state, uid ) => {
+				return blockA.uid === uid ? blockA : blockB;
+			};
 			const dispatch = jest.fn();
-			handler( mergeBlocks( blockA, blockB ), { dispatch } );
+			const getState = () => ( {} );
+			handler( mergeBlocks( blockA.uid, blockB.uid ), { dispatch, getState } );
 
 			expect( dispatch ).toHaveBeenCalledTimes( 2 );
 			expect( dispatch ).toHaveBeenCalledWith( selectBlock( 'chicken', -1 ) );
@@ -127,8 +138,12 @@ describe( 'effects', () => {
 				name: 'core/test-block2',
 				attributes: { content: 'ribs' },
 			};
+			selectors.getBlock = ( state, uid ) => {
+				return blockA.uid === uid ? blockA : blockB;
+			};
 			const dispatch = jest.fn();
-			handler( mergeBlocks( blockA, blockB ), { dispatch } );
+			const getState = () => ( {} );
+			handler( mergeBlocks( blockA.uid, blockB.uid ), { dispatch, getState } );
 
 			expect( dispatch ).not.toHaveBeenCalled();
 		} );
@@ -180,8 +195,12 @@ describe( 'effects', () => {
 				name: 'core/test-block-2',
 				attributes: { content2: 'ribs' },
 			};
+			selectors.getBlock = ( state, uid ) => {
+				return blockA.uid === uid ? blockA : blockB;
+			};
 			const dispatch = jest.fn();
-			handler( mergeBlocks( blockA, blockB ), { dispatch } );
+			const getState = () => ( {} );
+			handler( mergeBlocks( blockA.uid, blockB.uid ), { dispatch, getState } );
 
 			expect( dispatch ).toHaveBeenCalledTimes( 2 );
 			// expect( dispatch ).toHaveBeenCalledWith( focusBlock( 'chicken', { offset: -1 } ) );