[LWM] fix(mobile): reset navigation for token deeplinks#19807
Open
sarneijim wants to merge 2 commits into
Open
[LWM] fix(mobile): reset navigation for token deeplinks#19807sarneijim wants to merge 2 commits into
sarneijim wants to merge 2 commits into
Conversation
sarneijim
marked this pull request as ready for review
July 20, 2026 21:58
live-github-bot
Bot
changed the title
Contributor
Web Tools Build Status
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR mitigates an iOS crash scenario in Ledger Wallet Mobile where sequential token Asset Detail deeplinks can reuse an existing route, by forcing token deeplink navigation to dispatch a reset action (remounting the screen) while leaving other deeplinks’ navigation behavior unchanged.
Changes:
- Overrides React Navigation linking’s
getActionFromStateto reset navigation only for token Asset Detail deeplink states. - Adds a feature-scoped MVVM utility to detect token Asset Detail deeplink states and generate the appropriate action.
- Adds Jest coverage to validate reset behavior for token deeplinks and default behavior for coin + stake deeplinks.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/ledger-live-mobile/src/navigation/DeeplinksProvider.tsx | Hooks the custom getActionFromState into the app’s deeplink linking config. |
| apps/ledger-live-mobile/src/mvvm/features/AssetDetail/utils/getActionFromAssetDetailDeeplinkState.ts | Implements token-AssetDetail state detection and emits a reset action for those deeplinks. |
| apps/ledger-live-mobile/src/mvvm/features/AssetDetail/utils/tests/getActionFromAssetDetailDeeplinkState.test.ts | Adds unit tests asserting RESET for token Asset Detail states and NAVIGATE for coin/stake cases. |
| .changeset/calm-tokens-reset.md | Declares the mobile release note/version bump for the navigation behavior change. |
Contributor
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 1 project with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 mobilePath:
📦 Download Diff Report: mobile Bundle Diff Generated by Rsdoctor GitHub Action |
Contributor
|
sarneijim
force-pushed
the
bugfix/fix-token-deeplink-navigation
branch
from
July 20, 2026 22:44
3e6f8d2 to
7d6d2bd
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
apps/ledger-live-mobile/src/mvvm/features/AssetDetail/utils/getActionFromAssetDetailDeeplinkState.ts:17
hasMarketStateParamnarrowsparamstoobject, but then readsparams.marketStatedirectly. This defeats the purpose of takingunknown(and can fail type-checking under strict settings). It’s safer to cast to a dictionary type and usehasOwnPropertyto avoid matching prototype properties.
function hasMarketStateParam(params: unknown): params is { marketState: unknown } {
if (typeof params !== "object" || params === null) {
return false;
}
return "marketState" in params && params.marketState !== undefined;
}
LucasWerey
previously approved these changes
Jul 21, 2026
Comment on lines
1
to
5
| import React, { useEffect, useLayoutEffect } from "react"; | ||
| import type { NativeStackHeaderRightProps } from "@react-navigation/native-stack"; | ||
| import { useNavigation } from "@react-navigation/native"; | ||
| import type { CompositeNavigationProp } from "@react-navigation/native"; | ||
| import { NativeStackNavigationProp } from "@react-navigation/native-stack"; |
LucasWerey
reviewed
Jul 22, 2026
| const Stack = createNativeStackNavigator<BaseNavigatorStackParamList, typeof BASE_NAVIGATOR_ID>(); | ||
| const MARKET_LIST_TEST_ID = "market-list-screen"; | ||
|
|
||
| function MarketListScreen() { |
Member
There was a problem hiding this comment.
I think we should use a real existing screen rather than creating a non-existent one so we can test the real behavior
| const { currency, coinOptions, shouldRedirectToMarket } = viewModel; | ||
| const navigation = useNavigation<NavigationProps>(); | ||
|
|
||
| useEffect(() => { |
Member
There was a problem hiding this comment.
You should do it in the viewModel file
Member
|
Can you add a demo video please |
Reset nested asset-detail navigation state for token deeplinks. Redirect unresolved token assets to Market while preserving loading and error states. Co-authored-by: Cursor <cursoragent@cursor.com>
sarneijim
force-pushed
the
bugfix/fix-token-deeplink-navigation
branch
from
July 22, 2026 10:11
639db7a to
de69968
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✅ Checklist
npx changesetwas attached.earn?action=stakedeeplinks keep their existing navigation behavior.📝 Description
Opening a token Asset Detail deeplink followed by another token deeplink can crash Ledger Wallet Mobile on iOS. The available crash report shows a secondary React Native/Hermes failure while converting a native exception, but it does not expose the original native module, method, or exception reason.
Before: Token Asset Detail deeplinks used React Navigation's default action generation, allowing the existing Asset Detail route to be reused for the next token.
After: Token Asset Detail states are dispatched as reset actions so every token deeplink creates a fresh navigation state and remounts the screen. Coin Asset Detail, Stake, and all other deeplinks continue using React Navigation's default action generation. This is independent of how quickly the links are opened.
This is a targeted mitigation for route reuse. Confirming the native root cause still requires reproducing the original exception on the affected physical iPhone/build.
Automatic validation:
develop, primarily React 18/19 type conflicts and locally unbuilt domain package exports. No typecheck error is reported in the new utility or test.🎥 Demo
Non-existent token deeplink:
ledgerlive://market/ethereum/erc20/does_not_existpr-19807-nonexistent-token-deeplink.mp4
❓ Context
🧐 Checklist for the PR Reviewers