From c228df294f1c1042dc67289fd1e096544fa16dbd Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Thu, 21 Jan 2021 17:27:56 +0000 Subject: [PATCH 01/12] Hot fix: Reusable blocks: Prevent recursion --- packages/block-library/src/block/edit.js | 11 ++++ .../src/block/has-nested-reusable-blocks.js | 11 ++++ .../block/test/has-nested-reusable-blocks.js | 61 +++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 packages/block-library/src/block/has-nested-reusable-blocks.js create mode 100644 packages/block-library/src/block/test/has-nested-reusable-blocks.js diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index e28290836a0e8b..2233094071d186 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -29,6 +29,7 @@ import { store as reusableBlocksStore } from '@wordpress/reusable-blocks'; /** * Internal dependencies */ +import hasNestedReusableBlocks from './has-nested-reusable-blocks'; export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { const { isMissing, hasResolved } = useSelect( @@ -83,6 +84,16 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { const blockProps = useBlockProps(); + if ( hasNestedReusableBlocks( blocks ) ) { + return ( +
+ + { __( `Recursion is cool, until it isn't.` ) } + +
+ ); + } + if ( isMissing ) { return (
diff --git a/packages/block-library/src/block/has-nested-reusable-blocks.js b/packages/block-library/src/block/has-nested-reusable-blocks.js new file mode 100644 index 00000000000000..4517cf23c363d4 --- /dev/null +++ b/packages/block-library/src/block/has-nested-reusable-blocks.js @@ -0,0 +1,11 @@ +const DISALLOWED_BLOCKS = [ 'core/block', 'core/template-part' ]; + +export default function hasNestedReusableBlocks( block ) { + if ( Array.isArray( block ) ) { + return block.some( hasNestedReusableBlocks ); + } + return ( + DISALLOWED_BLOCKS.includes( block.name ) || + block.innerBlocks.some( hasNestedReusableBlocks ) + ); +} diff --git a/packages/block-library/src/block/test/has-nested-reusable-blocks.js b/packages/block-library/src/block/test/has-nested-reusable-blocks.js new file mode 100644 index 00000000000000..63a411d2cbed00 --- /dev/null +++ b/packages/block-library/src/block/test/has-nested-reusable-blocks.js @@ -0,0 +1,61 @@ +/** + * Internal dependencies + */ +import hasNestedReusableBlocks from '../has-nested-reusable-blocks'; + +const badSingleBlock = [ + { + name: 'core/block', + innerBlocks: [], + }, +]; + +const goodSingleBlock = [ + { + name: 'core/paragraph', + innerBlocks: [], + }, +]; + +const badNestedBlocks = [ + { + name: 'core/paragraph', + innerBlocks: [], + }, + { + name: 'core/group', + innerBlocks: [ + { + name: 'core/template-part', + innerBlocks: [], + }, + ], + }, +]; + +const goodNestedBlocks = [ + { + name: 'core/paragraph', + innerBlocks: [], + }, + { + name: 'core/group', + innerBlocks: [ + { + name: 'core/paragraph', + innerBlocks: [], + }, + ], + }, +]; + +describe( 'hasNestedReusableBlocks', () => { + it( 'handles shallow block sets', () => { + expect( hasNestedReusableBlocks( goodSingleBlock ) ).toBe( false ); + expect( hasNestedReusableBlocks( badSingleBlock ) ).toBe( true ); + } ); + it( 'handles nested blocks', () => { + expect( hasNestedReusableBlocks( goodNestedBlocks ) ).toBe( false ); + expect( hasNestedReusableBlocks( badNestedBlocks ) ).toBe( true ); + } ); +} ); From 1c5f5dacc0b92df48c75e369c87d3b8f574d3912 Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Thu, 21 Jan 2021 19:52:50 +0000 Subject: [PATCH 02/12] FIXME Don't let editor-styles-wrapper style Warning --- packages/block-editor/src/components/warning/style.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/warning/style.scss b/packages/block-editor/src/components/warning/style.scss index c76d71e998be6a..7fb3b14a653b9f 100644 --- a/packages/block-editor/src/components/warning/style.scss +++ b/packages/block-editor/src/components/warning/style.scss @@ -14,7 +14,7 @@ line-height: $default-line-height; font-family: $default-font; font-size: $default-font-size; - color: $gray-900; + color: $gray-900 !important; margin: 0; } From 9392953892200fd441c9b4c3220b5796c8724a6b Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Thu, 21 Jan 2021 20:29:09 +0000 Subject: [PATCH 03/12] Revert "Hot fix: Reusable blocks: Prevent recursion" This reverts commit c228df294f1c1042dc67289fd1e096544fa16dbd. --- packages/block-library/src/block/edit.js | 11 ---- .../src/block/has-nested-reusable-blocks.js | 11 ---- .../block/test/has-nested-reusable-blocks.js | 61 ------------------- 3 files changed, 83 deletions(-) delete mode 100644 packages/block-library/src/block/has-nested-reusable-blocks.js delete mode 100644 packages/block-library/src/block/test/has-nested-reusable-blocks.js diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 2233094071d186..e28290836a0e8b 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -29,7 +29,6 @@ import { store as reusableBlocksStore } from '@wordpress/reusable-blocks'; /** * Internal dependencies */ -import hasNestedReusableBlocks from './has-nested-reusable-blocks'; export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { const { isMissing, hasResolved } = useSelect( @@ -84,16 +83,6 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { const blockProps = useBlockProps(); - if ( hasNestedReusableBlocks( blocks ) ) { - return ( -
- - { __( `Recursion is cool, until it isn't.` ) } - -
- ); - } - if ( isMissing ) { return (
diff --git a/packages/block-library/src/block/has-nested-reusable-blocks.js b/packages/block-library/src/block/has-nested-reusable-blocks.js deleted file mode 100644 index 4517cf23c363d4..00000000000000 --- a/packages/block-library/src/block/has-nested-reusable-blocks.js +++ /dev/null @@ -1,11 +0,0 @@ -const DISALLOWED_BLOCKS = [ 'core/block', 'core/template-part' ]; - -export default function hasNestedReusableBlocks( block ) { - if ( Array.isArray( block ) ) { - return block.some( hasNestedReusableBlocks ); - } - return ( - DISALLOWED_BLOCKS.includes( block.name ) || - block.innerBlocks.some( hasNestedReusableBlocks ) - ); -} diff --git a/packages/block-library/src/block/test/has-nested-reusable-blocks.js b/packages/block-library/src/block/test/has-nested-reusable-blocks.js deleted file mode 100644 index 63a411d2cbed00..00000000000000 --- a/packages/block-library/src/block/test/has-nested-reusable-blocks.js +++ /dev/null @@ -1,61 +0,0 @@ -/** - * Internal dependencies - */ -import hasNestedReusableBlocks from '../has-nested-reusable-blocks'; - -const badSingleBlock = [ - { - name: 'core/block', - innerBlocks: [], - }, -]; - -const goodSingleBlock = [ - { - name: 'core/paragraph', - innerBlocks: [], - }, -]; - -const badNestedBlocks = [ - { - name: 'core/paragraph', - innerBlocks: [], - }, - { - name: 'core/group', - innerBlocks: [ - { - name: 'core/template-part', - innerBlocks: [], - }, - ], - }, -]; - -const goodNestedBlocks = [ - { - name: 'core/paragraph', - innerBlocks: [], - }, - { - name: 'core/group', - innerBlocks: [ - { - name: 'core/paragraph', - innerBlocks: [], - }, - ], - }, -]; - -describe( 'hasNestedReusableBlocks', () => { - it( 'handles shallow block sets', () => { - expect( hasNestedReusableBlocks( goodSingleBlock ) ).toBe( false ); - expect( hasNestedReusableBlocks( badSingleBlock ) ).toBe( true ); - } ); - it( 'handles nested blocks', () => { - expect( hasNestedReusableBlocks( goodNestedBlocks ) ).toBe( false ); - expect( hasNestedReusableBlocks( badNestedBlocks ) ).toBe( true ); - } ); -} ); From 51deb88aa9f8c3331b53875cf98c3503161f5aa6 Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Thu, 21 Jan 2021 20:32:26 +0000 Subject: [PATCH 04/12] Reusable blocks: stop client-side infinite recursion --- packages/block-library/src/block/edit.js | 60 ++++++++++++------- .../src/block/use-no-recursive-renders.js | 29 +++++++++ 2 files changed, 67 insertions(+), 22 deletions(-) create mode 100644 packages/block-library/src/block/use-no-recursive-renders.js diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index e28290836a0e8b..09b4f058a5227a 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -29,8 +29,12 @@ import { store as reusableBlocksStore } from '@wordpress/reusable-blocks'; /** * Internal dependencies */ +import useNoRecursiveRenders from './use-no-recursive-renders'; export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { + const [ hasAlreadyRendered, RecursionProvider ] = useNoRecursiveRenders( + ref + ); const { isMissing, hasResolved } = useSelect( ( select ) => { const persistedBlock = select( coreStore ).getEntityRecord( @@ -83,6 +87,16 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { const blockProps = useBlockProps(); + if ( hasAlreadyRendered ) { + return ( +
+ + { __( 'Block cannot be rendered inside itself.' ) } + +
+ ); + } + if ( isMissing ) { return (
@@ -104,28 +118,30 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { } return ( -
- - - convertBlockToStatic( clientId ) } - > - { __( 'Convert to regular blocks' ) } - - - - - - - - -
- {
} + +
+ + + convertBlockToStatic( clientId ) } + > + { __( 'Convert to regular blocks' ) } + + + + + + + + +
+ {
} +
-
+ ); } diff --git a/packages/block-library/src/block/use-no-recursive-renders.js b/packages/block-library/src/block/use-no-recursive-renders.js new file mode 100644 index 00000000000000..b17ddd9b4f6186 --- /dev/null +++ b/packages/block-library/src/block/use-no-recursive-renders.js @@ -0,0 +1,29 @@ +/** + * WordPress dependencies + */ +import { + createContext, + useCallback, + useContext, + useMemo, +} from '@wordpress/element'; + +const RenderedRefsContext = createContext( [] ); + +export default function useNoRecursiveRenders( ref ) { + const previouslyRenderedRefs = useContext( RenderedRefsContext ); + const hasAlreadyRendered = previouslyRenderedRefs.includes( ref ); + const newRenderedRefs = useMemo( () => [ ...previouslyRenderedRefs, ref ], [ + ref, + previouslyRenderedRefs, + ] ); + const Provider = useCallback( + ( { children } ) => ( + + { children } + + ), + [ newRenderedRefs ] + ); + return [ hasAlreadyRendered, Provider ]; +} From f9c9dd6f6845ad5415bd3d7cf9432d24267de31e Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Thu, 21 Jan 2021 20:35:21 +0000 Subject: [PATCH 05/12] Reusable blocks: stop server-side infinite recursion --- packages/block-library/src/block/index.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index 5fed48bd9a7c74..ab33374f3bf2e5 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -13,10 +13,18 @@ * @return string Rendered HTML of the referenced block. */ function render_block_core_block( $attributes ) { + static $seen_refs = array(); + if ( empty( $attributes['ref'] ) ) { return ''; } + if ( in_array( $attributes['ref'], $seen_refs, true ) ) { + return 'nope'; + } + + $seen_refs[] = $attributes['ref']; + $reusable_block = get_post( $attributes['ref'] ); if ( ! $reusable_block || 'wp_block' !== $reusable_block->post_type ) { return ''; From 2f93b2be9bf4cb737db57eec8292a32d125b994f Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Fri, 22 Jan 2021 11:15:59 +0000 Subject: [PATCH 06/12] Trigger user warning on the front end --- packages/block-library/src/block/index.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index ab33374f3bf2e5..b9d4585bd05b57 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -20,7 +20,16 @@ function render_block_core_block( $attributes ) { } if ( in_array( $attributes['ref'], $seen_refs, true ) ) { - return 'nope'; + trigger_error( + sprintf( + // translators: %s is the block name, e.g. core/block. + __( 'Block %s cannot be rendered inside itself.' ), + 'core/block' + ), + E_USER_WARNING + ); + + return '[block rendering halted]'; } $seen_refs[] = $attributes['ref']; From 28d8c429f113421e6827356d75b2c250e6130225 Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Fri, 22 Jan 2021 11:48:50 +0000 Subject: [PATCH 07/12] Add tests for useNoRecursiveRenders --- .../block/test/use-no-recursive-renders.js | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 packages/block-library/src/block/test/use-no-recursive-renders.js diff --git a/packages/block-library/src/block/test/use-no-recursive-renders.js b/packages/block-library/src/block/test/use-no-recursive-renders.js new file mode 100644 index 00000000000000..a9d9f6e53b789a --- /dev/null +++ b/packages/block-library/src/block/test/use-no-recursive-renders.js @@ -0,0 +1,100 @@ +/** + * External dependencies + */ +import { render } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import { Fragment } from '@wordpress/element'; + +// Mimics a block's Edit component, such as ReusableBlockEdit, which is capable +// of calling itself depending on its `ref` attribute. +function Edit( { attributes: { ref } } ) { + const [ hasAlreadyRendered, RecursionProvider ] = useNoRecursiveRenders( + ref + ); + + if ( hasAlreadyRendered ) { + return
Halt
; + } + + return ( + +
+ { ref === 'SIMPLE' &&

Done

} + { ref === 'SINGLY-RECURSIVE' && ( + + ) } + { ref === 'MUTUALLY-RECURSIVE-1' && ( + + ) } + { ref === 'MUTUALLY-RECURSIVE-2' && ( + + ) } +
+ + ); +} + +/** + * Internal dependencies + */ +import useNoRecursiveRenders from '../use-no-recursive-renders'; + +describe( 'useNoRecursiveRenders', () => { + it( 'allows a single block to render', () => { + const { container } = render( + + ); + expect( + container.querySelectorAll( '.wp-block__reusable-block' ) + ).toHaveLength( 1 ); + expect( + container.querySelectorAll( '.wp-block__reusable-block--halted' ) + ).toHaveLength( 0 ); + } ); + + it( 'allows equal but sibling blocks to render', () => { + const { container } = render( + + + + + ); + expect( + container.querySelectorAll( '.wp-block__reusable-block' ) + ).toHaveLength( 2 ); + expect( + container.querySelectorAll( '.wp-block__reusable-block--halted' ) + ).toHaveLength( 0 ); + } ); + + it( 'prevents a block from rendering itself', () => { + const { container } = render( + + + + ); + expect( + container.querySelectorAll( '.wp-block__reusable-block' ) + ).toHaveLength( 1 ); + expect( + container.querySelectorAll( '.wp-block__reusable-block--halted' ) + ).toHaveLength( 1 ); + } ); + + it( 'prevents mutual recursion between two blocks', () => { + const { container } = render( + + + + ); + expect( + container.querySelectorAll( '.wp-block__reusable-block' ) + ).toHaveLength( 2 ); + expect( + container.querySelectorAll( '.wp-block__reusable-block--halted' ) + ).toHaveLength( 1 ); + } ); +} ); From 9c7d4af1914309aff80bfbd5f2cd6ac5652fa4ab Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Fri, 22 Jan 2021 11:55:31 +0000 Subject: [PATCH 08/12] Include reusable block's title in front-end warning --- packages/block-library/src/block/index.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index b9d4585bd05b57..3065986e7bf044 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -19,12 +19,17 @@ function render_block_core_block( $attributes ) { return ''; } + $reusable_block = get_post( $attributes['ref'] ); + if ( ! $reusable_block || 'wp_block' !== $reusable_block->post_type ) { + return ''; + } + if ( in_array( $attributes['ref'], $seen_refs, true ) ) { trigger_error( sprintf( - // translators: %s is the block name, e.g. core/block. - __( 'Block %s cannot be rendered inside itself.' ), - 'core/block' + // translators: %s is the user-provided title of the reusable block. + __( 'Could not render Reusable Block %s: blocks cannot be rendered inside themselves.' ), + $reusable_block->post_title ), E_USER_WARNING ); @@ -34,11 +39,6 @@ function render_block_core_block( $attributes ) { $seen_refs[] = $attributes['ref']; - $reusable_block = get_post( $attributes['ref'] ); - if ( ! $reusable_block || 'wp_block' !== $reusable_block->post_type ) { - return ''; - } - if ( 'publish' !== $reusable_block->post_status || ! empty( $reusable_block->post_password ) ) { return ''; } From 90eb5866c4f37d81f0f4d65b1b1bb38f52f65dba Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Fri, 22 Jan 2021 12:38:00 +0000 Subject: [PATCH 09/12] Trigger PHP warning only in front end, not admin Addresses feedback by ntsekouras --- packages/block-library/src/block/index.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index 3065986e7bf044..653a30a592f429 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -25,14 +25,16 @@ function render_block_core_block( $attributes ) { } if ( in_array( $attributes['ref'], $seen_refs, true ) ) { - trigger_error( - sprintf( - // translators: %s is the user-provided title of the reusable block. - __( 'Could not render Reusable Block %s: blocks cannot be rendered inside themselves.' ), - $reusable_block->post_title - ), - E_USER_WARNING - ); + if ( ! is_admin() ) { + trigger_error( + sprintf( + // translators: %s is the user-provided title of the reusable block. + __( 'Could not render Reusable Block %s: blocks cannot be rendered inside themselves.' ), + $reusable_block->post_title + ), + E_USER_WARNING + ); + } return '[block rendering halted]'; } From 1ee976717b94b46318ba954f61b1e699021e6261 Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Fri, 22 Jan 2021 12:42:52 +0000 Subject: [PATCH 10/12] Revert "FIXME Don't let editor-styles-wrapper style Warning" This reverts commit 1c5f5dacc0b92df48c75e369c87d3b8f574d3912. --- packages/block-editor/src/components/warning/style.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/warning/style.scss b/packages/block-editor/src/components/warning/style.scss index 7fb3b14a653b9f..c76d71e998be6a 100644 --- a/packages/block-editor/src/components/warning/style.scss +++ b/packages/block-editor/src/components/warning/style.scss @@ -14,7 +14,7 @@ line-height: $default-line-height; font-family: $default-font; font-size: $default-font-size; - color: $gray-900 !important; + color: $gray-900; margin: 0; } From bee83e82fabaace0fa8881a95dc47872bd49b62b Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Fri, 22 Jan 2021 14:17:29 +0000 Subject: [PATCH 11/12] Honor WP_DEBUG_DISPLAY before displaying failure point --- packages/block-library/src/block/index.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index 653a30a592f429..86b899e23ce80f 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -36,7 +36,12 @@ function render_block_core_block( $attributes ) { ); } - return '[block rendering halted]'; + // WP_DEBUG_DISPLAY must only be honored when WP_DEBUG. This precedent + // is set in `wp_debug_mode()`. + $is_debug = defined( 'WP_DEBUG' ) && WP_DEBUG && + defined( 'WP_DEBUG_DISPLAY' ) && WP_DEBUG_DISPLAY; + + return $is_debug ? '[block rendering halted]' : ''; } $seen_refs[] = $attributes['ref']; From 0537efe507bdd0b37fa81e220af0c4db77f81121 Mon Sep 17 00:00:00 2001 From: Miguel Fonseca Date: Fri, 22 Jan 2021 14:25:20 +0000 Subject: [PATCH 12/12] Translate "[block rendering halted]" snippet --- packages/block-library/src/block/index.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index 86b899e23ce80f..771adf27048db8 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -41,7 +41,10 @@ function render_block_core_block( $attributes ) { $is_debug = defined( 'WP_DEBUG' ) && WP_DEBUG && defined( 'WP_DEBUG_DISPLAY' ) && WP_DEBUG_DISPLAY; - return $is_debug ? '[block rendering halted]' : ''; + return $is_debug ? + // translators: Visible only in the front end, this warning takes the place of a faulty block. + __( '[block rendering halted]' ) : + ''; } $seen_refs[] = $attributes['ref'];