refactor: speed up deeplinks handling on cold start [GE-207]#29324
Conversation
|
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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
baptiste-marchand
force-pushed
the
refactor/speed-up-deeplinks
branch
from
d71b34a to
7041d23
Compare
baptiste-marchand
changed the title
baptiste-marchand
changed the title
samir-acle
left a comment
samir-acle
left a comment
There was a problem hiding this comment.
Works great, nice job! 👏
baptiste-marchand
dismissed stale reviews from vinnyhoward and samir-acle
via
b88da46
baptiste-marchand
force-pushed
the
refactor/speed-up-deeplinks
branch
from
ab4da31 to
b88da46
Compare
baptiste-marchand
temporarily deployed
to
build-exp
GitHub Actions
Inactive
| * has rendered. | ||
| */ | ||
| export function* parseDeeplinkAfterNavReady(deeplink: string, origin: string) { | ||
| const isReady: boolean = yield select(selectIsMainNavigatorReady); |
There was a problem hiding this comment.
We don't need to manage states to handle this trigger. Instead of yield select, we can use yield take(ACTION) so that it's triggered whenever the ACTION is dispatched.
| { whitelist: ['onboarding'] }, | ||
| ); | ||
|
|
||
| const persistNavigationTransform = createTransform< |
There was a problem hiding this comment.
No need to manage navigation state. Checkout the comment in the saga file
| // immediately; if in flight it awaits the shared promise. This guards the | ||
| // branches below that touch `SDKConnect.getInstance().state.*`. | ||
| try { | ||
| await SDKConnect.init({ context: 'deeplink' }); |
There was a problem hiding this comment.
Since SDKConnect is a service, it feels off to initialize it inside of this file, which is specific for handling deeplinks. From my understanding, SDKConnect initialization isn't dependent on deeplinks right?
baptiste-marchand
force-pushed
the
refactor/speed-up-deeplinks
branch
from
600da97 to
a45a1a7
Compare
|
Agent-Logs-Url: https://github.com/MetaMask/metamask-mobile/sessions/6f9642f3-94a4-47b7-8559-684ad3a01fb6 Co-authored-by: baptiste-marchand <75846779+baptiste-marchand@users.noreply.github.com>
…deeplink and navigation/saga tests (#30585) `ci` run `26232737527` failed in job `77197577872` on `yarn format:check` due to Prettier drift in three TypeScript files. This PR aligns those files with repository formatting rules only (no behavioral changes intended). - **Formatting alignment in deeplink utilities** - Reformatted `app/core/DeeplinkManager/util/deeplinks/index.ts` to match Prettier output for multiline call expressions. - **Formatting alignment in navigation reducer tests** - Reformatted `app/reducers/navigation/index.test.ts` for expected wrapping of nested `expect(...)` expressions. - **Formatting alignment in root saga tests** - Reformatted `app/store/sagas/sagas.test.ts` for chained invocation wrapping consistency. Example of the kind of change applied: ```ts // before expect(navigationReducer(initialNavigationState, mainNavigatorReady())).toBe( initialNavigationState, ); // after expect( navigationReducer(initialNavigationState, mainNavigatorReady()), ).toBe(initialNavigationState); ```
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 750a5a5. Configure here.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: Deep links are entry points into multiple critical user flows:
Because navigation layer (MainNavigator, Routes, navigation actions, sagas) is modified, regressions could impact screen transitions, parameter passing, and entry into transaction flows across the app. This is a high-risk, cross-cutting change affecting multiple verticals. Tags selected ensure coverage of:
Tags not selected (e.g., SmokeSnaps, SmokeSeedlessOnboarding, SmokeIdentity, SmokeAccounts, SmokeNetworkExpansion, SmokeMultiChainAPI) are not directly implicated by the changes, as no controller, snap, onboarding, sync, or multi-chain provider code was modified. Given the breadth of navigation and deeplink changes, broad smoke coverage is warranted. Performance Test Selection: |
|
baptiste-marchand
removed
the
INVALID-PR-TEMPLATE
metamaskbotv2
Bot
added
INVALID-PR-TEMPLATE
github-project-automation
Bot
moved this from Has approvals, needs CODEOWNER
to Review finalised - Ready to be merged
in PR review queue
metamaskbotv2
Bot
added
the
release-7.80.0
Description
This PR makes deeplink handling significantly faster, especially on cold start.
When an external deeplink (for example
https://link.metamask.io/buy) opened the app from a killed state, users would first land on the home screen and the deeplink could take 3-5 seconds before navigating to the intended screen.Root cause
The main issue was that SDKConnect / WalletConnect initialization blocked every post-login deeplink.
handleDeeplinkSagapreviously calledinitializeSDKServicesbefore handling the pending deeplink, serializingWC2Manager.initandSDKConnect.initin front of every deeplink. This added several seconds on cold start, even for deeplinks such as buy / swap / perps / rewards that do not need those services.Once that was fixed, it exposed a navigation readiness race. The root
NavigationContainercan be ready before the post-loginMainNavigatorhas mounted and registered screens such as Wallet, Ramp, Bridge, Perps, etc. The previous fixedsetTimeout(..., 200)was not reliable enough on cold start, sonavigate(...)could run while the target screen was not registered yet.Changes
Non-blocking SDK initialization
initializeSDKServicesnow initializesWC2ManagerandSDKConnectin parallel and is forked fromhandleDeeplinkSagainstead of blocking deeplink parsing.This means non-SDK deeplinks no longer wait for WC2 / SDKConnect startup. SDK-specific paths still remain safe because
handleMetaMaskDeeplinkawaits the idempotentSDKConnect.init({ context: 'deeplink' })when it actually needs SDKConnect.MainNavigator readiness gating
Post-login deeplinks now go through
parseDeeplinkAfterNavReady.Instead of relying on a fixed 200 ms timeout,
MainNavigatordispatches a newMAIN_NAVIGATOR_READYaction when it mounts. The deeplink saga waits for that signal before parsing post-login deeplinks, with a 3 s safety timeout to avoid silently dropping the deeplink ifMainNavigatornever mounts.This is intentionally different from
ON_NAVIGATION_READY:ON_NAVIGATION_READYonly means the root navigation container exists, whileMAIN_NAVIGATOR_READYmeans the post-login app screens are registered.New-user onboarding deeplinks are excluded from this wait because onboarding lives outside
MainNavigator; they keep the previous fast onboarding behavior.Fire-and-forget Branch.io params fetch
handleUniversalLinkno longer awaitsbranch.getLatestReferringParams()before routing. It now passes abranchParamsPromisethrough the analytics context, preserving the existing 500 ms timeout/error handling while allowing navigation to proceed immediately.Changelog
CHANGELOG entry: Improved deeplinks handling speed on cold-starts
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/GE-207
Manual testing steps
On Android (primary) and iOS, from a killed app state:
Screenshots/Recordings
Before
After
Pre-merge author checklist
Performance checks (if applicable)
trace()for usage andaddTokenfor an exampleFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist
Note
Medium Risk
Touches core deeplink saga ordering, navigation timing, and SDK/WC init concurrency; wrong gating could drop or mis-route links, though timeouts and SDK-only waits mitigate that.
Overview
Cold-start deeplinks no longer block on WalletConnect / SDKConnect startup for ordinary links. The deeplink saga forks parallel
WC2Manager+SDKConnectinit and only joins that work whenisSDKServiceDeeplinkmatches (WC, connect, mmsdk, etc.).Navigation race fix:
MainNavigatordispatchesMAIN_NAVIGATOR_READYon mount. Post-login parsing goes throughparseDeeplinkAfterNavReady, which waits for that action (or a 3s fallback) instead of a fixed 200mssetTimeout, so targets like Ramp/Wallet are registered beforenavigate. Onboarding/onboardingstays on the fast path outside this wait.Universal links: Branch.io params are fetched via
branchParamsPromise(500ms race) so routing/interstitials are not blocked; analytics resolves the promise later.handleMetaMaskDeeplinkis async with.catchlogging fromhandleUniversalLink. Shared helpersisMetaMaskSDKDeeplinkAction,isSDKServiceDeeplink, andisMetaMaskUniversalLinkcentralize host/action checks.Tests cover saga timing, nav-ready gating, parallel SDK init, Branch analytics, and reducer behavior for the new action.
Reviewed by Cursor Bugbot for commit fd1652e. Bugbot is set up for automated code reviews on this repo. Configure here.