fix(curved-bottom-tabs): keep the hook order stable when tabs change - #49
Open
dennytosp wants to merge 1 commit into
Open
fix(curved-bottom-tabs): keep the hook order stable when tabs change#49dennytosp wants to merge 1 commit into
dennytosp wants to merge 1 commit into
Conversation
`CurvedBottomTabsCore` ran two hooks per tab from inside loop callbacks — `useSharedValue` via `useRef(tabs.map(...))` and `useAnimatedStyle` inside the render-body `tabs.map(...)`. The number of hooks the component ran was therefore driven by `tabs.length`, so adding or removing a route changed the hook order and React tore the tab bar down. Wrapping the shared values in `useRef` did not help: the argument is evaluated on every render, so `useSharedValue` still ran once per tab each time, while `.current` stayed pinned to the array built on the first render. Move the float animation into a `TabItem` child that owns its own shared value and animated style and reacts to an `isActive` prop, so each instance runs a fixed number of hooks and React can mount and unmount tabs normally. `animateToIndex` now only drives the background curve. The animation itself is unchanged — same spring config, same `-VIEWPORT_HEIGHT * 4.2` target for the active tab.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #48, same class of bug in a second component.
Problem
CurvedBottomTabsbreaks permanently as soon as the number of tabs changes — adding or removing a route, or rendering a different set of tabs per screen. React logs "React has detected a change in the order of Hooks" and the tab bar renders nothing from that point on.Cause
Two hooks ran once per tab, from inside loop callbacks:
The number of hooks the component ran was therefore
1 + 2 × tabs.length.The
useRefwrapper does not help, and in fact hides a second bug:useRef's argument is evaluated on every render, souseSharedValuestill ran once per tab each time — while.currentstayed pinned to the array built during the first render.Fix
Move the float animation into a
TabItemchild component that owns its own shared value and animated style and reacts to anisActiveprop. Each instance runs a fixed number of hooks, so React mounts and unmounts tabs normally.animateToIndexnow only drives the background curve.The animation is unchanged: same spring config, same
-VIEWPORT_HEIGHT * 4.2target for the active tab. No public API change.Verification
Rendered the real component before and after (native modules stubbed so the stubs map onto real React hooks), driving it through a tab switch and then two changes of tab count:
To confirm the refactor is behaviour-preserving, I also captured every shared value after a render with 3 tabs at
currentIndex: 2— the two versions are identical:tsc --noEmitreports the same error count asmain(none in this file).website/react-native/curved-bottom-tabs.tsxis regenerated viabun run sync:changed.