[CP Staging] Revert "Wait for TabNavigator mount before resolving waitForProtectedRoutes"#92046
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bb097d099
ℹ️ 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".
| // before any child router is registered to handle it. | ||
| return isTabNavigatorReady(state); | ||
| // If one protected screen is in the routeNames then other screens are there as well. | ||
| return state?.routeNames.includes(PROTECTED_SCREENS.CONCIERGE); |
There was a problem hiding this comment.
Keep waiting until TabNavigator mounts
When AuthScreens first mounts, routeNames already contains Concierge even though the nested TabNavigator route may still have no realized child state. Resolving waitForProtectedRoutes() at this point lets callers such as openReportFromDeepLink() dispatch Navigation.navigate(route) for report/workspace routes before the tab child router is registered, which React Navigation cannot handle and the deep link is effectively lost during sign-in/cold-start races. The readiness check needs to include the TabNavigator's mounted/non-stale nested state, not just a protected root screen name.
Useful? React with 👍 / 👎.
|
@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
…vigatorDeepLinkRace [CP Staging] Revert "Wait for TabNavigator mount before resolving waitForProtectedRoutes" (cherry picked from commit 1a666a5) (cherry-picked to staging by mountiny)
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @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! 🧪🧪
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.89-3 🚀
Bundle Size Analysis (Sentry): |
|
🤖 I reviewed the changes in this PR. No help site updates are required. This PR reverts #91796, removing internal navigation code in
There are no user-facing feature changes, no new/removed UI elements, no settings or workflow behavior changes, and nothing that would surface in customer-facing documentation under @yuwenmemon, please confirm — if you believe customer-facing behavior did shift as a result of this revert, let me know and I'll open a docs PR. |
|
Not emergency, straight revert |
Reverts #91796
Fixed Issues
$ #92023
$ #92030
$ #92041