From 62340740df209c806a8da5d158377c56ae254ea3 Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Wed, 2 Nov 2022 20:47:08 -0700 Subject: [PATCH] Block API: Refactor parser module in preparation for future work. This patch contains a mixture of small refators, mostly a combination of updating comments and explanations as well as rearranging some array iterating code to use more traditional loop control structures. While the comments should be self-explanatory, the loop control structures are refactored for a dual purpose. In framework code like this that is core to the project and often untouched, there is value in optimizing to eliminate memory allocations and reduce pressure on the garbage collector. These costs are often difficult to profile in existing tools. More imporantly, as we think about introducing an asynchronous block loading system we need to prepare to handle the semantic changes that asynchronous flow introduces. These changes are meant to smooth the transition to asynchronous code while clarifying how blocks are loaded into the editor. --- packages/blocks/README.md | 2 +- .../parser/apply-block-deprecated-versions.js | 2 +- .../src/api/parser/convert-legacy-block.js | 53 +++-- packages/blocks/src/api/parser/index.js | 209 +++++++++++++----- .../src/api/parser/serialize-raw-block.js | 30 ++- packages/blocks/src/store/reducer.js | 105 +++++++-- 6 files changed, 290 insertions(+), 111 deletions(-) diff --git a/packages/blocks/README.md b/packages/blocks/README.md index 20930df1a3a9f5..8f70be8274f088 100644 --- a/packages/blocks/README.md +++ b/packages/blocks/README.md @@ -614,7 +614,7 @@ _Parameters_ _Returns_ -- `Array`: Block list. +- `WPBlock[]`: Block list. ### parseWithAttributeSchema diff --git a/packages/blocks/src/api/parser/apply-block-deprecated-versions.js b/packages/blocks/src/api/parser/apply-block-deprecated-versions.js index 2a3e880a3f7460..ee11b68d3cc7c5 100644 --- a/packages/blocks/src/api/parser/apply-block-deprecated-versions.js +++ b/packages/blocks/src/api/parser/apply-block-deprecated-versions.js @@ -22,7 +22,7 @@ function stubFalse() { * and no eligible migrations exist. * * @param {import(".").WPBlock} block Parsed and invalid block object. - * @param {import(".").WPRawBlock} rawBlock Raw block object. + * @param {import(".").BlockNode} rawBlock Raw block object. * @param {import('../registration').WPBlockType} blockType Block type. This is normalize not necessary and * can be inferred from the block name, * but it's here for performance reasons. diff --git a/packages/blocks/src/api/parser/convert-legacy-block.js b/packages/blocks/src/api/parser/convert-legacy-block.js index c993d450099912..76e7d6e039c78c 100644 --- a/packages/blocks/src/api/parser/convert-legacy-block.js +++ b/packages/blocks/src/api/parser/convert-legacy-block.js @@ -3,29 +3,28 @@ * both in the parser level for previous content and to convert such blocks * used in Custom Post Types templates. * - * @param {string} name The block's name - * @param {Object} attributes The block's attributes + * @param {string|null} name The block's name + * @param {Object|null} attributes The block's attributes * - * @return {[string, Object]} The block's name and attributes, changed accordingly if a match was found + * @return {[string, Object|null]} The block's name and attributes, changed accordingly if a match was found */ export function convertLegacyBlockNameAndAttributes( name, attributes ) { - const newAttributes = { ...attributes }; // Convert 'core/cover-image' block in existing content to 'core/cover'. if ( 'core/cover-image' === name ) { - name = 'core/cover'; + return [ 'core/cover', attributes ]; } // Convert 'core/text' blocks in existing content to 'core/paragraph'. if ( 'core/text' === name || 'core/cover-text' === name ) { - name = 'core/paragraph'; + return [ 'core/paragraph', attributes ]; } // Convert derivative blocks such as 'core/social-link-wordpress' to the // canonical form 'core/social-link'. if ( name && name.indexOf( 'core/social-link-' ) === 0 ) { // Capture `social-link-wordpress` into `{"service":"wordpress"}` - newAttributes.service = name.substring( 17 ); - name = 'core/social-link'; + const service = name.substring( 17 ); + return [ 'core/social-link', { ...attributes, service } ]; } // Convert derivative blocks such as 'core-embed/instagram' to the @@ -37,45 +36,53 @@ export function convertLegacyBlockNameAndAttributes( name, attributes ) { speaker: 'speaker-deck', polldaddy: 'crowdsignal', }; + + const newAttributes = {}; + newAttributes.providerNameSlug = providerSlug in deprecated ? deprecated[ providerSlug ] : providerSlug; + // This is needed as the `responsive` attribute was passed // in a different way before the refactoring to block variations. if ( ! [ 'amazon-kindle', 'wordpress' ].includes( providerSlug ) ) { newAttributes.responsive = true; } - name = 'core/embed'; + + return [ 'core/embed', { ...attributes, ...newAttributes } ]; } // Convert Post Comment blocks in existing content to Comment blocks. // TODO: Remove these checks when WordPress 6.0 is released. if ( name === 'core/post-comment-author' ) { - name = 'core/comment-author-name'; + return [ 'core/comment-author-name', attributes ]; } if ( name === 'core/post-comment-content' ) { - name = 'core/comment-content'; + return [ 'core/comment-content', attributes ]; } if ( name === 'core/post-comment-date' ) { - name = 'core/comment-date'; + return [ 'core/comment-date', attributes ]; } if ( name === 'core/comments-query-loop' ) { - name = 'core/comments'; - const { className = '' } = newAttributes; - if ( ! className.includes( 'wp-block-comments-query-loop' ) ) { - newAttributes.className = [ - 'wp-block-comments-query-loop', - className, - ].join( ' ' ); - } + const { className = '' } = attributes; + const needsClassName = ! className.includes( + 'wp-block-comments-query-loop' + ); + const newAttributes = needsClassName + ? { + ...attributes, + className: `wp-block-comments-query-loop ${ className }`, + } + : attributes; + // Note that we also had to add a deprecation to the block in order // for the ID change to work. + return [ 'core/comments', newAttributes ]; } if ( name === 'core/post-comments' ) { - name = 'core/comments'; - newAttributes.legacy = true; + return [ 'core/comments', { ...attributes, legacy: true } ]; } - return [ name, newAttributes ]; + return [ name, attributes ]; } diff --git a/packages/blocks/src/api/parser/index.js b/packages/blocks/src/api/parser/index.js index 85721d5ecac92b..c61212910de8d8 100644 --- a/packages/blocks/src/api/parser/index.js +++ b/packages/blocks/src/api/parser/index.js @@ -22,32 +22,75 @@ import { applyBlockDeprecatedVersions } from './apply-block-deprecated-versions' import { applyBuiltInValidationFixes } from './apply-built-in-validation-fixes'; /** - * The raw structure of a block includes its attributes, inner - * blocks, and inner HTML. It is important to distinguish inner blocks from - * the HTML content of the block as only the latter is relevant for block - * validation and edit operations. + * The "block node" represents a transition from serialized block data + * into a fully-loaded block with its implementation. It contains the + * information parsed from the input document; be that serialized HTML + * as is the default case in WordPress, or directly loaded from a + * structured data store. * - * @typedef WPRawBlock + * Block nodes do not indicate all of a block's attributes, as some of + * its attributes may be sourced later on from the `innerHTML` by the + * block implementation. This is one example of where the block node + * is not a complete "block" and requires further processing. * - * @property {string=} blockName Block name - * @property {Object=} attrs Block raw or comment attributes. - * @property {string} innerHTML HTML content of the block. - * @property {(string|null)[]} innerContent Content without inner blocks. - * @property {WPRawBlock[]} innerBlocks Inner Blocks. + * Block validation only examines `innerHTML` and delegates the validation + * of any inner blocks to the block loading process for those blocks. + * This prevents an issue with a potentially deeply-nested inner block + * from cascading up and invalidating all of its parent blocks. + * + * @typedef {Object} BlockNode + * + * @property {string|null} blockName Name indicating namespaced block type, e.g. "my-plugin/my-block". + * A `null` block name is given to a section of freeform HTML content. + * @property {Object|null} attrs Attributes sourced from parsed JSON in the block comment delimiters. + * When unable to parse block comment attributes, `attrs` will be `null`. + * @property {BlockNode[]} innerBlocks Nested block nodes; may be empty. + * @property {(string|null)[]} innerContent Indicates arrangement of text chunks and inner blocks. + * @property {string} innerHTML Full text inside block boundaries excluding inner block content. */ /** - * Fully parsed block object. + * Fully parsed and runnable Gutenberg block. + * + * A runnable block combines a parsed block node with a matching + * block implementation. The implementation provides an `edit` + * (and possibly a `save`) function used to interact with the + * block node inside the editor and to serialize its contents + * on save. + * + * These blocks may be substantially different from parsed block + * nodes which created them as the loading process may run the + * block through a pipeline of automatic fixes, normalization, + * and upgrade through the deprecation process. + * + * It's possible that the editor was unable to recognize a block + * during the loading process. In such a case the block object + * may be an entirely different block than one expects from the + * block node which created it. + * + * Unrecognizable blocks exist for a few different reasons: + * - Raw HTML was found in the input document and wrapped in + * a "freeform" block to represent that non-block content. + * - A block node specifies a block type that isn't registered + * in the editor and is wrapped in a "missing" block. + * - Blocks which fail validation will be tagged with a `false` + * value for `isValid`. * - * @typedef WPBlock + * Except for freeform HTML content, the editor is unable to + * interact with unrecognizable blocks and will be inert in + * the editor unless converted into a recognizable block. * - * @property {string} name Block name - * @property {Object} attributes Block raw or comment attributes. - * @property {WPBlock[]} innerBlocks Inner Blocks. - * @property {string} originalContent Original content of the block before validation fixes. - * @property {boolean} isValid Whether the block is valid. - * @property {Object[]} validationIssues Validation issues. - * @property {WPRawBlock} [__unstableBlockSource] Un-processed original copy of block if created through parser. + * @typedef {Object} WPBlock + * + * @property {string} name Name indicating namespaced block type, e.g. "my-plugin/my-block". + * @property {Object} attributes All block attributes, combining those present in the associated + * block node which created this block, and those which the block + * implementation sourced from the block's `innerHTML`. + * @property {WPBlock[]} innerBlocks Nested inner blocks; may be empty. + * @property {string} originalContent Original content of the block before validation fixes. + * @property {boolean} isValid Whether the editor recognizes the block and can interact with it. + * @property {Object[]} validationIssues Validation issues. + * @property {BlockNode} [__unstableBlockSource] Original unprocessed block node which created this block, if available. */ /** @@ -61,31 +104,32 @@ import { applyBuiltInValidationFixes } from './apply-built-in-validation-fixes'; * both in the parser level for previous content and to convert such blocks * used in Custom Post Types templates. * - * @param {WPRawBlock} rawBlock + * @param {BlockNode} block * - * @return {WPRawBlock} The block's name and attributes, changed accordingly if a match was found + * @return {BlockNode} The block's name and attributes, changed accordingly if a match was found */ -function convertLegacyBlocks( rawBlock ) { - const [ correctName, correctedAttributes ] = - convertLegacyBlockNameAndAttributes( - rawBlock.blockName, - rawBlock.attrs - ); - return { - ...rawBlock, - blockName: correctName, - attrs: correctedAttributes, - }; +function convertLegacyBlocks( block ) { + const { blockName: rawBlockName, attrs: rawAttrs } = block; + + const [ blockName, attrs ] = convertLegacyBlockNameAndAttributes( + rawBlockName, + rawAttrs + ); + + const needsCorrection = blockName !== rawBlockName || attrs !== rawAttrs; + + // Avoid cloning block data if no conversion was performed. + return needsCorrection ? { ...block, blockName, attrs } : block; } /** * Normalize the raw block by applying the fallback block name if none given, * sanitize the parsed HTML... * - * @param {WPRawBlock} rawBlock The raw block object. + * @param {BlockNode} rawBlock The raw block object. * @param {ParseOptions?} options Extra options for handling block parsing. * - * @return {WPRawBlock} The normalized block object. + * @return {BlockNode} The normalized block object. */ export function normalizeRawBlock( rawBlock, options ) { const fallbackBlockName = getFreeformContentHandlerName(); @@ -103,6 +147,7 @@ export function normalizeRawBlock( rawBlock, options ) { rawBlockName === fallbackBlockName && ! options?.__unstableSkipAutop ) { + // @TODO: Should we be running autop on all of the text chunks of innerContents? rawInnerHTML = autop( rawInnerHTML ).trim(); } @@ -118,9 +163,9 @@ export function normalizeRawBlock( rawBlock, options ) { /** * Uses the "unregistered blockType" to create a block object. * - * @param {WPRawBlock} rawBlock block. + * @param {BlockNode} rawBlock block. * - * @return {WPRawBlock} The unregistered block object. + * @return {BlockNode} The unregistered block object. */ function createMissingBlockType( rawBlock ) { const unregisteredFallbackBlock = @@ -187,10 +232,10 @@ function applyBlockValidation( unvalidatedBlock, blockType ) { /** * Given a raw block returned by grammar parsing, returns a fully parsed block. * - * @param {WPRawBlock} rawBlock The raw block object. + * @param {BlockNode} rawBlock The raw block object. * @param {ParseOptions} options Extra options for handling block parsing. * - * @return {WPBlock} Fully parsed block. + * @return {WPBlock|undefined} Fully parsed block. */ export function parseRawBlock( rawBlock, options ) { let normalizedBlock = normalizeRawBlock( rawBlock, options ); @@ -221,11 +266,45 @@ export function parseRawBlock( rawBlock, options ) { return; } - // Parse inner blocks recursively. - const parsedInnerBlocks = normalizedBlock.innerBlocks - .map( ( innerBlock ) => parseRawBlock( innerBlock, options ) ) - // See https://github.com/WordPress/gutenberg/pull/17164. - .filter( ( innerBlock ) => !! innerBlock ); + /* + * Parse inner blocks recursively. + * + * Once we can asynchronously load blocks we'll store + * a Promise for each parse. We need to insert these + * Promises in sequence here to avoid the possibility + * of re-ordering the blocks due to data races in the + * parsing and loading. + */ + const parsedInnerBlocks = []; + for ( const innerBlock of normalizedBlock.innerBlocks ) { + parsedInnerBlocks.push( parseRawBlock( innerBlock, options ) ); + } + + /* + * Once these are also resolved promises, we will then + * need to run through the list and prune any blocks + * which were removed; for example, an empty freeform + * block. We will need to `await Promise.all( blocks )` + * when that time comes before moving on to this step. + * + * Note: We normally expect few or no removed blocks, + * particularly as a ratio of all blocks. Because + * of that we'll optimize for the normal case with + * the use of `splice`, which will collapse this + * step into a quick scan through the array. + */ + for ( let i = 0; i < parsedInnerBlocks.length; i++ ) { + if ( ! parsedInnerBlocks[ i ] ) { + /* + * Some inner blocks might be removed during parsing, + * e.g. an empty freeform block. We have to remove + * these from the final array. + * + * @See https://github.com/WordPress/gutenberg/pull/17164. + */ + parsedInnerBlocks.splice( i--, 1 ); + } + } // Get the fully parsed block. const parsedBlock = createBlock( @@ -298,20 +377,46 @@ export function parseRawBlock( rawBlock, options ) { * and isolating the blocks serialized in the document and manifestly not in the * content within the blocks. * - * @see - * https://developer.wordpress.org/block-editor/packages/packages-block-serialization-default-parser/ + * @see https://developer.wordpress.org/block-editor/packages/packages-block-serialization-default-parser/ * * @param {string} content The post content. * @param {ParseOptions} options Extra options for handling block parsing. * - * @return {Array} Block list. + * @return {WPBlock[]} Block list. */ export default function parse( content, options ) { - return grammarParse( content ).reduce( ( accumulator, rawBlock ) => { - const block = parseRawBlock( rawBlock, options ); - if ( block ) { - accumulator.push( block ); + const blockNodes = grammarParse( content ); + const blocks = []; + + /* + * When we can asynchronously load blocks we will store + * a Promise for each parse. We need to insert these + * Promises in sequence here to avoid the possibility + * of re-ordering the blocks due to data races in the + * parsing and loading. + */ + for ( const blockNode of blockNodes ) { + blocks.push( parseRawBlock( blockNode, options ) ); + } + + /* + * Once these are also resolved promises, we will then + * need to run through the list and prune any blocks + * which were removed; for example, an empty freeform + * block. We will need to `await Promise.all( blocks )` + * when that time comes before moving on to this step. + * + * Note: We normally expect few or no removed blocks, + * particularly as a ratio of all blocks. Because + * of that we'll optimize for the normal case with + * the use of `splice`, which will collapse this + * step into a quick scan through the array. + */ + for ( let i = 0; i < blocks.length; i++ ) { + if ( ! blocks[ i ] ) { + blocks.splice( i--, 1 ); } - return accumulator; - }, [] ); + } + + return blocks; } diff --git a/packages/blocks/src/api/parser/serialize-raw-block.js b/packages/blocks/src/api/parser/serialize-raw-block.js index f84cedb7864131..1324cca595c9ad 100644 --- a/packages/blocks/src/api/parser/serialize-raw-block.js +++ b/packages/blocks/src/api/parser/serialize-raw-block.js @@ -8,7 +8,7 @@ import { getCommentDelimitedContent } from '../serializer'; * @property {boolean} [isCommentDelimited=true] Whether to output HTML comments around blocks. */ -/** @typedef {import("./").WPRawBlock} WPRawBlock */ +/** @typedef {import("./").BlockNode} WPRawBlock */ /** * Serializes a block node into the native HTML-comment-powered block format. @@ -40,16 +40,24 @@ export function serializeRawBlock( rawBlock, options = {} ) { } = rawBlock; let childIndex = 0; - const content = innerContent - .map( ( item ) => - // `null` denotes a nested block, otherwise we have an HTML fragment. - item !== null - ? item - : serializeRawBlock( innerBlocks[ childIndex++ ], options ) - ) - .join( '\n' ) - .replace( /\n+/g, '\n' ) - .trim(); + let content = ''; + + for ( const chunk of innerContent ) { + /* + * A `null` value indicates a placeholder for inner blocks, + * while text content will be a string. While we can directly + * append text chunks, we have to recursively serialize any + * inner blocks before appending them. + */ + const serializedChunk = + chunk === null + ? serializeRawBlock( innerBlocks[ childIndex++ ], options ) + : chunk; + + content += `${ serializedChunk }\n`; + } + + content = content.replace( /\n+/g, '\n' ).trim(); return isCommentDelimited ? getCommentDelimitedContent( blockName, attrs, content ) diff --git a/packages/blocks/src/store/reducer.js b/packages/blocks/src/store/reducer.js index 242876476f14ff..b235aad8c936d7 100644 --- a/packages/blocks/src/store/reducer.js +++ b/packages/blocks/src/store/reducer.js @@ -36,25 +36,84 @@ export const DEFAULT_CATEGORIES = [ { slug: 'reusable', title: __( 'Reusable blocks' ) }, ]; -// Key block types by their name. -function keyBlockTypesByName( types ) { - return types.reduce( - ( newBlockTypes, block ) => ( { - ...newBlockTypes, - [ block.name ]: block, - } ), - {} - ); +/** + * Create an object from a list whose keys are a given property + * of each item and whose values are the items themselves. + * + * Example + * ```js + * keyBy( [ + * { name: 'high', level: 10 }, + * { name: 'low', level: 3 } + * ] ) === { + * high: { name: 'high', level: 10 }, + * low: { name: 'low', level: 3 } + * } + * ``` + * + * @template {Record} T + * @template {keyof T} Key + * + * @param {Key} keyName Name of property in each item of whose value to use as array key. + * @param {T[]} items List of objects to transform. + * + * @return {Record} keyed object from given input items. + */ +function keyBy( keyName, items ) { + /** @type {Record} */ + const keyedObject = {}; + + for ( const item of items ) { + keyedObject[ item[ keyName ] ] = item; + } + + return keyedObject; } -// Filter items to ensure they're unique by their name. -function getUniqueItemsByName( items ) { - return items.reduce( ( acc, currentItem ) => { - if ( ! acc.some( ( item ) => item.name === currentItem.name ) ) { - acc.push( currentItem ); +/** + * Return a list whose values are unique when compared + * with the value of a given property name. + * + * Duplicate values are discarded, leaving only the first + * of several non-unique items in the output. + * + * Example + * ```js + * uniqueBy( 'name', [ + * { name: 'JavaScript', project: 'Gutenberg' }, + * { name: 'PHP', project: 'WordPress' }, + * { name: 'PHP', project: 'php-cs' }, + * { name: 'JavaScript', project: 'jquery' }, + * { name: 'C', project: 'PHP' } + * ] ) === [ + * { name: 'JavaScript', project: 'Gutenberg' }, + * { name: 'PHP', project: 'WordPress' }, + * { name: 'C', project: 'PHP' } + * ] + * ``` + * + * @template {Record} T + * @template {keyof T} Key + * + * @param {Key} keyName Name of property in each item of whose value to compare against. + * @param {T[]} items List of objects to filter. + * + * @return {T[]} List of unique items based on value of given property. + */ +function uniqueBy( keyName, items ) { + const seen = new Set(); + + const uniqueItems = []; + for ( const item of items ) { + if ( seen.has( item[ keyName ] ) ) { + continue; } - return acc; - }, [] ); + + seen.add( item[ keyName ] ); + uniqueItems.push( item ); + } + + return uniqueItems; } /** @@ -95,7 +154,7 @@ export function blockTypes( state = {}, action ) { case 'ADD_BLOCK_TYPES': return { ...state, - ...keyBlockTypesByName( action.blockTypes ), + ...keyBy( 'name', action.blockTypes ), }; case 'REMOVE_BLOCK_TYPES': return omit( state, action.names ); @@ -118,9 +177,9 @@ export function blockStyles( state = {}, action ) { return { ...state, ...mapValues( - keyBlockTypesByName( action.blockTypes ), + keyBy( 'name', action.blockTypes ), ( blockType ) => - getUniqueItemsByName( [ + uniqueBy( 'name', [ ...get( blockType, [ 'styles' ], [] ).map( ( style ) => ( { ...style, @@ -136,7 +195,7 @@ export function blockStyles( state = {}, action ) { case 'ADD_BLOCK_STYLES': return { ...state, - [ action.blockName ]: getUniqueItemsByName( [ + [ action.blockName ]: uniqueBy( 'name', [ ...get( state, [ action.blockName ], [] ), ...action.styles, ] ), @@ -168,9 +227,9 @@ export function blockVariations( state = {}, action ) { return { ...state, ...mapValues( - keyBlockTypesByName( action.blockTypes ), + keyBy( 'name', action.blockTypes ), ( blockType ) => { - return getUniqueItemsByName( [ + return uniqueBy( 'name', [ ...get( blockType, [ 'variations' ], [] ).map( ( variation ) => ( { ...variation, @@ -187,7 +246,7 @@ export function blockVariations( state = {}, action ) { case 'ADD_BLOCK_VARIATIONS': return { ...state, - [ action.blockName ]: getUniqueItemsByName( [ + [ action.blockName ]: uniqueBy( 'name', [ ...get( state, [ action.blockName ], [] ), ...action.variations, ] ),