Repository navigation
Conversation
Review polish, no behaviour change: collapse multi-line comments to one line, drop narration, and fix comma splices and dash usage to match the plain comment style. Corrects a test comment that named history.go when the mocked symbol is window.history.go.
|
|
|
🚧 dylanexpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
…react-navigation # Conflicts: # package-lock.json # package.json # patches/react-navigation/@react-navigation+bottom-tabs+7.15.5+001+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/@react-navigation+bottom-tabs+7.18.16+001+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/@react-navigation+bottom-tabs+7.19.2+001+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/@react-navigation+elements+2.9.14+001+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/@react-navigation+elements+2.9.38+001+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/@react-navigation+elements+2.9.43+001+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/@react-navigation+native+7.1.33+001+initial.patch # patches/react-navigation/@react-navigation+native+7.1.33+002+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/@react-navigation+native+7.3.16+001+initial.patch # patches/react-navigation/@react-navigation+native+7.3.16+002+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/@react-navigation+native+7.3.16+003+increase-history-go-popstate-fallback-timeout.patch # patches/react-navigation/@react-navigation+native+7.4.1+001+initial.patch # patches/react-navigation/@react-navigation+native+7.4.1+002+fix-failing-jest-by-disabling-esmodule.patch # patches/react-navigation/details.md # tests/unit/Navigation/createMemoryHistoryTest.ts
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-10-12.at.4.29.19.PM.movWhatsApp.Video.2026-10-12.at.16.40.28.mp4 |
|
@TMisiukiewicz Can you please ready this PR for review? |
|
@shubham1206agra 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] |
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: 47d9481fa6
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Guard the stale history diff navigation with a try/catch to swallow interrupted navigations - Subtract pendingPopStateDelta from historyDeltaByKeys to keep history deltas in sync - Remove now-unneeded no-deprecated eslint-disable for NavigationContent and seatbelt the lint error instead
mountiny
left a comment
There was a problem hiding this comment.
Thanks for taking over and pushing it ahead. Looks good to me.
|
🚧 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! 🧪🧪
|
|
I think we have a regression from this PR. Can anyone check if it is? Mac-Chrome.mp4 |
|
Nevermind. The offending PR is #103346 |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.5.8-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.5.8-5 🚀
Bundle Size Analysis (Sentry): |
Supersedes #102146 (moved to a branch I can push to).
Explanation of Change
Bump the react-navigation family to the newest releases this repo's
min-release-agepolicy allows, re-port the vendored patch set onto the new versions, and drop two patches upstream now covers. No app navigation code was rewritten.@react-navigation/native@react-navigation/core@react-navigation/stack@react-navigation/native-stack@react-navigation/bottom-tabs@react-navigation/material-top-tabs@react-navigation/devtools(dev)@react-navigation/elements(transitive)@react-navigation/routers(transitive)react-native-tab-view.npmrcsetsmin-release-age=7, so these are the newest releases at least seven days old. A newer batch published 2026-09-29 (native7.5.0,core7.23.0,stack7.12.0) becomes installable around 2026-10-06 and is left for a follow-up.Patches. All 13 react-navigation and 5
react-native-tab-viewpatch files were regenerated against the new versions. Per-patch reasons and re-port notes are inpatches/react-navigation/details.mdandpatches/react-native-tab-view/details.md. Summary:native+001+initialneeded hand re-anchoring because upstream rewroteuseLinkingandcreateMemoryHistory.native+003+increase-history-go-popstate-fallback-timeout. Upstream shipped the same 100ms to 1000ms raise in 7.3.18 (#13217).stack+005+clip-inactive-card-inside-cardfrom feat: use floating RHP style on web #101093. Upstream shipped it in 7.11.2 (#13255). Verified the floating RHP card shadow still renders.core+003+root-state-not-undefined. Types only. Upstream narrowedgetRootState()toNavigationState | undefinedwhile the runtime has always returnedundefinedbefore the container mounts. Thesrc/call sites assume a state exists and all run behindnavigationRef.isReady(), so guarding each one is a separate refactor.One behaviour change. Since 7.3.8,
createMemoryHistory.go()rejects withHistory was changed during navigation.when the browser never lands on the target entry, where 7.1.33 resolved.useLinkingalready catches it, and the difference is that the trailinghistory.replaceis skipped. This is the code path #94571 (Firefox back button after a redirect) lives in, and it was verified on Firefox.App changes. Two comments in
src/libs/Navigation/PlatformStackNavigation/createPlatformStackNavigatorComponent/(index.tsx,index.native.tsx). Core 7.22.0 deprecatedNavigationContentin favour of a render callback (#13238) and@typescript-eslint/no-deprecatedis enabled here. Migrating that builder to therendercallback sits on the render path of every stack in the app and belongs in its own PR.Lockfile.
standard-navigation@0.0.8appears, pulled in bynative. It is types only and is not bundled by Metro or webpack.coredropsquery-string, which also removesdecode-uri-componentandfilter-obj. Nothing insrc/imported them.Fixed Issues
$ #101715
PROPOSAL:
Tests
Automated, on this branch after merging main:
npm run typecheck, after deleting the*.tsbuildinfocaches: exit 0.bash scripts/validatePatches.sh: exit 0.rm -rf node_modules/@react-navigation node_modules/react-native-tab-view,npm install --ignore-scripts,bun scripts/applyPatches.ts): patch-package succeeded with no errors or warnings.npx jest tests/navigation tests/unit/Navigation: 96 suites, 1320 tests, all passing.npx jest tests/unit/Navigation/createMemoryHistoryTest.ts: 2 passing. Updated in this PR to assert the new rejection contract.npx eslint --report-unused-disable-directiveson the changed TypeScript files andnpm run spell-changed: clean.Manual, each run against this branch's own bundle:
history.go(-3)and rapid consecutive traversals, and reload a deep link. No console errors beyond pre-existing dev environment noise.stack+005.go()rejection change above and [Due for payment 2026-07-29] [$250] Chat - The new group chat doesn't open in Firefox #94571 live.Offline tests
Same as tests. Nothing in this diff changes offline behaviour or persists navigation state differently.
QA Steps
Same as tests 7 through 10, on staging builds for Android mWeb Chrome, iOS mWeb Safari and iOS/Android native. The change is in the JS navigation layer, so coverage is transitions and web history rather than data.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
🤖 Generated with Claude Code