Skip to content

Commit

Permalink
Batch container block settings action calls (#43958)
Browse files Browse the repository at this point in the history
Co-authored-by: Nik Tsekouras <[email protected]>
Co-authored-by: Gerardo <[email protected]>
Co-authored-by: Matias Ventura <[email protected]>
  • Loading branch information
4 people authored and ockham committed Sep 19, 2022
1 parent 81b5252 commit c21b626
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 135 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function BlockListCompact( props ) {
};

return (
<View style={ containerStyle }>
<View style={ containerStyle } testID="block-list-wrapper">
{ blockClientIds.map( ( currentClientId ) => (
<BlockListBlock
clientId={ currentClientId }
Expand Down
7 changes: 7 additions & 0 deletions packages/block-editor/src/components/inner-blocks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ function UncontrolledInnerBlocks( props ) {
const context = useSelect(
( select ) => {
const block = select( blockEditorStore ).getBlock( clientId );

// This check is here to avoid the Redux zombie bug where a child subscription
// is called before a parent, causing potential JS errors when the child has been removed.
if ( ! block ) {
return;
}

const blockType = getBlockType( block.name );

if ( ! blockType || ! blockType.providesContext ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,42 +47,53 @@ export default function useInnerBlockTemplateSync(
( select ) => select( blockEditorStore ).getBlocks( clientId ),
[ clientId ]
);
const { getBlocks } = useSelect( blockEditorStore );

// Maintain a reference to the previous value so we can do a deep equality check.
const existingTemplate = useRef( null );
useLayoutEffect( () => {
// Only synchronize innerBlocks with template if innerBlocks are empty
// or a locking "all" or "contentOnly" exists directly on the block.
if (
innerBlocks.length === 0 ||
templateLock === 'all' ||
templateLock === 'contentOnly'
) {
// There's an implicit dependency between useInnerBlockTemplateSync and useNestedSettingsUpdate
// The former needs to happen after the latter and since the latter is using microtasks to batch updates (performance optimization),
// we need to schedule this one in a microtask as well.
// Exemple: If you remove queueMicrotask here, ctrl + click to insert quote block won't close the inserter.
window.queueMicrotask( () => {
// Only synchronize innerBlocks with template if innerBlocks are empty
// or a locking "all" or "contentOnly" exists directly on the block.
const currentInnerBlocks = getBlocks( clientId );
const shouldApplyTemplate =
currentInnerBlocks.length === 0 ||
templateLock === 'all' ||
templateLock === 'contentOnly';

const hasTemplateChanged = ! isEqual(
template,
existingTemplate.current
);
if ( hasTemplateChanged ) {
existingTemplate.current = template;
const nextBlocks = synchronizeBlocksWithTemplate(
innerBlocks,
template

if ( ! shouldApplyTemplate || ! hasTemplateChanged ) {
return;
}

existingTemplate.current = template;
const nextBlocks = synchronizeBlocksWithTemplate(
currentInnerBlocks,
template
);

if ( ! isEqual( nextBlocks, currentInnerBlocks ) ) {
replaceInnerBlocks(
clientId,
nextBlocks,
currentInnerBlocks.length === 0 &&
templateInsertUpdatesSelection &&
nextBlocks.length !== 0,
// This ensures the "initialPosition" doesn't change when applying the template
// If we're supposed to focus the block, we'll focus the first inner block
// otherwise, we won't apply any auto-focus.
// This ensures for instance that the focus stays in the inserter when inserting the "buttons" block.
getSelectedBlocksInitialCaretPosition()
);
if ( ! isEqual( nextBlocks, innerBlocks ) ) {
replaceInnerBlocks(
clientId,
nextBlocks,
innerBlocks.length === 0 &&
templateInsertUpdatesSelection &&
nextBlocks.length !== 0,
// This ensures the "initialPosition" doesn't change when applying the template
// If we're supposed to focus the block, we'll focus the first inner block
// otherwise, we won't apply any auto-focus.
// This ensures for instance that the focus stays in the inserter when inserting the "buttons" block.
getSelectedBlocksInitialCaretPosition()
);
}
}
}
} );
}, [ innerBlocks, template, templateLock, clientId ] );
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { useLayoutEffect, useMemo } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';
import { useSelect, useDispatch, useRegistry } from '@wordpress/data';
import isShallowEqual from '@wordpress/is-shallow-equal';

/**
Expand All @@ -13,6 +13,8 @@ import { getLayoutType } from '../../layouts';

/** @typedef {import('../../selectors').WPDirectInsertBlock } WPDirectInsertBlock */

const pendingSettingsUpdates = new WeakMap();

/**
* This hook is a side effect which updates the block-editor store when changes
* happen to inner block settings. The given props are transformed into a
Expand Down Expand Up @@ -46,6 +48,7 @@ export default function useNestedSettingsUpdate(
layout
) {
const { updateBlockListSettings } = useDispatch( blockEditorStore );
const registry = useRegistry();

const { blockListSettings, parentLock } = useSelect(
( select ) => {
Expand Down Expand Up @@ -98,7 +101,30 @@ export default function useNestedSettingsUpdate(
}

if ( ! isShallowEqual( blockListSettings, newSettings ) ) {
updateBlockListSettings( clientId, newSettings );
// Batch updates to block list settings to avoid triggering cascading renders
// for each container block included in a tree and optimize initial render.
// To avoid triggering updateBlockListSettings for each container block
// causing X re-renderings for X container blocks,
// we batch all the updatedBlockListSettings in a single "data" batch
// which results in a single re-render.
if ( ! pendingSettingsUpdates.get( registry ) ) {
pendingSettingsUpdates.set( registry, [] );
}
pendingSettingsUpdates
.get( registry )
.push( [ clientId, newSettings ] );
window.queueMicrotask( () => {
if ( pendingSettingsUpdates.get( registry )?.length ) {
registry.batch( () => {
pendingSettingsUpdates
.get( registry )
.forEach( ( args ) => {
updateBlockListSettings( ...args );
} );
pendingSettingsUpdates.set( registry, [] );
} );
}
} );
}
}, [
clientId,
Expand All @@ -112,5 +138,6 @@ export default function useNestedSettingsUpdate(
orientation,
updateBlockListSettings,
layout,
registry,
] );
}
3 changes: 3 additions & 0 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1714,9 +1714,12 @@ export function automaticChangeStatus( state, action ) {
case 'SET_BLOCK_VISIBILITY':
case 'START_TYPING':
case 'STOP_TYPING':
case 'UPDATE_BLOCK_LIST_SETTINGS':
return state;
}

// TODO: This is a source of bug, as each time there's a change in timing,
// or a new action is added, this could break.
// Reset the state by default (for any action not handled).
}

Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1435,7 +1435,7 @@ export function getTemplateLock( state, rootClientId ) {

const blockListSettings = getBlockListSettings( state, rootClientId );
if ( ! blockListSettings ) {
return null;
return undefined;
}

return blockListSettings.templateLock;
Expand Down
8 changes: 4 additions & 4 deletions packages/block-editor/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -3253,7 +3253,7 @@ describe( 'selectors', () => {
expect( getTemplateLock( state ) ).toBe( 'all' );
} );

it( 'should return null if the specified clientId was not found', () => {
it( 'should return undefined if the specified clientId was not found', () => {
const state = {
settings: { templateLock: 'all' },
blockListSettings: {
Expand All @@ -3263,10 +3263,10 @@ describe( 'selectors', () => {
},
};

expect( getTemplateLock( state, 'ribs' ) ).toBe( null );
expect( getTemplateLock( state, 'ribs' ) ).toBe( undefined );
} );

it( 'should return null if template lock was not set on the specified block', () => {
it( 'should return undefined if template lock was not set on the specified block', () => {
const state = {
settings: { templateLock: 'all' },
blockListSettings: {
Expand All @@ -3276,7 +3276,7 @@ describe( 'selectors', () => {
},
};

expect( getTemplateLock( state, 'ribs' ) ).toBe( null );
expect( getTemplateLock( state, 'ribs' ) ).toBe( undefined );
} );

it( 'should return the template lock for the specified clientId', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`List V2 block adds one item to the list 1`] = `
exports[`List block adds one item to the list 1`] = `
"<!-- wp:list -->
<ul><!-- wp:list-item -->
<li>First list item</li>
<!-- /wp:list-item --></ul>
<!-- /wp:list -->"
`;

exports[`List V2 block changes the indentation level 1`] = `
exports[`List block changes the indentation level 1`] = `
"<!-- wp:list -->
<ul><!-- wp:list-item -->
<li>Item 1<!-- wp:list -->
Expand All @@ -20,7 +20,7 @@ exports[`List V2 block changes the indentation level 1`] = `
<!-- /wp:list -->"
`;

exports[`List V2 block changes to ordered list 1`] = `
exports[`List block changes to ordered list 1`] = `
"<!-- wp:list {\\"ordered\\":true} -->
<ol><!-- wp:list-item -->
<li>Item 1</li>
Expand All @@ -36,7 +36,7 @@ exports[`List V2 block changes to ordered list 1`] = `
<!-- /wp:list -->"
`;

exports[`List V2 block changes to reverse ordered list 1`] = `
exports[`List block changes to reverse ordered list 1`] = `
"<!-- wp:list {\\"ordered\\":true,\\"reversed\\":true} -->
<ol reversed><!-- wp:list-item -->
<li>Item 1</li>
Expand All @@ -52,15 +52,15 @@ exports[`List V2 block changes to reverse ordered list 1`] = `
<!-- /wp:list -->"
`;

exports[`List V2 block inserts block 1`] = `
exports[`List block inserts block 1`] = `
"<!-- wp:list -->
<ul><!-- wp:list-item -->
<li></li>
<!-- /wp:list-item --></ul>
<!-- /wp:list -->"
`;

exports[`List V2 block removes the indentation level 1`] = `
exports[`List block removes the indentation level 1`] = `
"<!-- wp:list -->
<ul><!-- wp:list-item -->
<li>Item 1</li>
Expand All @@ -72,7 +72,7 @@ exports[`List V2 block removes the indentation level 1`] = `
<!-- /wp:list -->"
`;

exports[`List V2 block sets a start value to an ordered list 1`] = `
exports[`List block sets a start value to an ordered list 1`] = `
"<!-- wp:list {\\"ordered\\":true,\\"start\\":25} -->
<ol start=\\"25\\"><!-- wp:list-item -->
<li>Item 1</li>
Expand All @@ -88,7 +88,7 @@ exports[`List V2 block sets a start value to an ordered list 1`] = `
<!-- /wp:list -->"
`;
exports[`List V2 block shows different indentation levels 1`] = `
exports[`List block shows different indentation levels 1`] = `
"<!-- wp:list -->
<ul><!-- wp:list-item -->
<li>List item 1</li>
Expand Down Expand Up @@ -119,27 +119,3 @@ exports[`List V2 block shows different indentation levels 1`] = `
<!-- /wp:list-item --></ul>
<!-- /wp:list -->"
`;
exports[`List block inserts block 1`] = `
"<!-- wp:list -->
<ul><!-- wp:list-item -->
<li></li>
<!-- /wp:list-item --></ul>
<!-- /wp:list -->"
`;
exports[`List block renders a list with a few items 1`] = `
"<!-- wp:list -->
<ul><!-- wp:list-item -->
<li>Item 1</li>
<!-- /wp:list-item -->
<!-- wp:list-item -->
<li>Item 2</li>
<!-- /wp:list-item -->
<!-- wp:list-item -->
<li>Item 3</li>
<!-- /wp:list-item --></ul>
<!-- /wp:list -->"
`;
Loading

0 comments on commit c21b626

Please sign in to comment.