From 88f599c20a6e353f3f798331a97208b443709baf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:07:11 +0200 Subject: [PATCH 01/13] Remove hotfix --- packages/block-editor/src/store/selectors.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 787225f18a0a57..b5afb2891ef1cf 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -1628,18 +1628,8 @@ const isBlockVisibleInTheInserter = ( checkedBlocks.add( blockName ); // If parent blocks are not visible, child blocks should be hidden too. - // - // In some scenarios, blockType.parent may be a string. - // A better approach would be sanitize parent in all the places that can be modified: - // block registration, processBlockType, filters, etc. - // In the meantime, this is a hotfix to prevent the editor from crashing. - const parent = - typeof blockType.parent === 'string' || - blockType.parent instanceof String - ? [ blockType.parent ] - : blockType.parent; - if ( Array.isArray( parent ) ) { - return parent.some( + if ( Array.isArray( blockType.parent ) ) { + return blockType.parent.some( ( name ) => ( blockName !== name && isBlockVisibleInTheInserter( From ad17f4a7ef66e532eff9d10c0c59dbcb363df743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Fri, 18 Oct 2024 17:52:26 +0200 Subject: [PATCH 02/13] Convert parent to array in block registration --- packages/blocks/src/api/registration.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/blocks/src/api/registration.js b/packages/blocks/src/api/registration.js index ef1b6bd20a4cdd..429a7235cded48 100644 --- a/packages/blocks/src/api/registration.js +++ b/packages/blocks/src/api/registration.js @@ -232,6 +232,13 @@ export function registerBlockType( blockNameOrMetadata, settings ) { return; } + if ( + typeof settings?.parent === 'string' || + settings?.parent instanceof String + ) { + settings.parent = [ settings.parent ]; + } + if ( 1 === settings?.parent?.length && name === settings.parent[ 0 ] ) { warning( 'Block "' + From c9a842b23c663d84487f7e1e3ee7bdbdf19d7162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Fri, 18 Oct 2024 17:52:40 +0200 Subject: [PATCH 03/13] Convert parent to array in block processing --- packages/blocks/src/store/process-block-type.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/blocks/src/store/process-block-type.js b/packages/blocks/src/store/process-block-type.js index ae8f92f84cfabb..dfc8e47ac051d4 100644 --- a/packages/blocks/src/store/process-block-type.js +++ b/packages/blocks/src/store/process-block-type.js @@ -197,5 +197,12 @@ export const processBlockType = return; } + if ( + typeof settings.parent === 'string' || + settings.parent instanceof String + ) { + settings.parent = [ settings.parent ]; + } + return settings; }; From d73432c68261044abb6c1ce18eedec846f93d7ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 21 Oct 2024 12:33:21 +0200 Subject: [PATCH 04/13] Test parent is array after having registered as a string --- packages/blocks/src/api/test/registration.js | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/blocks/src/api/test/registration.js b/packages/blocks/src/api/test/registration.js index 656e9471a541b8..7db5cc7a65c8a6 100644 --- a/packages/blocks/src/api/test/registration.js +++ b/packages/blocks/src/api/test/registration.js @@ -742,6 +742,33 @@ describe( 'blocks', () => { } ); } ); + it( 'should transform parent string to array', () => { + const blockType = { + save: noop, + category: 'text', + title: 'block title', + parent: 'core/paragraph', + }; + registerBlockType( 'core/test-block-parent-string', blockType ); + expect( getBlockType( 'core/test-block-parent-string' ) ).toEqual( { + name: 'core/test-block-parent-string', + save: noop, + category: 'text', + title: 'block title', + icon: { src: BLOCK_ICON_DEFAULT }, + attributes: {}, + providesContext: {}, + usesContext: [], + keywords: [], + selectors: {}, + supports: {}, + styles: [], + variations: [], + blockHooks: {}, + parent: [ 'core/paragraph' ], + } ); + } ); + describe( 'applyFilters', () => { afterEach( () => { removeAllFilters( 'blocks.registerBlockType' ); From 098d5dcd379795bfc5974dd27d006a1b93069847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 21 Oct 2024 17:26:16 +0200 Subject: [PATCH 05/13] Enforce parent being an array if it is present --- packages/blocks/src/api/registration.js | 11 +++++++++++ packages/blocks/src/store/process-block-type.js | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/packages/blocks/src/api/registration.js b/packages/blocks/src/api/registration.js index 429a7235cded48..ef7a2d464b503c 100644 --- a/packages/blocks/src/api/registration.js +++ b/packages/blocks/src/api/registration.js @@ -232,6 +232,9 @@ export function registerBlockType( blockNameOrMetadata, settings ) { return; } + // Parent being an array hasn't been enforced in the past, + // so this is a way to maintain backwards compatibility + // with 3rd party blocks that may have been using it as a string. if ( typeof settings?.parent === 'string' || settings?.parent instanceof String @@ -239,6 +242,14 @@ export function registerBlockType( blockNameOrMetadata, settings ) { settings.parent = [ settings.parent ]; } + if ( ! Array.isArray( settings?.parent ) && 'parent' in settings ) { + warning( + 'Block parent must be an array of block types, but it is ', + settings.parent + ); + return; + } + if ( 1 === settings?.parent?.length && name === settings.parent[ 0 ] ) { warning( 'Block "' + diff --git a/packages/blocks/src/store/process-block-type.js b/packages/blocks/src/store/process-block-type.js index dfc8e47ac051d4..030ce594496074 100644 --- a/packages/blocks/src/store/process-block-type.js +++ b/packages/blocks/src/store/process-block-type.js @@ -197,6 +197,9 @@ export const processBlockType = return; } + // Parent being an array hasn't been enforced in the past, + // so this is a way to maintain backwards compatibility + // with 3rd party blocks that may have been using it as a string. if ( typeof settings.parent === 'string' || settings.parent instanceof String @@ -204,5 +207,13 @@ export const processBlockType = settings.parent = [ settings.parent ]; } + if ( ! Array.isArray( settings?.parent ) && 'parent' in settings ) { + warning( + 'Block parent must be an array of block types, but it is ', + settings?.parent + ); + return; + } + return settings; }; From 3ec812c3d1858c0aa39a71b3907e229306e512a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 21 Oct 2024 17:33:19 +0200 Subject: [PATCH 06/13] Prevent against setting being undefined --- packages/blocks/src/api/registration.js | 5 ++++- packages/blocks/src/store/process-block-type.js | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/blocks/src/api/registration.js b/packages/blocks/src/api/registration.js index ef7a2d464b503c..cd0c95c59fab83 100644 --- a/packages/blocks/src/api/registration.js +++ b/packages/blocks/src/api/registration.js @@ -242,7 +242,10 @@ export function registerBlockType( blockNameOrMetadata, settings ) { settings.parent = [ settings.parent ]; } - if ( ! Array.isArray( settings?.parent ) && 'parent' in settings ) { + if ( + ! Array.isArray( settings?.parent ) && + settings?.parent !== undefined + ) { warning( 'Block parent must be an array of block types, but it is ', settings.parent diff --git a/packages/blocks/src/store/process-block-type.js b/packages/blocks/src/store/process-block-type.js index 030ce594496074..95dcc86f20bc58 100644 --- a/packages/blocks/src/store/process-block-type.js +++ b/packages/blocks/src/store/process-block-type.js @@ -207,7 +207,10 @@ export const processBlockType = settings.parent = [ settings.parent ]; } - if ( ! Array.isArray( settings?.parent ) && 'parent' in settings ) { + if ( + ! Array.isArray( settings?.parent ) && + settings?.parent !== undefined + ) { warning( 'Block parent must be an array of block types, but it is ', settings?.parent From 0db777fe7c9e439f9ee984802aab1db63fb49ea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 21 Oct 2024 17:37:35 +0200 Subject: [PATCH 07/13] Raises warning if parent is string and document in CHANGELOG --- packages/blocks/CHANGELOG.md | 2 ++ packages/blocks/src/api/registration.js | 3 +++ packages/blocks/src/store/process-block-type.js | 3 +++ 3 files changed, 8 insertions(+) diff --git a/packages/blocks/CHANGELOG.md b/packages/blocks/CHANGELOG.md index 73a700428674b2..706b86f20dc200 100644 --- a/packages/blocks/CHANGELOG.md +++ b/packages/blocks/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Enforce `blockType.parent` to be an array. While `parent` being a string wasn't never supported, it worked with some unintended side-effects that have been fixed by [#66250](https://github.com/WordPress/gutenberg/pull/66250). For that reason, we've added some code that automatically migrates strings to arrays — though it still raises a warning. + ## 13.10.0 (2024-10-16) ## 13.9.0 (2024-10-03) diff --git a/packages/blocks/src/api/registration.js b/packages/blocks/src/api/registration.js index cd0c95c59fab83..85a3f2a213cd59 100644 --- a/packages/blocks/src/api/registration.js +++ b/packages/blocks/src/api/registration.js @@ -240,6 +240,9 @@ export function registerBlockType( blockNameOrMetadata, settings ) { settings?.parent instanceof String ) { settings.parent = [ settings.parent ]; + warning( + 'Block parent must be an array of block types, but it is a string' + ); } if ( diff --git a/packages/blocks/src/store/process-block-type.js b/packages/blocks/src/store/process-block-type.js index 95dcc86f20bc58..29ff031c6d1c10 100644 --- a/packages/blocks/src/store/process-block-type.js +++ b/packages/blocks/src/store/process-block-type.js @@ -205,6 +205,9 @@ export const processBlockType = settings.parent instanceof String ) { settings.parent = [ settings.parent ]; + warning( + 'Block parent must be an array of block types, but it is a string' + ); } if ( From 6248fd0738e0bdd9f26880e5862f3f92adab47a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 21 Oct 2024 17:47:56 +0200 Subject: [PATCH 08/13] Remove warning --- packages/blocks/src/api/registration.js | 3 --- packages/blocks/src/store/process-block-type.js | 3 --- 2 files changed, 6 deletions(-) diff --git a/packages/blocks/src/api/registration.js b/packages/blocks/src/api/registration.js index 85a3f2a213cd59..cd0c95c59fab83 100644 --- a/packages/blocks/src/api/registration.js +++ b/packages/blocks/src/api/registration.js @@ -240,9 +240,6 @@ export function registerBlockType( blockNameOrMetadata, settings ) { settings?.parent instanceof String ) { settings.parent = [ settings.parent ]; - warning( - 'Block parent must be an array of block types, but it is a string' - ); } if ( diff --git a/packages/blocks/src/store/process-block-type.js b/packages/blocks/src/store/process-block-type.js index 29ff031c6d1c10..95dcc86f20bc58 100644 --- a/packages/blocks/src/store/process-block-type.js +++ b/packages/blocks/src/store/process-block-type.js @@ -205,9 +205,6 @@ export const processBlockType = settings.parent instanceof String ) { settings.parent = [ settings.parent ]; - warning( - 'Block parent must be an array of block types, but it is a string' - ); } if ( From d9a16db65a030f5d95f9cdcf3036f66fab7e484f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 22 Oct 2024 08:34:58 +0200 Subject: [PATCH 09/13] Add Bug fixes heading --- packages/blocks/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/blocks/CHANGELOG.md b/packages/blocks/CHANGELOG.md index 706b86f20dc200..1404a6fe1e3e20 100644 --- a/packages/blocks/CHANGELOG.md +++ b/packages/blocks/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +### Bug fixes + - Enforce `blockType.parent` to be an array. While `parent` being a string wasn't never supported, it worked with some unintended side-effects that have been fixed by [#66250](https://github.com/WordPress/gutenberg/pull/66250). For that reason, we've added some code that automatically migrates strings to arrays — though it still raises a warning. ## 13.10.0 (2024-10-16) From 002ac7cec3337f5f5d56c9f154a3e337b890f3c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 22 Oct 2024 13:05:15 +0200 Subject: [PATCH 10/13] Move validation to processBlockType --- packages/blocks/src/api/registration.js | 30 ------------------- .../blocks/src/store/process-block-type.js | 17 ++++++++--- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/packages/blocks/src/api/registration.js b/packages/blocks/src/api/registration.js index cd0c95c59fab83..2f4bab2b5f2589 100644 --- a/packages/blocks/src/api/registration.js +++ b/packages/blocks/src/api/registration.js @@ -232,36 +232,6 @@ export function registerBlockType( blockNameOrMetadata, settings ) { return; } - // Parent being an array hasn't been enforced in the past, - // so this is a way to maintain backwards compatibility - // with 3rd party blocks that may have been using it as a string. - if ( - typeof settings?.parent === 'string' || - settings?.parent instanceof String - ) { - settings.parent = [ settings.parent ]; - } - - if ( - ! Array.isArray( settings?.parent ) && - settings?.parent !== undefined - ) { - warning( - 'Block parent must be an array of block types, but it is ', - settings.parent - ); - return; - } - - if ( 1 === settings?.parent?.length && name === settings.parent[ 0 ] ) { - warning( - 'Block "' + - name + - '" cannot be a parent of itself. Please remove the block name from the parent list.' - ); - return; - } - if ( ! /^[a-z][a-z0-9-]*\/[a-z][a-z0-9-]*$/.test( name ) ) { warning( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' diff --git a/packages/blocks/src/store/process-block-type.js b/packages/blocks/src/store/process-block-type.js index 95dcc86f20bc58..63bef081a6b5dc 100644 --- a/packages/blocks/src/store/process-block-type.js +++ b/packages/blocks/src/store/process-block-type.js @@ -201,8 +201,8 @@ export const processBlockType = // so this is a way to maintain backwards compatibility // with 3rd party blocks that may have been using it as a string. if ( - typeof settings.parent === 'string' || - settings.parent instanceof String + typeof settings?.parent === 'string' || + settings?.parent instanceof String ) { settings.parent = [ settings.parent ]; } @@ -212,8 +212,17 @@ export const processBlockType = settings?.parent !== undefined ) { warning( - 'Block parent must be an array of block types, but it is ', - settings?.parent + 'Block parent must be undefined or an array of block types, but it is ', + settings.parent + ); + return; + } + + if ( 1 === settings?.parent?.length && name === settings.parent[ 0 ] ) { + warning( + 'Block "' + + name + + '" cannot be a parent of itself. Please remove the block name from the parent list.' ); return; } From f35fb12525adc54d8ae54cd0da8e66901bf539de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 22 Oct 2024 13:16:04 +0200 Subject: [PATCH 11/13] Update changelog --- packages/blocks/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/blocks/CHANGELOG.md b/packages/blocks/CHANGELOG.md index 1404a6fe1e3e20..c8225d811031a6 100644 --- a/packages/blocks/CHANGELOG.md +++ b/packages/blocks/CHANGELOG.md @@ -4,7 +4,7 @@ ### Bug fixes -- Enforce `blockType.parent` to be an array. While `parent` being a string wasn't never supported, it worked with some unintended side-effects that have been fixed by [#66250](https://github.com/WordPress/gutenberg/pull/66250). For that reason, we've added some code that automatically migrates strings to arrays — though it still raises a warning. +- Normalize `blockType.parent` to be an array. While string values were never supported, they appeared to work with some unintended side-effects that have been fixed by [#66250](https://github.com/WordPress/gutenberg/pull/66250). For that reason, we've added some code that automatically migrates strings to arrays — though it still raises a warning. ## 13.10.0 (2024-10-16) From 4369ec0f999dbd33d599a0d5d43f01df5a603cf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 22 Oct 2024 13:26:21 +0200 Subject: [PATCH 12/13] Add back test and warning about parent as a string --- packages/blocks/src/api/test/registration.js | 10 ++++++++-- packages/blocks/src/store/process-block-type.js | 14 ++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/blocks/src/api/test/registration.js b/packages/blocks/src/api/test/registration.js index 7db5cc7a65c8a6..5941415e61fe55 100644 --- a/packages/blocks/src/api/test/registration.js +++ b/packages/blocks/src/api/test/registration.js @@ -749,8 +749,14 @@ describe( 'blocks', () => { title: 'block title', parent: 'core/paragraph', }; - registerBlockType( 'core/test-block-parent-string', blockType ); - expect( getBlockType( 'core/test-block-parent-string' ) ).toEqual( { + const block = registerBlockType( + 'core/test-block-parent-string', + blockType + ); + expect( console ).toHaveWarnedWith( + 'Parent must be undefined or an array of strings (block types), but it is a string.' + ); + expect( block ).toEqual( { name: 'core/test-block-parent-string', save: noop, category: 'text', diff --git a/packages/blocks/src/store/process-block-type.js b/packages/blocks/src/store/process-block-type.js index 63bef081a6b5dc..bc7b1a0e10e774 100644 --- a/packages/blocks/src/store/process-block-type.js +++ b/packages/blocks/src/store/process-block-type.js @@ -197,14 +197,20 @@ export const processBlockType = return; } - // Parent being an array hasn't been enforced in the past, - // so this is a way to maintain backwards compatibility - // with 3rd party blocks that may have been using it as a string. if ( typeof settings?.parent === 'string' || settings?.parent instanceof String ) { settings.parent = [ settings.parent ]; + warning( + 'Parent must be undefined or an array of strings (block types), but it is a string.' + ); + // Intentionally continue: + // + // While string values were never supported, they appeared to work with some unintended side-effects + // that have been fixed by [#66250](https://github.com/WordPress/gutenberg/pull/66250). + // + // To be backwards-compatible, this code that automatically migrates strings to arrays. } if ( @@ -212,7 +218,7 @@ export const processBlockType = settings?.parent !== undefined ) { warning( - 'Block parent must be undefined or an array of block types, but it is ', + 'Parent must be undefined or an array of block types, but it is ', settings.parent ); return; From 83bfdf98a9a4ad73e7cc720ccc1d8a33d299e9d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:34:29 +0200 Subject: [PATCH 13/13] Change Bug to BreakingChange --- packages/blocks/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/blocks/CHANGELOG.md b/packages/blocks/CHANGELOG.md index c8225d811031a6..eebbe7148e8055 100644 --- a/packages/blocks/CHANGELOG.md +++ b/packages/blocks/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -### Bug fixes +### Breaking changes - Normalize `blockType.parent` to be an array. While string values were never supported, they appeared to work with some unintended side-effects that have been fixed by [#66250](https://github.com/WordPress/gutenberg/pull/66250). For that reason, we've added some code that automatically migrates strings to arrays — though it still raises a warning.