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

Improve the getBlock and getBlocks performance #34241

Merged
merged 8 commits into from
Aug 26, 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
8 changes: 2 additions & 6 deletions docs/reference-guides/data/data-core-block-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,6 @@ Returns all block objects for the current post being edited as an array in
the order they appear in the post. Note that this will exclude child blocks
of nested inner block controllers.

Note: It's important to memoize this selector to avoid return a new instance
on each call. We use the block cache state for each top-level block of the
given clientID. This way, the selector only refreshes on changes to blocks
associated with the given entity, and does not refresh when changes are made
to blocks which are part of different inner block controllers.

_Parameters_

- _state_ `Object`: Editor state.
Expand Down Expand Up @@ -1225,6 +1219,8 @@ Returns an action object used in signalling that blocks have been received.
Unlike resetBlocks, these should be appended to the existing known set, not
replacing.

Todo: This should be deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Was catching up with this PR and wanted to bring attention to this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I'll need to deprecate this action properly, it's not used anywhere.


_Parameters_

- _blocks_ `Object[]`: Array of block objects.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export default compose( [
isBlockInsertionPointVisible,
getSettings,
getBlockParents,
__unstableGetBlockWithoutInnerBlocks,
getBlock,
} = select( blockEditorStore );

const blockClientIds = getBlockOrder( rootClientId );
Expand All @@ -225,14 +225,11 @@ export default compose( [

const isReadOnly = getSettings().readOnly;

const block = __unstableGetBlockWithoutInnerBlocks( clientId );
const { attributes, name } = block || {};
const { attributes, name } = getBlock( clientId );
Copy link
Member

Choose a reason for hiding this comment

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

Can getBlock( clientId ) be null? If so, wouldn't it throw a type error because it can't unwrap the properties?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it can in some rare occasions, I'll restore the fallback.

const { align } = attributes || {};
const parents = getBlockParents( clientId, true );
const hasParents = !! parents.length;
const parentBlock = hasParents
? __unstableGetBlockWithoutInnerBlocks( parents[ 0 ] )
: {};
const parentBlock = hasParents ? getBlock( parents[ 0 ] ) : {};
const { align: parentBlockAlignment } =
parentBlock?.attributes || {};
const { name: parentBlockName } = parentBlock || {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ export default compose( [
getBlockIndex,
getSettings,
isBlockSelected,
__unstableGetBlockWithoutInnerBlocks,
getBlock,
getSelectedBlockClientId,
getLowestCommonAncestorWithSelectedBlock,
getBlockParents,
Expand All @@ -315,7 +315,7 @@ export default compose( [
const order = getBlockIndex( clientId, rootClientId );
const isSelected = isBlockSelected( clientId );
const isInnerBlockSelected = hasSelectedInnerBlock( clientId );
const block = __unstableGetBlockWithoutInnerBlocks( clientId );
const block = getBlock( clientId );
const { name, attributes, isValid } = block || {};

const blockType = getBlockType( name || 'core/missing' );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ function wrapperSelector( select ) {
getSelectedBlockClientId,
getFirstMultiSelectedBlockClientId,
getBlockRootClientId,
__unstableGetBlockWithoutInnerBlocks,
getBlock,
getBlockParents,
__experimentalGetBlockListSettingsForBlocks,
} = select( blockEditorStore );
Expand All @@ -293,8 +293,7 @@ function wrapperSelector( select ) {
return;
}

const { name, attributes = {}, isValid } =
__unstableGetBlockWithoutInnerBlocks( clientId ) || {};
const { name, attributes = {}, isValid } = getBlock( clientId ) || {};
const blockParentsClientIds = getBlockParents( clientId );

// Get Block List Settings for all ancestors of the current Block clientId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,13 @@ function BlockSelectionButton( { clientId, rootClientId, blockElement } ) {
const selected = useSelect(
( select ) => {
const {
__unstableGetBlockWithoutInnerBlocks,
getBlock,
getBlockIndex,
hasBlockMovingClientId,
getBlockListSettings,
} = select( blockEditorStore );
const index = getBlockIndex( clientId, rootClientId );
const { name, attributes } = __unstableGetBlockWithoutInnerBlocks(
clientId
);
const { name, attributes } = getBlock( clientId );
const blockMovingMode = hasBlockMovingClientId();
return {
index,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ function RichTextWrapper(
getSelectionEnd,
getSettings,
didAutomaticChange,
__unstableGetBlockWithoutInnerBlocks,
getBlock,
isMultiSelecting,
hasMultiSelection,
} = select( blockEditorStore );
Expand All @@ -149,8 +149,7 @@ function RichTextWrapper(
// If the block of this RichText is unmodified then it's a candidate for replacing when adding a new block.
// In order to fix https://github.com/wordpress-mobile/gutenberg-mobile/issues/1126, let's blur on unmount in that case.
// This apparently assumes functionality the BlockHlder actually
const block =
clientId && __unstableGetBlockWithoutInnerBlocks( clientId );
const block = clientId && getBlock( clientId );
const shouldBlurOnUnmount =
block && isSelected && isUnmodifiedDefaultBlock( block );
extraProps = {
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ export function resetSelection(
* Unlike resetBlocks, these should be appended to the existing known set, not
* replacing.
*
* Todo: This should be deprecated
*
* @param {Object[]} blocks Array of block objects.
*
* @return {Object} Action object.
Expand Down
Loading