Skip to content

Commit 6787571

Browse files
chore(runway): cherry-pick fix(perps): prevent infinite loop in usePerpsPositionData cp-7.59.0 (#22305)
- fix(perps): prevent infinite loop in usePerpsPositionData cp-7.59.0 (#22257) ## **Description** This PR fixes a bug in the Perps candle subscription system where the refresh interval was being continuously cleared and re-established, causing excessive logging and inefficient resource usage. **What is the reason for the change?** - `candleData` was in the dependency array of the candle refresh interval effect in `usePerpsPositionData.ts` - This created a self-triggering loop: the effect updates `candleData` → triggers re-run → clears interval → creates new interval → repeats - The result was continuous logging of "Cleared candle refresh interval" and "Setting up candle refresh every..." messages - This caused unnecessary interval teardown/recreation on every candle data update (potentially multiple times per second) **What is the improvement/solution?** - Removed `candleData` from the dependency array of the interval effect (line 230 in `usePerpsPositionData.ts`) - Removed duplicate `initializationState` dependency (already covered by `isControllerInitialized`) - Added explanatory comment and ESLint disable directive to document why `candleData` is intentionally excluded - The interval now only resets when configuration actually changes (interval period, duration, etc.), not on every data update ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: Infinite loop in candle subscription causing excessive logging ## **Manual testing steps** ```gherkin Feature: Candle Subscription Refresh Behavior Scenario: user observes stable candle refresh intervals Given user is on Perps position details screen And developer console is visible When user selects a time interval (e.g., "1h") Then user should see "Setting up candle refresh every 3600 seconds for 1h" ONCE And user should NOT see continuous "Cleared candle refresh interval" messages When user waits for 5 seconds Then user should NOT see any new interval setup messages When user changes the time interval to "3m" Then user should see "Cleared candle refresh interval" ONCE And user should see "Setting up candle refresh every 180 seconds for 3m" ONCE Scenario: user verifies candles still update correctly Given user is on Perps position details screen And user has selected any time interval When user observes the price chart Then the live candle should update in real-time as prices change And historical candles should remain stable When the interval period completes (e.g., after 3 minutes for 3m interval) Then a new candle should appear And the chart should refresh with updated historical data ``` ## **Screenshots/Recordings** N/A - Internal logging fix ## **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 (all existing tests pass) - [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. --- ## **Additional Context** ### Root Cause Analysis The bug was in the `useEffect` at lines 161-234 of `usePerpsPositionData.ts`: **Problem Chain:** 1. Effect depends on `candleData` (line 228 in original code) 2. Effect's interval callback calls `setCandleData()` (lines 207-216) 3. Separate effect merges live candles into `candleData` (lines 300-336) 4. Every `candleData` update triggers the interval effect to re-run 5. Re-run clears old interval and creates new one 6. New data from interval or live candle merge triggers step 4 again ### Technical Details **Changed File:** - `app/components/UI/Perps/hooks/usePerpsPositionData.ts` **Changes Made:** 1. Removed `candleData` from dependency array (line 230) 2. Removed duplicate `initializationState` from dependency array (line 232) 3. Added explanatory comment (lines 227-228) 4. Added `eslint-disable-next-line react-hooks/exhaustive-deps` (line 229) **Test Results:** - ✅ All 38 tests in `usePerpsPositionData.test.ts` pass - ✅ No functional changes to candle fetching or display - ✅ Interval still refreshes at correct intervals - ✅ Live candles still merge correctly ### Why ESLint Disable is Necessary The ESLint rule `react-hooks/exhaustive-deps` flags `candleData` as missing from dependencies. However: - Including `candleData` would recreate the bug - The effect doesn't depend on previous `candleData` values - It only needs to re-run when interval configuration changes - This is a legitimate case for disabling the rule with proper documentation ### Impact **Components Affected:** - `PerpsMarketDetailsView.tsx` (main consumer) - `usePerpsMarketStats.ts` (secondary consumer) **Performance Improvement:** - Before: Interval cleared/created multiple times per second - After: Interval cleared/created only on configuration changes - Reduction: ~99% fewer interval operations **User Impact:** - No visible changes to end users - Chart behavior remains identical - Reduced console noise for developers - Improved performance and memory usage <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Prevents infinite interval re-creation by removing `candleData` (and duplicate `initializationState`) from the refresh effect deps and documenting with comments/ESLint disable. > > - **Perps Hook (`usePerpsPositionData.ts`)**: > - **Interval Effect Stability**: > - Remove `candleData` from dependency array to avoid infinite interval reset loop. > - Remove redundant `initializationState` dependency (covered by `isControllerInitialized`). > - Add explanatory comments and `eslint-disable-next-line react-hooks/exhaustive-deps` for intentional dep exclusion. > - Effect now depends on `isLoadingHistory`, `selectedInterval`, `fetchHistoricalCandles`, and `isControllerInitialized` only. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 53119ff. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> [f817b25](f817b25) Co-authored-by: abretonc7s <107169956+abretonc7s@users.noreply.github.com>
1 parent 0ccab95 commit 6787571

1 file changed

Lines changed: 3 additions & 2 deletions

File tree

app/components/UI/Perps/hooks/usePerpsPositionData.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,12 +224,13 @@ export const usePerpsPositionData = ({
224224
clearInterval(intervalId);
225225
DevLogger.log('Cleared candle refresh interval');
226226
};
227+
// Note: candleData is intentionally excluded from deps to prevent infinite loop
228+
// This effect only needs to re-run when the interval settings change
229+
// eslint-disable-next-line react-hooks/exhaustive-deps
227230
}, [
228-
candleData,
229231
isLoadingHistory,
230232
selectedInterval,
231233
fetchHistoricalCandles,
232-
initializationState,
233234
isControllerInitialized,
234235
]);
235236

0 commit comments

Comments
 (0)