Skip to content

Commit d4803d4

Browse files
authored
chore(predict): implement sequential refresh pattern for order preview (#22499)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Changes the auto-refresh behavior in usePredictOrderPreview from interval-based to sequential pattern. The timer now waits for the preview response before starting the timeout countdown, preventing overlapping requests. Key changes: - Replace setInterval with recursive setTimeout pattern - Add scheduleNextRefresh function to trigger after response - Schedule next refresh after both successful and error responses - Clear pending timers on unmount and parameter changes - Update tests to verify sequential refresh behavior This ensures the autoRefreshTimeout is a delay AFTER each response completes, rather than a fixed interval that could cause overlapping requests when API response times exceed the timeout value. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: [PRED-299](https://consensyssoftware.atlassian.net/browse/PRED-299?atlOrigin=eyJpIjoiOTQwNThkMTg4N2UzNDQ1MjljZTEyNGM0MTlhOGZjMjUiLCJwIjoiaiJ9) ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] 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 - [ ] 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] > Replaces interval-based auto-refresh with response-triggered sequential timeouts in `usePredictOrderPreview`, adds cleanup on param changes/unmount, and updates tests accordingly. > > - **Hook `usePredictOrderPreview`**: > - Replace `setInterval` auto-refresh with sequential `setTimeout` scheduled via `scheduleNextRefresh` after each response (success or error). > - Introduce refs for timer and callbacks: `refreshTimerRef`, `calculatePreviewRef`, `scheduleNextRefreshRef`. > - Clear pending refresh timers on unmount and when parameters change; debounce initial calculation. > - Minor safety: optional chaining on `calculatePreviewRef.current?.()`. > - **Tests `usePredictOrderPreview.test.ts`**: > - Rename and add cases to verify response-triggered scheduling, waiting before countdown, scheduling after errors, and clearing timers on parameter changes and unmount. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 2c9fb28. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> [PRED-299]: https://consensyssoftware.atlassian.net/browse/PRED-299?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
1 parent cabf6d9 commit d4803d4

File tree

2 files changed

+140
-13
lines changed

2 files changed

+140
-13
lines changed

app/components/UI/Predict/hooks/usePredictOrderPreview.test.ts

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ describe('usePredictOrderPreview', () => {
157157
expect(mockPreviewOrder).toHaveBeenCalledTimes(1);
158158
});
159159

160-
it('auto-refreshes preview at specified interval', async () => {
160+
it('schedules next refresh after receiving response', async () => {
161161
mockPreviewOrder.mockResolvedValue(mockPreview);
162162

163163
const params = { ...defaultParams, autoRefreshTimeout: 2000 };
@@ -189,6 +189,109 @@ describe('usePredictOrderPreview', () => {
189189

190190
expect(mockPreviewOrder).toHaveBeenCalledTimes(3);
191191
});
192+
193+
it('waits for response before starting timeout countdown', async () => {
194+
let callCount = 0;
195+
mockPreviewOrder.mockImplementation(async () => {
196+
callCount += 1;
197+
return mockPreview;
198+
});
199+
200+
const params = { ...defaultParams, autoRefreshTimeout: 1000 };
201+
const { waitForNextUpdate } = renderHook(() =>
202+
usePredictOrderPreview(params),
203+
);
204+
205+
act(() => {
206+
jest.advanceTimersByTime(100);
207+
});
208+
209+
await waitForNextUpdate();
210+
211+
expect(callCount).toBe(1);
212+
213+
act(() => {
214+
jest.advanceTimersByTime(500);
215+
});
216+
217+
expect(callCount).toBe(1);
218+
219+
act(() => {
220+
jest.advanceTimersByTime(500);
221+
});
222+
223+
await waitForNextUpdate();
224+
225+
expect(callCount).toBe(2);
226+
});
227+
228+
it('schedules next refresh after error response', async () => {
229+
mockPreviewOrder.mockRejectedValue(new Error('API Error'));
230+
231+
const consoleErrorSpy = jest
232+
.spyOn(console, 'error')
233+
.mockImplementation(() => {
234+
// Suppress console.error output during test
235+
});
236+
237+
const params = { ...defaultParams, autoRefreshTimeout: 2000 };
238+
const { waitForNextUpdate } = renderHook(() =>
239+
usePredictOrderPreview(params),
240+
);
241+
242+
act(() => {
243+
jest.advanceTimersByTime(100);
244+
});
245+
246+
await waitForNextUpdate();
247+
248+
expect(mockPreviewOrder).toHaveBeenCalledTimes(1);
249+
250+
mockPreviewOrder.mockResolvedValue(mockPreview);
251+
252+
act(() => {
253+
jest.advanceTimersByTime(2000);
254+
});
255+
256+
await waitForNextUpdate();
257+
258+
expect(mockPreviewOrder).toHaveBeenCalledTimes(2);
259+
260+
consoleErrorSpy.mockRestore();
261+
});
262+
263+
it('clears pending refresh timer when parameters change', async () => {
264+
mockPreviewOrder.mockResolvedValue(mockPreview);
265+
266+
const params = { ...defaultParams, autoRefreshTimeout: 2000 };
267+
const { waitForNextUpdate, rerender } = renderHook(
268+
(props: PreviewOrderParams & { autoRefreshTimeout?: number }) =>
269+
usePredictOrderPreview(props),
270+
{ initialProps: params },
271+
);
272+
273+
act(() => {
274+
jest.advanceTimersByTime(100);
275+
});
276+
277+
await waitForNextUpdate();
278+
279+
expect(mockPreviewOrder).toHaveBeenCalledTimes(1);
280+
281+
act(() => {
282+
jest.advanceTimersByTime(1000);
283+
});
284+
285+
rerender({ ...params, size: 200 });
286+
287+
act(() => {
288+
jest.advanceTimersByTime(100);
289+
});
290+
291+
await waitForNextUpdate();
292+
293+
expect(mockPreviewOrder).toHaveBeenCalledTimes(2);
294+
});
192295
});
193296

194297
describe('error handling', () => {

app/components/UI/Predict/hooks/usePredictOrderPreview.ts

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export function usePredictOrderPreview(
2020

2121
const currentOperationRef = useRef<number>(0);
2222
const isMountedRef = useRef<boolean>(true);
23+
const refreshTimerRef = useRef<NodeJS.Timeout | null>(null);
2324

2425
const { previewOrder } = usePredictTrading();
2526

@@ -35,6 +36,26 @@ export function usePredictOrderPreview(
3536
positionId,
3637
} = params;
3738

39+
// Define ref before using it in scheduleNextRefresh
40+
const calculatePreviewRef = useRef<(() => Promise<void>) | null>(null);
41+
42+
const scheduleNextRefresh = useCallback(() => {
43+
if (!autoRefreshTimeout || !isMountedRef.current) return;
44+
45+
// Clear any existing timer
46+
if (refreshTimerRef.current) {
47+
clearTimeout(refreshTimerRef.current);
48+
}
49+
50+
// Schedule next refresh after the timeout
51+
refreshTimerRef.current = setTimeout(() => {
52+
calculatePreviewRef.current?.();
53+
}, autoRefreshTimeout);
54+
}, [autoRefreshTimeout]);
55+
56+
const scheduleNextRefreshRef = useRef(scheduleNextRefresh);
57+
scheduleNextRefreshRef.current = scheduleNextRefresh;
58+
3859
const calculatePreview = useCallback(async () => {
3960
const operationId = ++currentOperationRef.current;
4061

@@ -63,6 +84,8 @@ export function usePredictOrderPreview(
6384
if (operationId === currentOperationRef.current && isMountedRef.current) {
6485
setPreview(p);
6586
setError(null);
87+
// Schedule next refresh after successful response
88+
scheduleNextRefreshRef.current();
6689
}
6790
} catch (err) {
6891
console.error('Failed to preview order:', err);
@@ -93,6 +116,8 @@ export function usePredictOrderPreview(
93116
defaultCode: PREDICT_ERROR_CODES.PREVIEW_FAILED,
94117
});
95118
setError(parsedErrorMessage);
119+
// Schedule next refresh after error response
120+
scheduleNextRefreshRef.current();
96121
}
97122
} finally {
98123
if (operationId === currentOperationRef.current && isMountedRef.current) {
@@ -110,32 +135,31 @@ export function usePredictOrderPreview(
110135
positionId,
111136
]);
112137

113-
const calculatePreviewRef = useRef(calculatePreview);
114138
calculatePreviewRef.current = calculatePreview;
115139

116140
useEffect(
117141
() => () => {
118142
isMountedRef.current = false;
143+
// Clear refresh timer on unmount
144+
if (refreshTimerRef.current) {
145+
clearTimeout(refreshTimerRef.current);
146+
}
119147
},
120148
[],
121149
);
122150

123151
useEffect(() => {
152+
// Clear any pending refresh timer when parameters change
153+
if (refreshTimerRef.current) {
154+
clearTimeout(refreshTimerRef.current);
155+
refreshTimerRef.current = null;
156+
}
157+
124158
const debounceTimer = setTimeout(() => {
125-
calculatePreviewRef.current();
159+
calculatePreviewRef.current?.();
126160
}, 100);
127161

128162
return () => clearTimeout(debounceTimer);
129-
}, [providerId, marketId, outcomeId, outcomeTokenId, side, size]);
130-
131-
useEffect(() => {
132-
if (!autoRefreshTimeout) return undefined;
133-
134-
const refreshTimer = setInterval(() => {
135-
calculatePreviewRef.current();
136-
}, autoRefreshTimeout);
137-
138-
return () => clearInterval(refreshTimer);
139163
}, [
140164
providerId,
141165
marketId,

0 commit comments

Comments
 (0)