Upgrade react-navigation to latest#85743
Conversation
|
|
e8614ab to
093c38c
Compare
|
|
||
| function SearchContextProvider({children}: SearchContextProps) { | ||
| const focusedScreen = useNavigationState((state) => getDeepestFocusedScreen(state)); | ||
| const focusedScreen = useRootNavigationState((state) => getDeepestFocusedScreen(state)); |
There was a problem hiding this comment.
Isn't this going to trigger rerender every focused screen change?
There was a problem hiding this comment.
We may want to use much stronger selector for this one e.g. return undefined for screens other thant search root. You have something like that in the code below
| function RequireTwoFactorAuthenticationOverlay() { | ||
| const shouldShowRequire2FAPage = useShouldShowRequire2FAPage(); | ||
| const isIn2FASetupFlow = useNavigationState((state) => { | ||
| const isIn2FASetupFlow = useRootNavigationState((state) => { |
There was a problem hiding this comment.
This one looks safe 👍
2c50533 to
a3c9875
Compare
|
@aimane-chnaif Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3c9875f82
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| isExecutingRef.current = true; | ||
| navigation.goBack(); | ||
| setTimeout(() => { | ||
| isExecutingRef.current = false; | ||
| }, CONST.ANIMATED_TRANSITION); | ||
| const currentState = navigationRef.getRootState(); | ||
|
|
||
| // There is a brief moment when the RHP is not in the state anymore but the overlay is still visible (closing RHP animation) | ||
| // We need to block overlay press function in such case because it would go back from the currently active full screen. | ||
| if (currentState.routes.at(-1)?.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR) { |
There was a problem hiding this comment.
Reset overlay debounce flag when no back navigation occurs
handleOverlayPress now sets isExecutingRef.current = true before checking whether the root route is RIGHT_MODAL_NAVIGATOR, but the flag is only cleared inside that if block. In flows where the overlay press happens while another root route is on top (the exact race this change is guarding against), the function exits with the flag still true, so subsequent backdrop presses in the same mounted RHP instance are ignored and the overlay can appear unresponsive. Please clear the flag in the non-matching branch as well (or use a finally-style reset path).
Useful? React with 👍 / 👎.
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
🔍 PR ReviewSummary: Upgrades CI StatusAll checks pass (ESLint, typecheck, all 8 test shards, perf-tests, validate-patches, etc.). Source Code Changes
PatchesAll existing patches properly re-targeted to new versions. The TestsTest mocks updated to match the new VerdictThe code changes look correct and well-motivated. The |
|
Bug: After following steps in #78456, it lands on Search page. Tested in android adhoc build. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3c9875f82
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const params = useRootNavigationState((state) => { | ||
| const focusedScreen = getDeepestFocusedScreen(state); | ||
| if (focusedScreen?.name !== SCREENS.SEARCH.ROOT) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Read the initial search route before defaulting to empty query
useRootNavigationState initializes with selector(undefined) and only updates on later root state events, so on a cold load or deep link directly into Search the first value here is undefined and there is no initial ready-state update to correct it. That makes definedQueryParam fall back to buildSearchQueryString(), and useSearchPageSetup() then fires search() for the default hash instead of the requested q/rawQuery. In practice, refreshing or opening a saved-search URL will show the wrong search until some later navigation change happens.
Useful? React with 👍 / 👎.
| const isIn2FASetupFlow = useRootNavigationState((state) => { | ||
| const focusedScreen = getDeepestFocusedScreen(state); | ||
| return isTwoFactorSetupScreen(focusedScreen?.name); |
There was a problem hiding this comment.
Read the initial 2FA route before showing the blocking overlay
This has the same initial-state problem: when the app restores directly into a 2FA setup screen, useRootNavigationState returns undefined until some later navigation event, so isIn2FASetupFlow stays false on the initial load. Because useShouldShowRequire2FAPage() is still true during setup, the RequireTwoFactorAuthenticationOverlay renders on top of SETTINGS_2FA_ROOT/SETTINGS_2FA_VERIFY_ACCOUNT and can block the setup flow until the user triggers another navigation change.
Useful? React with 👍 / 👎.
…ted navigation changes
a3c9875 to
0236ffe
Compare
I've investigated it and I know what's causing this issue, now I want to check if this issue occurs on main as well. If so, I'll raise a separate PR to fix it! |
…y when root state is undefined
7ddfa70 to
cf26342
Compare
Explanation of Change
This PR upgrades the
react-navigationpackages to the latest versions, which were previously reverted from staging (#78443) due to several deploy blockers caused by focus and navigation regressions. Those regressions have now been identified and fixed, allowing the upgrade to land cleanly.Package version changes:
@react-navigation/core7.16.1@react-navigation/native7.1.107.1.33@react-navigation/native-stack7.3.147.14.5@react-navigation/stack7.3.37.8.5@react-navigation/material-top-tabs7.2.137.4.19@react-navigation/devtools^6.0.107.0.52Why
useNavigationStatewas replaced withuseRootNavigationStatein 2 filesIn newer versions of
react-navigation,useNavigationStatereads the state from the nearest navigator in the component tree rather than the root navigator. BothSearchContextProvider(inSearchContext.tsx) andRequireTwoFactorAuthenticationOverlayare rendered outside of any navigator, which meansuseNavigationStateno longer has access to a navigation context and throws an error or returns stale/incorrect state.useRootNavigationStateis a custom hook that subscribes directly tonavigationRef(the root navigator ref), bypassing the context hierarchy entirely. This makes it safe to call from any component regardless of where it sits in the tree, and ensures both hooks always receive the full, up-to-date root navigation state.Fixed Issues
$ #75700
PROPOSAL:
Tests
Test steps
new.expensify.com. Verify the email field is auto-focused and no validation error appears underneath it on the sign-in page.Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-03-19.at.15.04.50.mov