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

React methods are executed infinited times when block is inserted through a custom pattern #63909

Closed
2 tasks done
htmgarcia opened this issue Jul 24, 2024 · 10 comments · Fixed by #65029
Closed
2 tasks done
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Bug An existing feature does not function as intended

Comments

@htmgarcia
Copy link

Description

Custom Gutenberg blocks that uses React native methods such as componentDidMount() can break the editor due an infinite loop. Only happens if the block is inserted through a custom pattern.

Errors in console are:
Uncaught Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.

This started happening in WordPress 6.6.
In WordPress 6.5 this wasn't happening.

Step-by-step reproduction instructions

Create a custom plugin that will load our custom block that includes componentDidMount() and constructor() methods. If you have a ready custom plugin, you can just add these methods in the edit component too.

File structure of the custom plugin is as follows. Simply create those 2 files in an existing WordPress 6.6 site:

wp-content/plugins/custom-plugin/custom-plugin.php

<?php
/*
Plugin Name: My Custom Plugin
Description: A custom Gutenberg block for paragraphs with additional settings.
Version: 1.0
Author: @htmgarcia
*/

function my_custom_paragraph_block() {
    // Register the block script
    wp_register_script(
        'my-custom-paragraph',
        plugins_url('block.js', __FILE__),
        array('wp-blocks', 'wp-i18n', 'wp-element', 'wp-editor', 'wp-components', 'wp-block-editor'),
        filemtime(plugin_dir_path(__FILE__) . 'block.js')
    );

    // Register the block type with the editor script
    register_block_type('my-plugin/my-custom-paragraph', array(
        'editor_script' => 'my-custom-paragraph',
    ));
}
add_action('init', 'my_custom_paragraph_block');

wp-content/plugins/custom-plugin/block.js

const { __ } = wp.i18n;
const { RichText, InspectorControls } = wp.blockEditor;
const { PanelBody, TextControl } = wp.components;
const { Component, Fragment, createElement } = wp.element;

class EditComponent extends Component {
    constructor(props) {
        super(props);
        this.onChangeContent = this.onChangeContent.bind(this);
        this.onChangeCustomText = this.onChangeCustomText.bind(this);

        // Let's observe how many times this is outputted
        console.log('constructor()');
    }

    componentDidMount() {
        const { clientId, attributes, setAttributes } = this.props;

        // Let's observe how many times this is outputted
        console.log('componentDidMount()');

        // Set the blockID to the clientId to maybe use later to link to custom CSS
        setAttributes({ blockID: clientId });
    }

    onChangeContent(newContent) {
        this.props.setAttributes({ content: newContent });
    }

    onChangeCustomText(newCustomText) {
        this.props.setAttributes({ customText: newCustomText });
    }

    render() {
        const { attributes: { content, customText } } = this.props;

        return createElement(
            Fragment,
            null,
            createElement(
                InspectorControls,
                null,
                createElement(
                    PanelBody,
                    { title: __('Custom Settings', 'custom-plugin'), initialOpen: true },
                    createElement(
                        TextControl,
                        {
                            label: __('Custom Text', 'custom-plugin'),
                            value: customText,
                            onChange: this.onChangeCustomText
                        }
                    )
                )
            ),
            createElement(
                RichText,
                {
                    tagName: "p",
                    value: content,
                    onChange: this.onChangeContent,
                    placeholder: __('Write your custom paragraph...', 'custom-plugin')
                }
            )
        );
    }
}

// Ensure that this code runs only after the Gutenberg editor has loaded
wp.domReady(function() {
    const { registerBlockType } = wp.blocks;

    // Register the custom paragraph block
    registerBlockType('custom-plugin/my-custom-paragraph', {
        title: __('Custom Paragraph', 'custom-plugin'),
        icon: 'editor-paragraph',
        category: 'common',
        attributes: {
            content: {
                type: 'string',
                source: 'html',
                selector: 'p',
            },
            customText: {
                type: 'string',
                default: '',
            },
            blockID: {
                type: 'string',
            }
        },
        edit: EditComponent,
        save: function(props) {
            const { attributes: { content, customText } } = props;

            return createElement(
                "p",
                null,
                content,
                customText && createElement(
                    "span",
                    null,
                    ' - ',
                    customText
                )
            );
        },
    });
});

The parts of the code we're analyzing here are componentDidMount() and constructor() from EditComponent class. I added on purpose a couple of log lines to observe in browser console when inserting "Custom Paragraph" block.

class EditComponent extends Component {
    constructor(props) {
        super(props);
        this.onChangeContent = this.onChangeContent.bind(this);
        this.onChangeCustomText = this.onChangeCustomText.bind(this);

        // Let's observe how many times this is outputted
        console.log('constructor()');
    }

    componentDidMount() {
        const { clientId, attributes, setAttributes } = this.props;

        // Let's observe how many times this is outputted
        console.log('componentDidMount()');

        // Set the blockID to the clientId to maybe use later to link to custom CSS
        setAttributes({ blockID: clientId });
    }

    // REST OF THE CODE ...
}

Activate the custom plugin and test "Custom paragraph" block

  • Go to Plugins admin page and activate "My Custom Plugin"
  • Edit or create a page
  • Insert a "Custom paragraph" block and type some text to confirm the block is listed and works

Screenshot 2024-07-24 at 9 15 50 a m

Create a custom pattern

Convert the "Custom paragraph" block into a pattern by doing click in the 3-dots > Create pattern

Screenshot 2024-07-24 at 9 38 20 a m

Type a name and click "Add"

Screenshot 2024-07-24 at 9 39 16 a m

Then the screen will get stuck in the loading process and editor stop working

Screenshot 2024-07-24 at 9 39 23 a m

If you repeat the process by having the browser console opened, you'll see the logs for componentDidMount() and constructor() are outputted infinite times.

Screenshot 2024-07-24 at 8 56 11 a m

The problem happens also when you try to insert the custom pattern (because pattern is created but editor can't insert into page).

Partial solution?

The culprit of the infinite loop according to logs points to setAttributes() method inside componentDidMount(). If I add a check, the issue seems "solved", however this check is not needed when the block is inserted normally (not as a pattern), plus the logic of the block requires a unique value for blockID for every new instance, meaning the if() is messing with it, particularly if I duplicate the block. The value for the clones will be the same (non unique).

if (!attributes.blockID) {
    setAttributes({ blockID: clientId });
}

This is how 2 instances of the custom block looks like after duplicating the first one. Same value for blockID. So the fix is not good enough.

Screenshot 2024-07-24 at 9 52 33 a m

I wonder what changed in core to potentially cause an infinity execution of native React methods?

Screenshots, screen recording, code snippet

YouTube screencast
https://youtu.be/4B7XiLaWiOM

Environment info

  • WordPress 6.6
  • Gutenberg plugin 18.8
  • Firefox 128
  • macOS Sonoma 14.4.1
  • My Custom Plugin

Tested with and without Gutenberg plugin active. Same result.

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@htmgarcia htmgarcia added the [Type] Bug An existing feature does not function as intended label Jul 24, 2024
@Mamaduka Mamaduka added Needs Testing Needs further testing to be confirmed. [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Jul 24, 2024
@talldan
Copy link
Contributor

talldan commented Jul 25, 2024

There's also a report in trac of this - https://core.trac.wordpress.org/ticket/61592.

Also noting some similarities with #63713.

@Mamaduka
Copy link
Member

Here's an example plugin that can be run via the DevTools console.

const { createElement: el, useEffect } = wp.element;
const { registerBlockType } = wp.blocks;
const { InnerBlocks, useBlockProps } = wp.blockEditor;

registerBlockType( 'test/block-infinity-and-beyond', {
    apiVersion: 3,
    title: 'Test: Infinity and Beyond',
    icon: 'pets',
    category: 'text',
    attributes: {
        id: {
            type: 'string',
        },
    },
    edit: function Edit( { clientId, setAttributes } ) {
        const blockProps = useBlockProps();

        // Effect dependencies are intentionally omitted.
        useEffect( () => {
            console.log( 'mounted', clientId );
            setAttributes( { id: clientId } );
            
            return () => {
                console.log( 'unmounted', clientId );
            };
        }, [] );

        return el(
            'div',
            { ...blockProps },
            el(
                'strong',
                null,
               'Infinity and Beyond'
            )
        );
    },
    save() {
        return null;
    },
} );

@Mamaduka Mamaduka removed the Needs Testing Needs further testing to be confirmed. label Jul 30, 2024
@talldan talldan removed their assignment Aug 14, 2024
@talldan talldan removed the [Status] In Progress Tracking issues with work in progress label Aug 14, 2024
@talldan
Copy link
Contributor

talldan commented Aug 14, 2024

The culprit of the infinite loop according to logs points to setAttributes() method inside componentDidMount(). If I add a check, the issue seems "solved", however this check is not needed when the block is inserted normally (not as a pattern), plus the logic of the block requires a unique value for blockID for every new instance, meaning the if() is messing with it, particularly if I duplicate the block. The value for the clones will be the same (non unique).

@htmgarcia I think you should at least have some code like this, particularly when setting attributes in useEffect or the older lifecycle methods like componentDidMount:

if (clientId !== attributes.blockID) {
    setAttributes({ blockID: clientId });
}

setAttributes doesn't optimize against setting to the current attribute value for you, as it's common to set multiple attributes in one call, so it's good to do this yourself. Without it your block will have unnecessary re-renders.

That said, the core issue within the pattern block is still something to fix in the gutenberg codebase.

@annezazu
Copy link
Contributor

Adding to the 6.6 board for consideration with a future point release and dropping into the 6.6 release leads channel for awareness.

@arthur791004
Copy link
Contributor

arthur791004 commented Aug 20, 2024

It seems to be related to the sync logic when you update the attribute programmatically 🤔 It will try to remove the existing block and insert it again, so it jumps into an infinite loop.

@Mamaduka
Copy link
Member

Mamaduka commented Aug 21, 2024

It looks like regression is caused by this line in the Pattern block:

value: innerBlocks.length > 0 ? innerBlocks : blocks,

There's no infinite loop if I point value to blocks. I requested addition context in the original PR - #60721 (comment).

@talldan
Copy link
Contributor

talldan commented Aug 28, 2024

There's no infinite loop if I point value to blocks.

@Mamaduka Yeah, this does seem to fix it, and I'm not seeing any issues with the functionality of the block.

The innerBlocks value is still needed for the setBlockEditMode code so that the correct client ids are used, but patterns and overrides still seem to work if blocks is used for the innerBlocksProps value.

It'd still be good to know if @ellatrix has any further feedback or reasons not to make that change.

If you want to push a PR with the change I'd be happy to test/review. It'd also be good to see if the e2e tests for patterns pass as there's pretty good coverage.

@Mamaduka
Copy link
Member

@talldan, I don't mind pushing a fix, but I would like to get more context around the original change first.

cc @SantosGuillamot

@SantosGuillamot
Copy link
Contributor

I just left a comment there: link. I'm fine changing that line and see if tests still pass, but I must say I am not sure about the implications.

@ellatrix
Copy link
Member

I am pretty sure I asked about this line somewhere, but I can't find it now. If I remember correctly, this also came from a different PR that was a proof of concept. In any case, I don't know anything more, I think you should just try it, see what tests fail and test a bit manually. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Bug An existing feature does not function as intended
Projects
Status: Done
7 participants