fix: Address multiple memory leaks in CodeBlock component

[kiwina]

PR Type

• Bugfix

Description

This PR addresses multiple potential memory leaks in the CodeBlock component located at webview-ui/src/components/common/CodeBlock.tsx related to unmanaged asynchronous operations and setTimeout calls.

Specifically, it resolves issues where:

1. The useEffect hook for syntax highlighting (original line 247) could call setHighlightedCode on an unmounted component.
2. A useEffect hook (original line 456) using setTimeout (original line 459) to update button positions did not clear the timeout on unmount or re-render.
3. Nested setTimeout calls in the collapse/expand button's onClick handler (original lines 694, 699) were not cleared on unmount.

The fix implements the following patterns:

1. An isMountedRef is used to track the component's mounted state, preventing state updates on unmounted components for async operations.
2. setTimeout IDs are stored in refs (buttonPositionTimeoutRef, collapseTimeout1Ref, collapseTimeout2Ref).
3. Timeouts are cleared before new ones are set and in useEffect cleanup functions to ensure they don't execute after the component unmounts.
4. Checks for codeBlockRef.current are added before DOM manipulations within timeout callbacks.

These changes ensure that state updates are not attempted on unmounted components and that timeouts are properly managed, resolving potential memory leaks and eliminating associated React warnings.

Related Issue

#4243

Changes

• Modified webview-ui/src/components/common/CodeBlock.tsx to include mounted state checks and proper cleanup for asynchronous operations and setTimeout calls.

Testing

Manual testing:

1. Introduce delays in getHighlighter, codeToHtml, or within setTimeout callbacks to simulate longer async operations.
2. Render a CodeBlock component.
3. Quickly unmount the CodeBlock component (e.g., by removing its parent, navigating away, or rapidly clicking the collapse/expand button).
4. Observe the console for "Can't perform a React state update on an unmounted component" warnings. With the fix, this warning should no longer appear.

---

Important

Fixes memory leaks in CodeBlock component by managing async operations and setTimeout calls with proper cleanup and mount state checks.

• Behavior:
  • Fixes memory leaks in CodeBlock component by managing async operations and setTimeout calls.
  • Uses isMountedRef to prevent state updates on unmounted components.
  • Clears setTimeout IDs stored in refs (buttonPositionTimeoutRef, collapseTimeout1Ref, collapseTimeout2Ref) on unmount.
  • Adds checks for codeBlockRef.current before DOM manipulations in timeout callbacks.
• Code Changes:
  • Modifies useEffect hooks to include cleanup functions for timeouts.
  • Updates highlight() function to check isMountedRef before setting state.
  • Adjusts onClick handler for collapse/expand button to clear nested timeouts.
• Testing:
  • Manual testing involves simulating async delays and unmounting CodeBlock to check for React warnings.

^{This description was created by for 4313d4c. You can customize this summary. It will automatically update as commits are pushed.}

[daniel-lxs]

Hey @kiwina, Thank you for your contribution,

I left a couple of comments about code organization, nothing major.

Thank you for taking the time to cleanup the memory leaks, let me know what you think!

[daniel-lxs]

Is there a specific reason for having a dedicated useEffect just for managing isMountedRef? This could potentially be integrated into the existing syntax highlighting effect since they have the same lifecycle. Just curious about the design choice here!

[daniel-lxs]

I noticed that the cleanup pattern for timeouts is a bit inconsistent. The buttonPositionTimeoutRef is cleaned up in the same effect that sets it (lines 521-526), while the collapse timeouts have a separate cleanup effect here.

Could we consider consolidating these patterns? Having a consistent approach would improve maintainability. What do you think about either:

1. Moving all timeout cleanups to their respective effects, or
2. Having a single cleanup effect for all timeouts?

Both approaches would work, just wondering if there's a preference!

[daniel-lxs]

Applied the requested changes.

LGTM