Tabs: Fix infinite loop in useEffect #58861
Conversation
mirka
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| @@ -151,13 +151,7 @@ function Tabs( { | |||
| if ( tabsHavePopulated.current && !! selectedTabId && ! selectedTab ) { | |||
| setSelectedId( null ); | |||
| } | |||
| }, [ | |||
| isControlled, | |||
| selectedId, | |||
There was a problem hiding this comment.
selectedId is not referenced inside this hook, and nothing in the history suggests that it was an intentional addition to the dep array. I have to think it was just a mistake.
There was a problem hiding this comment.
Given that selectedTab is explicitly derived from selectedId, and selectedId isn't used inside the hook, the change works for me.
| @@ -151,13 +151,7 @@ function Tabs( { | |||
| if ( tabsHavePopulated.current && !! selectedTabId && ! selectedTab ) { | |||
| setSelectedId( null ); | |||
| } | |||
| }, [ | |||
| isControlled, | |||
| selectedId, | |||
There was a problem hiding this comment.
Given that selectedTab is explicitly derived from selectedId, and selectedId isn't used inside the hook, the change works for me.
What?
Removes an extraneous dependency in a Tabs
useLayoutEffectthat can cause an infinite loop crash.Why?
Fixes a bug that manifests in WP Core (first breaking commit), but not within the Gutenberg repo. (I have not yet looked into why.)
Testing Instructions
Set up
wordpress-developrepo and follow the instructions to set up a local dev environment. (Don't forget to read the extra steps for Apple Silicon if that applies to you.)packages/components/build-modulefrom thegutenbergdirectory to thenode_modules/@wordpress/components/build-moduleof thewordpress-developdirectory. (Something likersync --recursive ../gutenberg/packages/components/build-module/ node_modules/@wordpress/components/build-module.) I don't know why, but simply aliasing the@wordpress/componentsinpackage.jsonto the locally-built package file didn't work for me.npm run build:devinwordpress-developSteps
Go to the post editor and toggle the Settings sidebar on and off and on again. The editor should not crash. (Also, doing this same thing in the
gutenbergdev env should also not crash, as before.)