Repository navigation
Conversation
React Navigation 7.2+ dispatches GO_BACK instead of RESET on browser back, bypassing OnboardingGuard.
|
@TMisiukiewicz @ZhenjaHorbach I'd like your opinion on this one. Context Since #103346 / #103699, browser back dispatches This PR is the minimal fix: Open question A few cases still differ from prod. Examples are onboarding that opens with more than one step, and MigratedUserWelcomeModal and SubmitPlanWelcomeModal, which now close on browser back. Fully matching prod would need a small patch to our I'd prefer not to patch a library we just bumped on purpose, so I kept the guard fix. What do you think: patch it, or keep this fix and handle the rest separately? |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b2c14e480
ℹ️ 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 isGoBack = action.type === CONST.NAVIGATION.ACTION_TYPE.GO_BACK; | ||
| if (!isGoBack && (action.type !== CONST.NAVIGATION_ACTIONS.RESET || !action?.payload)) { |
There was a problem hiding this comment.
Don't pass payload-less back actions to
findFocusedRoute
When browser back produces the payload-less action used by the new test ({type: 'GO_BACK'}), this condition now falls through to findFocusedRoute(action?.payload as NavigationState) below. The cast does not create a runtime state, and React Navigation's helper dereferences state.routes, so the guard throws before returning BLOCK or restoring browser history; the added test would fail for the same reason. Handle GO_BACK without attempting to derive a target route from its missing payload.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
findFocusedRoute handles an undefined state. It uses optional chaining throughout (current?.routes[...]), so it returns undefined and the guard returns BLOCK. Verified with CommonActions.goBack() ({type: 'GO_BACK'}) on a nested onboarding state, and the new test fails without the fix (ALLOW).
Yeah |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppNA Android: mWeb ChromeScreen.Recording.2026-10-13.at.11.58.49.moviOS: HybridAppNA iOS: mWeb SafariScreen.Recording.2026-10-13.at.11.58.49.movMacOS: Chrome / SafariScreen.Recording.2026-10-13.at.11.53.35.movScreen.Recording.2026-10-13.at.11.54.18.mov |
|
TS issue is not related to this PR |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.5.9-0 🚀
|
|
No help site update is needed. This PR fixes a regression so that the browser back button can't skip onboarding again, and no help article describes that behavior. |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.5.9-4 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Since #103346, browser back on onboarding dispatches
GO_BACKinstead ofRESET, soOnboardingGuardno longer blocked it. The guard now blocksGO_BACKtoo.Fixed Issues
$ #103839
PROPOSAL: N/A
Tests
Offline tests
Same as tests.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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.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