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

useNoRecursiveRenders can cause all of its children to be unmounted from the DOM #39044

Closed
talldan opened this issue Feb 24, 2022 · 0 comments · Fixed by #42916
Closed

useNoRecursiveRenders can cause all of its children to be unmounted from the DOM #39044

talldan opened this issue Feb 24, 2022 · 0 comments · Fixed by #42916
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended

Comments

@talldan
Copy link
Contributor

talldan commented Feb 24, 2022

Description

Discovered when debugging #38169.

As the title says, the RecursionProvider returned by useNoRecursiveRenders can cause all of its children to unmount and remount from the DOM, which can lead to issues like focus loss, and may also be bad for performance.

The explanation for this is a little lengthy, so buckle in.

The first important thing to know if that when using function components, React uses the function reference during render to reconcile the DOM. If that function reference changes, React considers it to be a completely different component, so when rendering it unmounts it and remounts what it considers the new component. It makes sense because the function reference is the only way React knows that a component is the same one across two renders.

Looking at the code for useNoRecursiveRenders, we can see that it uses useCallback to define a React component:

const Provider = useCallback(
( { children } ) => (
<RenderedRefsContext.Provider value={ newRenderedBlocks }>
{ children }
</RenderedRefsContext.Provider>
),
[ newRenderedBlocks ]
);

When newRenderedBlocks changes, the function reference for the Provider also changes, which is the root cause of the issue. In the navigation block, this happens when selecting a new menu—almost the entire block will be unmounted and remounted.

Tracing back a little, the unique id for the currently selected menu is passed to useNoRecursiveRenders which results in newRenderedBlocks being recalculated:

const newRenderedBlocks = useMemo(
() => addToBlockType( previouslyRenderedBlocks, blockName, uniqueId ),
[ previouslyRenderedBlocks, blockName, uniqueId ]
);
and causes a new component reference for the Provider.

Step-by-step reproduction instructions

The easiest way to test this is in the Navigation Block.

  1. Add a nav block and select an initial menu
  2. Using the keyboard, change to a different menu using 'Select menu' option in the toolbar
  3. Observe there's a focus loss

The only thing to be aware of is that there are multiple causes of this bug in the Navigation Block, not just the recursion provider.

Screenshots, screen recording, code snippet

No response

Environment info

No response

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended
Projects
None yet
2 participants