[pull] main from MetaMask:main#554
Merged
Merged
Conversation
## **Description**
Migrated `usePredictActivity` from manual loading/refresh state to a
React Query-based implementation and aligned transactions view + tests
with the query-result contract. This removes legacy hook options/refresh
plumbing and keeps activity data fresh by invalidating the new activity
query after relevant Predict mutations.
## **Changelog**
CHANGELOG entry: null
## **Related issues**
Fixes:
## **Manual testing steps**
```gherkin
Feature: Predict activity history uses React Query
Scenario: user views and refreshes transaction history
Given user is on the Predict Transactions tab with an account that has activity
When user opens the Transactions tab
Then activity rows are loaded from the query and rendered
When user pulls to refresh
Then the view calls query refetch and keeps rendering using query loading states
```
## **Screenshots/Recordings**
### **Before**
N/A
### **After**
N/A
## **Pre-merge author checklist**
- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
## **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.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Moderate risk due to a behavioral refactor of activity
fetching/refresh semantics (React Query caching, address-scoped keys,
and removal of focus-based refresh), which could affect when/which
activity data is shown.
>
> **Overview**
> Migrates `usePredictActivity` from manual state management +
navigation focus refresh to a `@tanstack/react-query` `useQuery`
implementation keyed by selected EVM `address`, with centralized query
config in new `predictQueries.activity`.
>
> Updates `PredictTransactionsView` (and its tests) to consume the
query-result contract (`data`, `isRefetching`, `refetch`) and adds
activity query invalidation after order placement and when Predict
transactions are confirmed, keeping the activity list in sync.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
b2f1553. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
…connect tunnel routing conflicts (#26519) ## **Description** All MM-Connect performance tests on CI were consistently failing with "not able to connect to bs-local.com:8090", and after the initial fix a second issue surfaced causing `BROWSERSTACK_LOCAL_CONNECTION_FAILED` for all appwright tests. ### Root cause 1: localIdentifier tunnel group conflict Every job in a workflow run — `imported-wallet` (Android), `imported-wallet` (iOS), and `mm-connect` (Android) — calls the reusable `performance-test-runner.yml` and starts a BrowserStack Local tunnel using `local-identifier: ${{ github.run_id }}`. Because all concurrent jobs share the same `run_id`, BrowserStack treats their tunnel binaries as a **tunnel group** and may route any session's request to any binary in the group. The appwright patch hardcodes `local: true` for all sessions, so imported-wallet sessions also participate in the group. When a mm-connect session navigates to `http://bs-local.com:8090`, BrowserStack can route the request to an imported-wallet runner — where no dapp server is listening on port 8090 — causing a consistent connection failure. **Fix:** Append `build_type` and `matrix.device.name` to the `local-identifier` so each job registers an isolated tunnel that only its own session uses. ### Root cause 2: unsafe characters in localIdentifier breaking BrowserStack Local binary Device names (e.g. `"Samsung Galaxy S23 Ultra"`) contain spaces. When passed directly as `--local-identifier` to the BrowserStack Local binary, the spaces caused argument misparsing — the tunnel registered with a truncated identifier. The session then sent the full string as `localIdentifier`, found no matching tunnel, and threw `BROWSERSTACK_LOCAL_CONNECTION_FAILED` for every test. **Fix:** Add a dedicated "Compute BrowserStack Local Identifier" step that sanitizes both `inputs.build_type` and `matrix.device.name` using `tr -c 'a-zA-Z0-9-' '_'`, replacing every unsafe character (spaces, `/`, `\`, `@`, etc.) with an underscore. Uses `printf` instead of `echo` to avoid a trailing newline becoming a trailing underscore. Both values are validated to be non-empty and fail fast with a clear error if not, preventing malformed identifiers with trailing hyphens. The sanitized value is stored as a step output and referenced consistently in both `setup-local` and `BROWSERSTACK_LOCAL_IDENTIFIER`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MMQA-1488 ## **Manual testing steps** ```gherkin Feature: MM-Connect performance tests on CI Scenario: mm-connect tests run without tunnel routing conflicts Given the performance E2E workflow is triggered And imported-wallet and mm-connect jobs run concurrently When BrowserStack Local is started with a unique sanitized identifier per job Then each BrowserStack session routes bs-local.com traffic to its own runner And mm-connect tests can reach the dapp server on port 8090 And all three mm-connect specs pass on BrowserStack And no BROWSERSTACK_LOCAL_CONNECTION_FAILED errors appear for any test ``` ## **Screenshots/Recordings** ### **Before** - MM-Connect specs: "not able to connect to bs-local.com:8090" - All appwright tests after initial fix: `BROWSERSTACK_LOCAL_CONNECTION_FAILED` ### **After** [BrowserStack Build Results](https://app-automate.browserstack.com/dashboard/v2/public-build/Tjh6Y3N1b3pwYXNxSmtvQjAwNkV0TFA4ekNVOFZxdXBQMmNka041OC9XbDVHc1JHVVJabzczb254Qmc3SThYVjVybWlhYW5qbXA1RTZOZHk2TjM2Y0E5PS0tV0VIMEh3U2tUbnUrZGw5WmppWCt0UT09--74149fd798c3d43122ccbfd107c699ad988d74e1) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Workflow-only change limited to BrowserStack Local tunnel configuration; main risk is mis-sanitization or identifier formatting causing tunnels not to match sessions. > > **Overview** > Prevents BrowserStack Local tunnel collisions in the reusable performance test workflow by generating a **unique per-job** `local-identifier` from `github.run_id`, `inputs.build_type`, and `matrix.device.name`. > > Adds a dedicated step to **sanitize** identifier components (replace non-alphanumeric/hyphen chars with `_`) and fail fast if inputs are empty, then reuses the computed value for both `setup-local` and `BROWSERSTACK_LOCAL_IDENTIFIER` during test execution. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5d9ba63. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…26571) ## **Description** **Reason for the change:** This is a preparatory change for upgrading the React Navigation library to v6. With global default types now applied to the navigation API, we no longer need to apply generic types to the `useNavigation` hook in most cases. **Improvement/Solution:** - Removed generic types from `useNavigation` hook invocations (now uses default global navigation types) - Applied `NavigationProp<NavigatableRootParamList>` where deeply nested navigation is required - Part of a series of PRs splitting the cleanup by codebase ownership **Note on exceptions:** Generic types may still be needed when navigating to deeper nested stacks. For those instances, we apply a recursive navigation type pattern. **References:** - Parent issue: #23763 - React Navigation nesting docs: https://reactnavigation.org/docs/nesting-navigators/ ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: #23763 (partial) ## **Manual testing steps** ```gherkin Feature: Navigation functionality after useNavigation cleanup Scenario: Web3auth/Onboarding navigation works correctly Given the app is running And the user is going through onboarding or social login flows When user navigates between onboarding screens Then the navigation should complete successfully And no TypeScript errors should occur ``` **Additional verification:** - Run `yarn lint:tsc` to verify no TypeScript errors are introduced - Run unit tests for affected components ## **Screenshots/Recordings** N/A - No visual changes (internal refactoring/type cleanup) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches navigation behavior across onboarding/social login and vault recovery flows by switching from `replace` to dispatching `StackActions.replace`, which can subtly change how navigation state updates. Risk is mitigated by expanded/updated unit tests covering the new action dispatching behavior. > > **Overview** > Refactors several onboarding/social-login and vault-recovery screens to stop using explicit `useNavigation` generic typings (and removes related `@react-navigation/stack` type imports) in preparation for React Navigation v6. > > Standardizes route replacement to use `navigation.dispatch(StackActions.replace(...))` instead of `navigation.replace(...)` in `Onboarding`, `SocialLoginIosUser`, and the vault recovery flow (`RestoreWallet`, `WalletResetNeeded`, `WalletRestored`), including the offline social-login error-sheet path. > > Updates and adds unit tests to match the new dispatch-based replace behavior (mocking `dispatch` and asserting `REPLACE` actions), and refactors `SocialLoginErrorSheet` tests for more robust rendering and async assertions. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6920402. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor <cursoragent@cursor.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )