-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Implement LegacyLinkAdapter between old and new 'core' links #22202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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. |
| const actionParams: Record<string, string> = {}; | ||
| if (actionPath.includes('?')) { | ||
| const [basePath, queryString] = actionPath.split('?'); | ||
| actionPath = basePath; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a false positive. Anyone using multiple ? in a URL shall get no consideration 😁 follow standards or get left behind
| [ACTIONS.ONBOARDING]: 'onboardingPath', | ||
| [ACTIONS.CREATE_ACCOUNT]: 'createAccountPath', | ||
| [ACTIONS.DEPOSIT]: 'depositCashPath', | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incomplete action path mapping for PERPS_MARKETS
The actionPaths mapping is missing an entry for ACTIONS.PERPS_MARKETS. The CoreLinkParams interface includes a perpsMarketsPath field, and toProtocolUrl (line 276) deletes this field from query params, indicating it's a valid action-specific path. However, extractActionParams cannot properly extract the path for PERPS_MARKETS actions because the mapping is incomplete. The fix should add [ACTIONS.PERPS_MARKETS]: 'perpsMarketsPath' to the actionPaths object.
| [ACTIONS.ONBOARDING]: 'onboardingPath', | ||
| [ACTIONS.CREATE_ACCOUNT]: 'createAccountPath', | ||
| [ACTIONS.DEPOSIT]: 'depositCashPath', | ||
| [ACTIONS.PERPS_MARKETS]: 'perpsMarketsPath', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Action Path Mapping: A Dead End
The ACTIONS.PERPS_MARKETS entry in ACTION_PATH_MAP is unreachable dead code. Since PERPS_MARKETS is included in the PERPS_ACTIONS array, the getActionSpecificParams method will always match it in the PERPS_ACTIONS.includes(action) branch and set perpsPath instead of perpsMarketsPath. The ACTION_PATH_MAP lookup only occurs in the else branch, which PERPS_MARKETS never reaches.
|



Part 2 of 4 to fix:
https://github.com/MetaMask/mobile-planning/issues/2343
https://consensyssoftware.atlassian.net/jira/polaris/projects/MWMR/ideas/view/8845676?selectedIssue=MWMR-10
Previous Part: #21869
Description
This PR adds
LegacyLinkAdapter, a bidirectional adapter that bridges the newCoreUniversalLinkformat (from PR #XXXX) with the existing legacy deep link handlers. This is the second PR in Phase 1 of the deep link consolidation project.Context
The current deep link system has routing logic fragmented across multiple files with nested switch-case statements. This adapter enables gradual migration to a unified routing system while maintaining 100% backward compatibility with existing handlers.
What Changed
Created
LegacyLinkAdapterwith bidirectional conversion methods:toLegacyFormat()- ConvertsCoreUniversalLinkto legacyurlObj+paramsformatfromLegacyFormat()- CreatesCoreUniversalLinkfrom legacy URL stringsextractActionParams()- Extracts action-specific paths and query parameterstoProtocolUrl()- Converts links between protocols (metamask://, https://, ethereum://, dapp://)shouldUseNewSystem()- Feature flag system for gradual rollout (currently disabled for all actions)wrapHandler()- Wraps legacy handlers to acceptCoreUniversalLinkAdded comprehensive test coverage (23 test cases):
Why This Approach
shouldUseNewSystem()allows action-by-action rolloutDependencies
None
How to Test
Changelog
Added
LegacyLinkAdapterfor bidirectional conversion between newCoreUniversalLinkformat and legacy deep link handlersshouldUseNewSystem()feature flag for gradual rolloutNote
Introduce LegacyLinkAdapter for bidirectional conversion between new core links and legacy handlers, add protocol/path utilities and tests, and map
ACTIONS.PERPS_MARKETSin the normalizer.LegacyLinkAdapterwith:toLegacyFormat/fromLegacyFormatfor bidirectional conversion betweenCoreUniversalLinkand legacyurlObj+params.wrapHandlerto adapt legacy handlers to core links.extractActionParamsto derive action path/query params.toProtocolUrlto convert links acrossmetamask,https,ethereum, anddapp.shouldUseNewSystemfeature-flag list (currently empty).ACTIONS.PERPS_MARKETS→perpsMarketsPathtoACTION_PATH_MAPinCoreLinkNormalizer.LegacyLinkAdapter.test.tscovering conversions, param handling (incl. SDK and custom params), protocol conversions, empty/edge cases, and handler wrapping.Written by Cursor Bugbot for commit d81867b. This will update automatically on new commits. Configure here.