feat: add tokens search discovery controller #13111
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. |
Bigshmow
changed the title
|
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
Bigshmow
added
Run Smoke E2E
|
|
@SocketSecurity ignore npm/@metamask/[email protected] |
…s path (#5195) ## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> The `/tokens-search/name` endpoint has changed to `/tokens-search` to better reflect it's usecase in [PR-75](consensys-vertical-apps/va-mmcx-portfolio-api#75) in the portfolio api. The endpoint searches by both name and address of tokens from Moralis. Simply "/name" was misleading. We also updated the param "name" to "query" to better reflect the nature of the request. ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/token-search-discover-controller` - **Added**: The logoUrl is now exposed in TokenSearchResponseItem via `searchTokens` - **Breaking**: - Update the TokenSearchApiService to use the updated URL for `searchTokens` - The URL is now `/tokens-search` instead of `/tokens-search/name` - Changed the "name" parameter to "query" in the `searchTokens` method - These updates align with the Portfolio API's `/tokens-search` endpoint - The old endpoint `/tokens-search/name` will remain in a deprecated state until parity across apps is achieved which for now is the portfolio-api and core repos. The mobile [PR-13111](MetaMask/metamask-mobile#13111) is not yet merged. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
Bigshmow
added
Run Smoke E2E
|
Bigshmow
added
Run Smoke E2E
|
Bigshmow
added
Run Smoke E2E
|
Bigshmow
added
Run Smoke E2E
|
Bigshmow
added
Run Smoke E2E
salimtb
added
Run Smoke E2E
Bigshmow
added
Run Smoke E2E
|
| import { selectRecentTokenSearches } from '../../../selectors/tokenSearchDiscoveryController'; | ||
| import { TokenSearchParams } from '@metamask/token-search-discovery-controller/dist/types.cjs'; | ||
|
|
||
| export const useTokenSearchDiscovery = () => { |
There was a problem hiding this comment.
One important consideration: If the searchTokens hook is triggered with every change to the search parameters (e.g., when TokenSearchParams contains a search string and the user rapidly types "testToken"), would this execute the search logic individually for each character input?
If this is the intended behavior, I recommend implementing debouncing. However, let's first confirm whether each keystroke indeed triggers a separated call, as this would help determine if the debounce optimization is necessary to prevent excessive execution during rapid input.
There was a problem hiding this comment.
I hope to keep the controller itself as close to single responsibility as we can so as to not confuse callers let's agree to introduce that debounce here in the consuming app. It's a good idea, thanks; however I'm not sure on exact implementation yet (re: each key press). Either way I see a useDebouncedValue hook that might help here, is that what you suggest?
Something like:
const SEARCH_DEBOUNCE_DELAY = 300;
const searchTokens = useCallback(async (params: TokenSearchParams) => {
const debouncedParams = useDebouncedValue(params, SEARCH_DEBOUNCE_DELAY);
const { TokenSearchDiscoveryController } = Engine.context;
return await TokenSearchDiscoveryController.searchTokens(debouncedParams);
}, []);
|
Description
This PR introduces the
@metamask/token-search-discovery-controllerto mobile. The package provides token search functionality via the Portfolio API & Moralis, which will enable the front end to implement a token search view.We have created a factory function to introduce the class into Engine context, and redux selectors and a custom hook were added as well as some tests.
See MMPD-1504 MMPD-1526 in Jira for added context.
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist