Skip to content

Commit a82a469

Browse files
authored
fix(perps): Button flash when open a market with an open position (#27671)
## **Description** When navigating to a market with an existing position, Long/Short buttons briefly flashed before switching to Modify/Close. Root cause: `positions` in `usePerpsLivePositions` was derived from `rawPositions` via a `useEffect`, creating a one-frame lag where `isInitialLoading` was `false` but `positions` still held stale data. Fix: replace the state+effect pattern with `useMemo` for synchronous derivation. ## **Changelog** CHANGELOG entry: Fixed button flash (Long/Short briefly visible) when opening a market with an existing position ## **Related issues** Fixes: [TAT-2236](https://consensyssoftware.atlassian.net/browse/TAT-2236) ## **Manual testing steps** ```gherkin Feature: Market detail action buttons Scenario: Open market with existing position Given I have an open BTC position When I navigate to the BTC market detail screen Then I see Modify/Close buttons immediately And I do not see Long/Short buttons flash Scenario: Open market without position Given I have no position on SOL When I navigate to the SOL market detail screen Then I see Long/Short buttons immediately ``` ## **Screenshots/Recordings** ### **Before** See automation/27671/before.mp4 ### **After** See automation/27671/after.mp4 ## **Pre-merge author checklist** - [x] I've followed MetaMask Contributor Docs and Coding Standards - [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 format if applicable - [x] I've applied the right labels on the PR ## **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** > Low risk: small hook refactor to compute `positions` synchronously plus a targeted regression test; main risk is subtle rendering behavior changes around initial WebSocket updates. > > **Overview** > Fixes a one-render mismatch in `usePerpsLivePositions` where `isInitialLoading` could flip to `false` before derived `positions` updated, causing a brief Long/Short button flash when opening a market with an existing position. > > `positions` is now derived from `rawPositions` and `priceData` via `useMemo` (instead of state set in an effect), and a new regression test asserts that the first WebSocket update makes `isInitialLoading=false` and `positions` available in the same render. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 997d441. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 2da84f5 commit a82a469

2 files changed

Lines changed: 38 additions & 14 deletions

File tree

app/components/UI/Perps/hooks/stream/useLivePositions.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,38 @@ describe('usePerpsLivePositions', () => {
738738
});
739739
});
740740

741+
describe('TAT-2236: no button flash on first WebSocket update', () => {
742+
it('positions are available synchronously when isInitialLoading becomes false', () => {
743+
// This test verifies the fix for TAT-2236: when the first WebSocket update
744+
// arrives, positions must be resolved in the SAME render as isInitialLoading
745+
// becoming false. If positions lag by one render, the market detail view
746+
// briefly shows Long/Short buttons before switching to Modify/Close.
747+
let capturedCallback: (positions: Position[] | null) => void = jest.fn();
748+
mockPositionsSubscribe.mockImplementation((params) => {
749+
capturedCallback = params.callback;
750+
return jest.fn();
751+
});
752+
mockPricesSubscribe.mockReturnValue(jest.fn());
753+
754+
const { result } = renderHook(() => usePerpsLivePositions());
755+
756+
// Initially loading with no positions
757+
expect(result.current.isInitialLoading).toBe(true);
758+
expect(result.current.positions).toEqual([]);
759+
760+
// Simulate first WebSocket update with positions
761+
act(() => {
762+
capturedCallback([mockPosition]);
763+
});
764+
765+
// CRITICAL: Both isInitialLoading=false AND positions must be set
766+
// in the same render — no intermediate state allowed
767+
expect(result.current.isInitialLoading).toBe(false);
768+
expect(result.current.positions).toEqual([mockPosition]);
769+
expect(result.current.positions.length).toBe(1);
770+
});
771+
});
772+
741773
describe('enrichPositionsWithLivePnL', () => {
742774
const basePriceData: Record<string, PriceUpdate> = {
743775
'BTC-PERP': {

app/components/UI/Perps/hooks/stream/usePerpsLivePositions.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useState, useRef } from 'react';
1+
import { useEffect, useMemo, useState, useRef } from 'react';
22
import { usePerpsStream } from '../../providers/PerpsStreamManager';
33
import { DevLogger } from '../../../../../core/SDKConnect/utils/DevLogger';
44
import { type Position, type PriceUpdate } from '@metamask/perps-controller';
@@ -99,9 +99,6 @@ export function usePerpsLivePositions(
9999
): UsePerpsLivePositionsReturn {
100100
const { throttleMs = 0, useLivePnl = false } = options; // No live PnL by default to avoid unnecessary re-renders
101101
const stream = usePerpsStream();
102-
const [positions, setPositions] = useState<Position[]>(
103-
() => getPreloadedData<Position[]>('cachedPositions') ?? EMPTY_POSITIONS,
104-
);
105102
const [isInitialLoading, setIsInitialLoading] = useState(
106103
() => !hasPreloadedData('cachedPositions'),
107104
);
@@ -113,18 +110,13 @@ export function usePerpsLivePositions(
113110
);
114111
const [priceData, setPriceData] = useState<Record<string, PriceUpdate>>({});
115112

116-
// Enrich and update positions whenever raw positions or prices change
117-
useEffect(() => {
113+
// Derive enriched positions synchronously to avoid one-frame flash
114+
// where isInitialLoading is false but positions haven't been enriched yet
115+
const positions = useMemo(() => {
118116
if (rawPositions.length === 0) {
119-
setPositions(EMPTY_POSITIONS);
120-
return;
117+
return EMPTY_POSITIONS;
121118
}
122-
123-
const enrichedPositions = enrichPositionsWithLivePnL(
124-
rawPositions,
125-
priceData,
126-
);
127-
setPositions(enrichedPositions);
119+
return enrichPositionsWithLivePnL(rawPositions, priceData);
128120
}, [rawPositions, priceData]);
129121

130122
// Subscribe to position updates

0 commit comments

Comments
 (0)