Skip to content

Commit 1e30122

Browse files
committed
fix: Address review feedback for CodeBlock memory leak fixes
- Consolidate isMountedRef management into syntax highlighting useEffect - Unify timeout cleanup patterns for consistent maintainability - Maintain all existing memory leak protections - Improve code organization and readability Addresses review comments from daniel-lxs in PR #4244
1 parent 4313d4c commit 1e30122

File tree

1 file changed

+19
-19
lines changed

1 file changed

+19
-19
lines changed

webview-ui/src/components/common/CodeBlock.tsx

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -247,28 +247,11 @@ const CodeBlock = memo(
247247
}
248248
}, [language, currentLanguage])
249249

250-
// Manage mounted state
250+
// Syntax highlighting with cached Shiki instance and mounted state management
251251
useEffect(() => {
252+
// Set mounted state at the beginning of this effect
252253
isMountedRef.current = true
253-
return () => {
254-
isMountedRef.current = false
255-
}
256-
}, [])
257-
258-
// Cleanup for collapse/expand timeouts
259-
useEffect(() => {
260-
return () => {
261-
if (collapseTimeout1Ref.current) {
262-
clearTimeout(collapseTimeout1Ref.current)
263-
}
264-
if (collapseTimeout2Ref.current) {
265-
clearTimeout(collapseTimeout2Ref.current)
266-
}
267-
}
268-
}, [])
269254

270-
// Syntax highlighting with cached Shiki instance.
271-
useEffect(() => {
272255
const fallback = `<pre style="padding: 0; margin: 0;"><code class="hljs language-${currentLanguage || "txt"}">${source || ""}</code></pre>`
273256

274257
const highlight = async () => {
@@ -317,6 +300,23 @@ const CodeBlock = memo(
317300
setHighlightedCode(fallback)
318301
}
319302
})
303+
304+
// Cleanup function - manage mounted state and clear all timeouts
305+
return () => {
306+
isMountedRef.current = false
307+
if (buttonPositionTimeoutRef.current) {
308+
clearTimeout(buttonPositionTimeoutRef.current)
309+
buttonPositionTimeoutRef.current = null
310+
}
311+
if (collapseTimeout1Ref.current) {
312+
clearTimeout(collapseTimeout1Ref.current)
313+
collapseTimeout1Ref.current = null
314+
}
315+
if (collapseTimeout2Ref.current) {
316+
clearTimeout(collapseTimeout2Ref.current)
317+
collapseTimeout2Ref.current = null
318+
}
319+
}
320320
}, [source, currentLanguage, collapsedHeight])
321321

322322
// Check if content height exceeds collapsed height whenever content changes

0 commit comments

Comments
 (0)