Infinite loop when registered plugin locks post saving

[dlh01]

Description

While testing the WordPress 6.3 release candidates, I encountered an infinite loop in the post editor that began in a component that was registered via registerPlugin().

I think the loop begins with a call to lockPostSaving(), although I don't know why it happens after that call. The loop begins almost immediately and prevents the editor from rendering.

The component is in use now on sites running WordPress 6.2, so I believe that whatever has caused this change is new to 6.3.

Step-by-step reproduction instructions

Activating the test theme mentioned below and attempting to load the post editor should demonstrate the issue.

Screenshots, screen recording, code snippet

I've created a test theme that can be activated to replicate the infinite loop: test-theme.zip

I've also posted the theme files in a Gist: https://gist.github.com/dlh01/c99a48d888ca77a0f98a9209da37532d

The relevant portion of the component:

  const postTitle = useSelect((select) => select('core/editor').getEditedPostAttribute('title'));
  const titleIsOk = 0 < postTitle.length;

  const { lockPostSaving, unlockPostSaving } = useDispatch('core/editor');
  const { createWarningNotice, removeNotice } = useDispatch('core/notices');

  if (! titleIsOk) {
    lockPostSaving('title-empty-lock'); // Here
    createWarningNotice(
      __('Please enter a headline before publishing.', 'my-textdomain'),
      { id: 'title-empty-lock', isDismissible: false },
    );
  } else {
    unlockPostSaving('title-empty-lock');
    removeNotice('title-empty-lock');
  }

Environment info

• WordPress 6.3-RC3
• Chrome 114
• Gutenberg plugin is not active

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

[Mamaduka]

Hi, @dlh01

Have you tried moving the logic inside the useEffect? You shouldn't call store actions during the renders. Instead, your component should "react" to store value changes - hasTitle.

Adjusted example

const {
  i18n: { __ },
  plugins: {
    registerPlugin,
  },
  element: {
      useEffect,
  },
  data: {
    useSelect,
    useDispatch,
  },
} = wp;

function TitleChecker() {
  const hasTitle = useSelect((select) => select('core/editor').getEditedPostAttribute('title').length > 0, []);

  const { lockPostSaving, unlockPostSaving } = useDispatch('core/editor');
  const { createWarningNotice, removeNotice } = useDispatch('core/notices');

  useEffect(() => {
      if ( ! hasTitle ) {
          lockPostSaving('title-empty-lock');
          createWarningNotice(
              __('Please enter a headline before publishing.', 'my-textdomain'),
              { id: 'title-empty-lock', isDismissible: false },
            );
      }
      
      return () => {
        unlockPostSaving('title-empty-lock');
        removeNotice('title-empty-lock');
      };
  }, [hasTitle]);

  // On the site where this issue occurred, a <PluginPrePublishPanel> was rendered here.
  return null;
}

registerPlugin('empty-title-check', { render: TitleChecker });

[dlh01]

Thanks @Mamaduka! My intention at the moment is only to raise to the editor team that an integration that works in 6.2 causes a crash in 6.3. If whatever caused this change on the editor side isn't considered a regression, then we can close this issue.

[Mamaduka]

Thanks for the clarification, @dlh01.

I think recent changes highlighted the issue with the integration code, like in the example. Trying to bring back the "old behavior" will just hide that fact, and the code might break during a different release.

Cc @jsnajdr, @tyxla, what do you think?

P.S. I can see the same error message when using the code with WP 6.2.2.

Error message from orignal code

[tyxla]

@dlh01 I've checked the code and I believe @Mamaduka is right. In short, what @Mamaduka suggested should resove the bug.

With the original version of the code (without wrapping the dispatch calls in useEffect()) they will be called on every re-render. Then every re-render issues another call of dispatch, and that predictably creates an infinite loop. I'd be surprised if this works well in earlier version, and if it did - it was not supposed to 😉

Even if you use plain redux, you'll be able to reproduce this issue - dispatching an action directly at render time will trigger a new render, and therefore a new dispatch, and thus an infinite loop.

That's why we usually dispatch actions when handling an event triggered by user action (click, hover, etc.), but in this case, you want to render it immediately. For those cases, you need to use useEffect() and specify dependencies - then only when any of them changes, the effect will be triggered. That means, that if titleIsOk (or hasTitle in @Mamaduka's code) doesn't change, the effect won't be rerun, a re-render won't happen, and an infinite loop won't happen.

[dlh01]

Alright. If it wasn't supposed to work, we'll fix it now. Thanks for taking a look!

[jsnajdr]

Yes, the error is caused by missing useEffect, namely by the lockPostSaving dispatch directy during render. The React error:

Cannot update a component (`Unknown`) while rendering a different component (`TitleChecker`).
To locate the bad setState() call inside `TitleChecker`, follow the stack trace

complains about exactly that. While rendering TitleChecker, something was called (the lockPostSaving) that in the end caused setState call inside another component. And that's bad.

The error started to be reported happening most likely after upgrade to React 18, after switch to its concurrent mode, and re-implementation of useSelect that accompanied that. We probably couldn't return to the old behavior even if we wanted, we'd have to downgrade back to previous React. The "dispatch without effect" code was always buggy, and the new React version started actually failing on it.