Skip to content

fix: Wrap navigation proxy using requestAnimationFrame#25241

Merged
Cal-L merged 10 commits intomainfrom
fix/25240-fix-navigation-race-condition
Jan 28, 2026
Merged

fix: Wrap navigation proxy using requestAnimationFrame#25241
Cal-L merged 10 commits intomainfrom
fix/25240-fix-navigation-race-condition

Conversation

@Cal-L
Copy link
Contributor

@Cal-L Cal-L commented Jan 27, 2026

Description

The change wraps both navigate and reset methods from navigation with requestAnimationFrame to ensure that it respects React's render cycles. This ensures that the navigation ref is being invoked with the latest context, which fixes race conditions associated with the navigation system not responding.

Changelog

CHANGELOG entry:

Related issues

Fixes: #25240

Manual testing steps

Expected behavior

  • Immediate lock and biometrics should be enabled
  • Navigate to any screen
  • Background and foreground app
  • Get prompted biometrics and fail it
  • Navigates to the login screen

Screenshots/Recordings

Before

fail.mp4

After

fix.mp4

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Changes how NavigationService.navigation behaves by returning a Proxy that defers navigate/reset, which can affect timing/order of navigation actions and introduces reliance on requestAnimationFrame. Tests were expanded to cover deferred vs pass-through behavior, reducing regressions but still touching app-wide navigation flow.

Overview
NavigationService now wraps the provided navigation ref in a Proxy that defers navigate and reset calls to the next frame via requestAnimationFrame, aiming to avoid React render-cycle timing issues.

Adds resetForTesting() to clear the stored navigation ref, and expands the unit tests to validate deferred behavior for navigate/reset plus pass-through/binding for other methods and properties.

Written by Cursor Bugbot for commit f3eecc8. This will update automatically on new commits. Configure here.

@Cal-L Cal-L requested a review from a team as a code owner January 27, 2026 03:23
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-mobile-platform Mobile Platform team label Jan 27, 2026
@Cal-L Cal-L added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jan 27, 2026
@github-project-automation github-project-automation bot moved this to Needs dev review in PR review queue Jan 27, 2026
@Cal-L Cal-L added No QA Needed Apply this label when your PR does not need any QA effort. no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed no changelog required No changelog entry is required for this change labels Jan 27, 2026
weitingsun
weitingsun previously approved these changes Jan 27, 2026
@github-project-automation github-project-automation bot moved this from Needs dev review to Review finalised - Ready to be merged in PR review queue Jan 27, 2026
tommasini
tommasini previously approved these changes Jan 27, 2026
@Cal-L Cal-L dismissed stale reviews from tommasini and weitingsun via dd09c62 January 27, 2026 09:31
@Cal-L Cal-L enabled auto-merge January 27, 2026 09:31
weitingsun
weitingsun previously approved these changes Jan 27, 2026
weitingsun
weitingsun previously approved these changes Jan 27, 2026
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 4 potential issues.

@github-actions
Copy link
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmationsRedesigned, SmokeIdentity, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeTrade, SmokeWalletPlatform, SmokeCard, SmokeRewards, SmokePerps, SmokeRamps, SmokePredictions, FlaskBuildTests
  • Selected Performance tags: @PerformanceLaunch, @PerformanceLogin, @PerformanceOnboarding
  • Risk Level: high
  • AI Confidence: 90%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR modifies the core NavigationService, which is a critical infrastructure component used throughout the entire MetaMask Mobile app. The change introduces a significant behavioral modification: wrapping navigate and reset methods with requestAnimationFrame to defer navigation calls to the next frame.

Impact Analysis:
The NavigationService is imported and used by 30+ files across the codebase, including:

  • Authentication (app/core/Authentication/Authentication.ts) - Uses navigation.reset() for login/logout flows, vault recovery
  • EngineService (app/core/EngineService/EngineService.ts) - Uses navigation.reset() for vault recovery during startup
  • Redux Sagas (app/store/sagas/index.ts) - Uses both navigate and reset for state-driven navigation
  • SDKConnect/SDKConnectV2 - dApp connection flows
  • WalletConnect - dApp connection flows
  • Deeplink handlers - handlePerpsUrl, handlePredictUrl, handleSwapUrl, handleBrowserUrl, handleRampUrl, handleDepositUrl, handleHomeUrl, handleTrendingUrl, handleApproveUrl
  • Network Selection (useNetworkSelection hook) - Network switching modals
  • Perps components - PerpsRedirect, PerpsTutorialCarousel
  • Ramps components - handleDepositUrl, handleRampUrl, handleRedirection

Why this is high risk:

  1. The requestAnimationFrame deferral changes the timing of ALL navigation calls throughout the app
  2. This could affect race conditions, navigation state consistency, and user experience
  3. Critical flows like authentication, vault recovery, and deeplink handling depend on this service
  4. The change affects both navigate (50+ usages found) and reset (used in critical auth/engine flows)

Recommended testing:
All E2E test tags should run to verify that navigation works correctly across all major user flows, including accounts, confirmations, identity sync, network management, trading, wallet platform features, card, rewards, perps, ramps, predictions, and Snaps functionality.

Performance Test Selection:
The NavigationService change introduces requestAnimationFrame deferral for navigate and reset methods, which directly affects navigation timing. This could impact: 1) App launch performance - EngineService uses navigation.reset() during startup/vault recovery, 2) Login performance - Authentication uses navigation.reset() for login/logout flows, 3) Onboarding performance - Navigation timing during initial setup. The deferral adds a frame delay to all navigation calls, which could affect perceived responsiveness during critical user flows like app startup, login, and onboarding.

View GitHub Actions results

@sonarqubecloud
Copy link

@Cal-L Cal-L added this pull request to the merge queue Jan 28, 2026
Merged via the queue into main with commit 7b7bbb8 Jan 28, 2026
97 checks passed
@Cal-L Cal-L deleted the fix/25240-fix-navigation-race-condition branch January 28, 2026 18:49
@github-project-automation github-project-automation bot moved this from Review finalised - Ready to be merged to Merged, Closed or Archived in PR review queue Jan 28, 2026
@github-actions github-actions bot locked and limited conversation to collaborators Jan 28, 2026
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jan 28, 2026
@metamaskbot metamaskbot added release-7.65.0 Issue or pull request that will be included in release 7.65.0 release-7.64.0 Issue or pull request that will be included in release 7.64.0 and removed release-7.65.0 Issue or pull request that will be included in release 7.65.0 labels Jan 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no changelog required No changelog entry is required for this change No QA Needed Apply this label when your PR does not need any QA effort. no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-7.64.0 Issue or pull request that will be included in release 7.64.0 size-M team-mobile-platform Mobile Platform team

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Stuck on lock screen when unlocking wallet

4 participants