refactor(linter): fix errors on build

[RedBoardDev]

fix of #7

[CryptoGnome]

@claude can you check this

[claude]

Claude encountered an error —— View job

Failed with exit code 128

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error —— View job

Failed with exit code 128

---

I'll analyze this and get back to you.

[github-actions]

Code Review for PR #12: refactor(linter): fix errors on build

✅ Security Review

• No dependency changes detected - No modifications to package.json or lock files
• No suspicious code patterns - All changes are legitimate React optimization refactorings
• No security vulnerabilities introduced - Changes only affect component rendering behavior

📝 Summary

This PR addresses React linting errors by properly managing dependencies and preventing unnecessary re-renders. The changes use React refs to capture initial prop values to avoid dependency array warnings.

👍 Positive Aspects

1. Proper use of React refs - Using refs to capture initial values prevents re-running effects when props change, which is the intended behavior
2. Consistent implementation - The same pattern is applied across multiple components (LiquidationFeed, LiquidationSidebar, PositionTable)
3. Addresses the root cause - Fixes React exhaustive-deps warnings without disabling the linter
4. Clear comments - Added helpful comments explaining the use of refs

⚠️ Issues & Recommendations

1. Critical: Incorrect dependency array in PositionTable.tsx

The dependency array includes loadVWAPData callback at line 215, but this creates a circular dependency issue:

• loadVWAPData is defined using useCallback with config?.symbols dependency
• This means loadVWAPData changes when config changes
• The effect will re-run when config changes, which may not be intended

Recommendation: Either:

• Remove loadVWAPData from the dependency array and disable the linter warning with a comment explaining why
• Or move the loadVWAPData definition inside the useEffect to avoid the dependency

2. Potential stale closure issue

While using refs for initial values solves the linter warning, it creates a potential issue where the components won't respond to prop updates. If volumeThresholds or maxEvents are expected to change during the component's lifecycle, the historical liquidations won't be reloaded with new values.

Recommendation: Consider if this is the intended behavior. If props should trigger re-fetching, you might need a different approach.

3. Function hoisting issue in PositionTable

The loadVWAPData function is defined after it's used in the useEffect. While JavaScript hoisting with useCallback works, it makes the code harder to follow.

Recommendation: Move the loadVWAPData definition before the useEffect that uses it for better readability.

🔧 Suggested Improvements

1. Fix the dependency array issue in PositionTable.tsx:

// Option 1: Remove loadVWAPData from deps and add eslint comment
}, [positions.length]); // eslint-disable-line react-hooks/exhaustive-deps
// loadVWAPData intentionally excluded to prevent re-fetching on config changes

2. Consider documenting the decision to use initial values only:

// Design decision: Historical liquidations are loaded once on mount using
// initial prop values. Changes to volumeThresholds or maxEvents after
// mount will not trigger a re-fetch to avoid unexpected data refreshes.
const initialMaxEvents = useRef(maxEvents);

✅ Verdict

The PR successfully addresses the linter errors and improves code quality. However, the dependency array issue in PositionTable.tsx should be resolved before merging. Once that's fixed, this PR is ready to merge.

📊 Risk Assessment

• Security Risk: None ✅
• Breaking Changes: None ✅
• Performance Impact: Positive - reduces unnecessary re-renders
• Test Coverage: Consider adding tests to verify the components don't re-fetch on prop changes if that's the intended behavior