Skip to content

Commit e96c9e1

Browse files
committed
Squashed commit of the following:
commit bc89ede Author: Peter C <12292660+PeterC89@users.noreply.github.com> Date: Mon Jun 8 12:30:27 2026 +0100 fix: Coderabbit review comments commit 2af65b9 Author: Peter C <12292660+PeterC89@users.noreply.github.com> Date: Mon Jun 8 12:26:02 2026 +0100 fix: Coderabbit review comments commit c5f4070 Author: Peter C <12292660+PeterC89@users.noreply.github.com> Date: Mon Jun 8 12:15:18 2026 +0100 fix(webui): Prevent unexpected crash in VTSourceRenderer commit 99fd994 Author: Peter C <12292660+PeterC89@users.noreply.github.com> Date: Fri Jun 5 00:01:54 2026 +0100 fix(webui): Fix more memory leaks [copilot]
1 parent 5b16647 commit e96c9e1

30 files changed

Lines changed: 825 additions & 228 deletions

packages/webui/src/client/lib/Escape.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ function usePortal(id: string, style?: Partial<Record<keyof React.CSSProperties,
4040
const existingParent = document.querySelector<HTMLElement>(`#${id}`)
4141
// Parent is either a new root or the existing dom element
4242
const parentElem = existingParent || createRootElement(id)
43+
const parentCreatedByThisHook = !existingParent
4344

4445
// If there is no existing DOM element, add a new one.
4546
if (!existingParent) {
@@ -53,14 +54,21 @@ function usePortal(id: string, style?: Partial<Record<keyof React.CSSProperties,
5354

5455
return function removeElement() {
5556
rootElemRef.current?.remove()
56-
if (!parentElem.childElementCount) {
57+
if (parentCreatedByThisHook && !parentElem.childElementCount) {
5758
parentElem.remove()
5859
}
5960
}
6061
},
6162
[id]
6263
)
6364

65+
useEffect(function cleanupOnUnmount() {
66+
return function removeOnUnmount() {
67+
rootElemRef.current?.remove()
68+
rootElemRef.current = null
69+
}
70+
}, [])
71+
6472
/**
6573
* It's important we evaluate this lazily:
6674
* - We need first render to contain the DOM element, so it shouldn't happen

packages/webui/src/client/lib/VirtualElement.tsx

Lines changed: 129 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -76,26 +76,38 @@ export function VirtualElement({
7676
const isTransitioning = useRef(false)
7777

7878
const isCurrentlyObserving = useRef(false)
79+
const observedElementRef = useRef<HTMLDivElement | null>(null)
80+
const isMountedRef = useRef(true)
81+
82+
useEffect(() => {
83+
return () => {
84+
isMountedRef.current = false
85+
}
86+
}, [])
87+
88+
const placeholderHeightPx = measurements?.clientHeight ?? placeholderHeight ?? ref?.clientHeight ?? 0
7989

8090
const styleObj = useMemo<React.CSSProperties>(
8191
() => ({
8292
width: width ?? 'auto',
83-
height: ((placeholderHeight || ref?.clientHeight) ?? '0') + 'px',
93+
height: `${placeholderHeightPx}px`,
8494
// These properties are used to ensure that if a prior element is changed from
85-
// placeHolder to element, the position of visible elements are not affected.
95+
// placeholder to element, the position of visible elements are not affected.
8696
contentVisibility: 'auto',
87-
containIntrinsicSize: `0 ${(placeholderHeight || ref?.clientHeight) ?? '0'}px`,
97+
containIntrinsicSize: `0 ${placeholderHeightPx}px`,
8898
contain: 'size layout',
8999
}),
90-
[width, placeholderHeight]
100+
[width, placeholderHeightPx]
91101
)
92102

93103
const handleResize = useCallback(() => {
104+
if (!isMountedRef.current) return
94105
if (ref) {
95106
// Show children during measurement
96107
setIsShowingChildren(true)
97108

98109
requestAnimationFrame(() => {
110+
if (!isMountedRef.current) return
99111
const measurements = measureElement(ref, placeholderHeight)
100112
if (measurements) {
101113
setMeasurements(measurements)
@@ -111,6 +123,18 @@ export function VirtualElement({
111123
}
112124
}, [ref, inView, placeholderHeight])
113125

126+
const unobserveElement = useCallback(
127+
(element: HTMLDivElement | null) => {
128+
if (!element) return
129+
resizeObserverManager.unobserve(element)
130+
if (observedElementRef.current === element) {
131+
observedElementRef.current = null
132+
}
133+
isCurrentlyObserving.current = false
134+
},
135+
[resizeObserverManager]
136+
)
137+
114138
// failsafe to ensure visible elements if resizing happens while scrolling
115139
useEffect(() => {
116140
if (!isShowingChildren) {
@@ -165,6 +189,10 @@ export function VirtualElement({
165189
}, [inView, isShowingChildren])
166190

167191
useEffect(() => {
192+
if (observedElementRef.current && observedElementRef.current !== ref) {
193+
unobserveElement(observedElementRef.current)
194+
}
195+
168196
if (inView) {
169197
setIsShowingChildren(true)
170198
}
@@ -195,14 +223,12 @@ export function VirtualElement({
195223
if (ref) {
196224
if (!isCurrentlyObserving.current) {
197225
resizeObserverManager.observe(ref, handleResize)
226+
observedElementRef.current = ref
198227
isCurrentlyObserving.current = true
199228
}
200229
}
201230
} else {
202-
if (ref && isCurrentlyObserving.current) {
203-
resizeObserverManager.unobserve(ref)
204-
isCurrentlyObserving.current = false
205-
}
231+
if (ref) unobserveElement(ref)
206232
setIsShowingChildren(false)
207233
}
208234
} catch (error) {
@@ -212,7 +238,7 @@ export function VirtualElement({
212238
inViewChangeTimerRef.current = undefined
213239
}
214240
}, 100)
215-
}, [inView, ref, handleResize, resizeObserverManager])
241+
}, [inView, ref, handleResize, resizeObserverManager, unobserveElement])
216242

217243
const onVisibleChanged = useCallback(
218244
(visible: boolean) => {
@@ -225,12 +251,13 @@ export function VirtualElement({
225251
)
226252

227253
const isScrolling = (): boolean => {
254+
const { isProgrammaticScrollInProgress, lastProgrammaticScrollTime } = getViewPortScrollingState()
228255
// Don't do updates while scrolling:
229-
if (getViewPortScrollingState().isProgrammaticScrollInProgress) {
256+
if (isProgrammaticScrollInProgress) {
230257
return true
231258
}
232259
// And wait if a programmatic scroll was done recently:
233-
const timeSinceLastProgrammaticScroll = Date.now() - getViewPortScrollingState().lastProgrammaticScrollTime
260+
const timeSinceLastProgrammaticScroll = Date.now() - lastProgrammaticScrollTime
234261
if (timeSinceLastProgrammaticScroll < 100) {
235262
return true
236263
}
@@ -241,22 +268,23 @@ export function VirtualElement({
241268
// Setup initial observer if element is in view
242269
if (ref && inView && !isCurrentlyObserving.current) {
243270
resizeObserverManager.observe(ref, handleResize)
271+
observedElementRef.current = ref
244272
isCurrentlyObserving.current = true
245273
}
246274

247275
// Cleanup function
248276
return () => {
249277
// Clean up resize observer
250-
if (ref && isCurrentlyObserving.current) {
251-
resizeObserverManager.unobserve(ref)
252-
isCurrentlyObserving.current = false
278+
if (ref) unobserveElement(ref)
279+
if (observedElementRef.current && observedElementRef.current !== ref) {
280+
unobserveElement(observedElementRef.current)
253281
}
254282

255283
if (inViewChangeTimerRef.current) {
256284
clearTimeout(inViewChangeTimerRef.current)
257285
}
258286
}
259-
}, [ref, inView, handleResize])
287+
}, [ref, inView, handleResize, unobserveElement])
260288

261289
useEffect(() => {
262290
if (inView === true) {
@@ -296,6 +324,7 @@ export function VirtualElement({
296324
}
297325
idleCallback = window.requestIdleCallback(
298326
() => {
327+
if (!isMountedRef.current) return
299328
// Measure the entire wrapper element instead of just the childRef
300329
if (ref) {
301330
const measurements = measureElement(ref, placeholderHeight)
@@ -413,6 +442,23 @@ export class ElementObserverManager {
413442
private resizeObserver: ResizeObserver
414443
private mutationObserver: MutationObserver
415444
private observedElements: Map<HTMLElement, () => void>
445+
private isMutationObserverActive = false
446+
447+
private hasConnectedObservedElements(): boolean {
448+
for (const observedElement of this.observedElements.keys()) {
449+
if (document.contains(observedElement)) return true
450+
}
451+
return false
452+
}
453+
454+
private pruneDetachedObservedElements(): void {
455+
for (const observedElement of Array.from(this.observedElements.keys())) {
456+
if (!document.contains(observedElement)) {
457+
this.observedElements.delete(observedElement)
458+
this.resizeObserver.unobserve(observedElement)
459+
}
460+
}
461+
}
416462

417463
private constructor() {
418464
this.observedElements = new Map()
@@ -421,39 +467,85 @@ export class ElementObserverManager {
421467
this.resizeObserver = new ResizeObserver((entries) => {
422468
entries.forEach((entry) => {
423469
const element = entry.target as HTMLElement
470+
if (!document.contains(element)) {
471+
this.observedElements.delete(element)
472+
this.resizeObserver.unobserve(element)
473+
return
474+
}
424475
const callback = this.observedElements.get(element)
425476
if (callback) {
426477
callback()
427478
}
428479
})
480+
481+
// Ensure detached entries are aggressively cleaned even without follow-up DOM mutations.
482+
this.pruneDetachedObservedElements()
483+
if (this.observedElements.size === 0) {
484+
this.disconnectMutationObserver()
485+
}
429486
})
430487

431-
// Configure MutationObserver
488+
// Configure MutationObserver once and only connect/disconnect based on active observed elements.
432489
this.mutationObserver = new MutationObserver((mutations) => {
490+
if (this.observedElements.size === 0) return
491+
492+
this.pruneDetachedObservedElements()
493+
if (this.observedElements.size === 0) {
494+
this.disconnectMutationObserver()
495+
return
496+
}
433497
const targets = new Set<HTMLElement>()
434498

435499
mutations.forEach((mutation) => {
436-
const target = mutation.target as HTMLElement
437-
// Find the closest observed element
438-
let element = target
500+
let element: HTMLElement | null = null
501+
if (mutation.target instanceof HTMLElement) {
502+
element = mutation.target
503+
} else {
504+
element = mutation.target.parentElement
505+
}
506+
507+
if (!element || !document.contains(element)) return
508+
439509
while (element) {
440510
if (this.observedElements.has(element)) {
441511
targets.add(element)
442512
break
443513
}
444-
if (!element.parentElement) break
445514
element = element.parentElement
446515
}
447516
})
448517

449-
// Call callbacks for affected elements
450518
targets.forEach((element) => {
519+
if (!document.contains(element)) {
520+
this.observedElements.delete(element)
521+
this.resizeObserver.unobserve(element)
522+
return
523+
}
451524
const callback = this.observedElements.get(element)
452525
if (callback) callback()
453526
})
454527
})
455528
}
456529

530+
private ensureMutationObserverConnected(): void {
531+
if (this.isMutationObserverActive) return
532+
if (this.observedElements.size === 0) return
533+
if (!this.hasConnectedObservedElements()) return
534+
if (!document.body) return
535+
536+
this.mutationObserver.observe(document.body, {
537+
childList: true,
538+
subtree: true,
539+
})
540+
this.isMutationObserverActive = true
541+
}
542+
543+
private disconnectMutationObserver(): void {
544+
if (!this.isMutationObserverActive) return
545+
this.mutationObserver.disconnect()
546+
this.isMutationObserverActive = false
547+
}
548+
457549
public static getInstance(): ElementObserverManager {
458550
if (!ElementObserverManager.instance) {
459551
ElementObserverManager.instance = new ElementObserverManager()
@@ -463,31 +555,31 @@ export class ElementObserverManager {
463555

464556
public observe(element: HTMLElement, callback: () => void): void {
465557
if (!element) return
558+
if (!document.contains(element)) return
559+
this.pruneDetachedObservedElements()
466560

467561
this.observedElements.set(element, callback)
468562
this.resizeObserver.observe(element)
469-
this.mutationObserver.observe(element, {
470-
childList: true,
471-
subtree: true,
472-
attributes: true,
473-
characterData: true,
474-
})
563+
this.ensureMutationObserverConnected()
475564
}
476565

477566
public unobserve(element: HTMLElement): void {
478567
if (!element) return
479568
this.observedElements.delete(element)
480569
this.resizeObserver.unobserve(element)
570+
this.pruneDetachedObservedElements()
481571

482-
// Disconnect and reconnect mutation observer to refresh the list of observed elements
483-
this.mutationObserver.disconnect()
484-
this.observedElements.forEach((_, el) => {
485-
this.mutationObserver.observe(el, {
486-
childList: true,
487-
subtree: true,
488-
attributes: true,
489-
characterData: true,
490-
})
491-
})
572+
if (this.observedElements.size === 0) {
573+
this.resizeObserver.disconnect()
574+
this.disconnectMutationObserver()
575+
return
576+
}
577+
578+
if (!this.hasConnectedObservedElements()) {
579+
this.disconnectMutationObserver()
580+
return
581+
}
582+
583+
this.ensureMutationObserverConnected()
492584
}
493585
}

0 commit comments

Comments
 (0)