Skip to content

Commit b7314ee

Browse files
PeterC89PeterC89
committed
Merge branch 'fix-memory-leaks' into bbc-main-autonext
2 parents 09dbcb2 + 2ba1f1a commit b7314ee

7 files changed

Lines changed: 50 additions & 33 deletions

File tree

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

Lines changed: 3 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,7 +54,8 @@ function usePortal(id: string, style?: Partial<Record<keyof React.CSSProperties,
5354

5455
return function removeElement() {
5556
rootElemRef.current?.remove()
56-
if (!parentElem.childElementCount) {
57+
rootElemRef.current = null
58+
if (parentCreatedByThisHook && !parentElem.childElementCount) {
5759
parentElem.remove()
5860
}
5961
}

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

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ export function VirtualElement({
7676
const isTransitioning = useRef(false)
7777

7878
const isCurrentlyObserving = useRef(false)
79+
const observedElementRef = useRef<HTMLDivElement | null>(null)
7980
const isMountedRef = useRef(true)
8081

8182
useEffect(() => {
@@ -117,6 +118,18 @@ export function VirtualElement({
117118
}
118119
}, [ref, inView, placeholderHeight])
119120

121+
const unobserveElement = useCallback(
122+
(element: HTMLDivElement | null) => {
123+
if (!element) return
124+
resizeObserverManager.unobserve(element)
125+
if (observedElementRef.current === element) {
126+
observedElementRef.current = null
127+
}
128+
isCurrentlyObserving.current = false
129+
},
130+
[resizeObserverManager]
131+
)
132+
120133
// failsafe to ensure visible elements if resizing happens while scrolling
121134
useEffect(() => {
122135
if (!isShowingChildren) {
@@ -171,6 +184,10 @@ export function VirtualElement({
171184
}, [inView, isShowingChildren])
172185

173186
useEffect(() => {
187+
if (observedElementRef.current && observedElementRef.current !== ref) {
188+
unobserveElement(observedElementRef.current)
189+
}
190+
174191
if (inView) {
175192
setIsShowingChildren(true)
176193
}
@@ -201,14 +218,12 @@ export function VirtualElement({
201218
if (ref) {
202219
if (!isCurrentlyObserving.current) {
203220
resizeObserverManager.observe(ref, handleResize)
221+
observedElementRef.current = ref
204222
isCurrentlyObserving.current = true
205223
}
206224
}
207225
} else {
208-
if (ref) {
209-
resizeObserverManager.unobserve(ref)
210-
isCurrentlyObserving.current = false
211-
}
226+
if (ref) unobserveElement(ref)
212227
setIsShowingChildren(false)
213228
}
214229
} catch (error) {
@@ -218,7 +233,7 @@ export function VirtualElement({
218233
inViewChangeTimerRef.current = undefined
219234
}
220235
}, 100)
221-
}, [inView, ref, handleResize, resizeObserverManager])
236+
}, [inView, ref, handleResize, resizeObserverManager, unobserveElement])
222237

223238
const onVisibleChanged = useCallback(
224239
(visible: boolean) => {
@@ -242,22 +257,23 @@ export function VirtualElement({
242257
// Setup initial observer if element is in view
243258
if (ref && inView && !isCurrentlyObserving.current) {
244259
resizeObserverManager.observe(ref, handleResize)
260+
observedElementRef.current = ref
245261
isCurrentlyObserving.current = true
246262
}
247263

248264
// Cleanup function
249265
return () => {
250266
// Clean up resize observer
251-
if (ref) {
252-
resizeObserverManager.unobserve(ref)
253-
isCurrentlyObserving.current = false
267+
if (ref) unobserveElement(ref)
268+
if (observedElementRef.current && observedElementRef.current !== ref) {
269+
unobserveElement(observedElementRef.current)
254270
}
255271

256272
if (inViewChangeTimerRef.current) {
257273
clearTimeout(inViewChangeTimerRef.current)
258274
}
259275
}
260-
}, [ref, inView, handleResize])
276+
}, [ref, inView, handleResize, unobserveElement])
261277

262278
useEffect(() => {
263279
if (inView === true) {

‎packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ export const PreviewPopUp = React.forwardRef<
105105

106106
useEffect(() => {
107107
return () => {
108+
anchorRef.current = null
108109
anchorYRef.current = 0
109110
virtualElement.current.getBoundingClientRect = generateGetBoundingClientRect(0, 0)
110111
updateRef.current = null

‎packages/webui/src/client/ui/SegmentAdlibTesting/SegmentAdlibTesting.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ export const SegmentAdlibTesting = React.memo(
304304
}
305305
}
306306

307-
const onSegmentWheel = (e: WheelEvent) => {
307+
const onSegmentWheel = useCallback((e: WheelEvent) => {
308308
let scrollDelta = 0
309309
if (
310310
(!e.ctrlKey && e.altKey && !e.metaKey && !e.shiftKey) ||
@@ -329,7 +329,7 @@ export const SegmentAdlibTesting = React.memo(
329329
return newScrollLeft
330330
})
331331
}
332-
}
332+
}, [maxScrollLeft, props.onScroll])
333333

334334
useEffect(() => {
335335
if (!grabbed) return
@@ -416,7 +416,7 @@ export const SegmentAdlibTesting = React.memo(
416416
return () => {
417417
segment.removeEventListener('wheel', onSegmentWheel)
418418
}
419-
}, [innerRef.current])
419+
}, [onSegmentWheel])
420420

421421
return (
422422
<div

‎packages/webui/src/client/ui/SegmentStoryboard/SegmentStoryboard.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ export const SegmentStoryboard = React.memo(
406406
}
407407
}
408408

409-
const onSegmentWheel = (e: WheelEvent) => {
409+
const onSegmentWheel = useCallback((e: WheelEvent) => {
410410
let scrollDelta = 0
411411
if (
412412
(!e.ctrlKey && e.altKey && !e.metaKey && !e.shiftKey) ||
@@ -431,7 +431,7 @@ export const SegmentStoryboard = React.memo(
431431
return newScrollLeft
432432
})
433433
}
434-
}
434+
}, [maxScrollLeft, props.onScroll])
435435

436436
useEffect(() => {
437437
if (!grabbed) return
@@ -518,7 +518,7 @@ export const SegmentStoryboard = React.memo(
518518
return () => {
519519
segment.removeEventListener('wheel', onSegmentWheel)
520520
}
521-
}, [innerRef.current])
521+
}, [onSegmentWheel])
522522

523523
const onScrollbarChange = useCallback((left: number) => {
524524
setScrollLeft(Math.max(0, Math.min(left, maxScrollLeft)))

‎packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ export const SourceLayerItem = (props: Readonly<ISourceLayerItemProps>): JSX.Ele
174174
previewSession.current.close()
175175
previewSession.current = null
176176
}
177+
itemElementRef.current = null
177178
}
178179
}, [])
179180

@@ -278,9 +279,14 @@ export const SourceLayerItem = (props: Readonly<ISourceLayerItemProps>): JSX.Ele
278279
}
279280

280281
animFrameHandle.current = requestAnimationFrame(updatePos)
281-
}, [piece, contentStatus, timeScale])
282+
}, [timeScale])
282283
const togglePreviewPopUp = useCallback(
283284
(e: React.MouseEvent, state: boolean) => {
285+
if (animFrameHandle.current !== undefined) {
286+
cancelAnimationFrame(animFrameHandle.current)
287+
animFrameHandle.current = undefined
288+
}
289+
284290
if (!state && previewSession.current) {
285291
previewSession.current.close()
286292
previewSession.current = null
@@ -321,9 +327,10 @@ export const SourceLayerItem = (props: Readonly<ISourceLayerItemProps>): JSX.Ele
321327
animFrameHandle.current = requestAnimationFrame(updatePos)
322328
} else if (animFrameHandle.current !== undefined) {
323329
cancelAnimationFrame(animFrameHandle.current)
330+
animFrameHandle.current = undefined
324331
}
325332
},
326-
[piece, cursorTimePosition, contentStatus, timeScale]
333+
[piece, cursorTimePosition, contentStatus, updatePos, layer.type, previewContext, props.piece.renderedDuration, props.piece.renderedInPoint]
327334
)
328335
const moveMiniInspector = useCallback((e: MouseEvent | any) => {
329336
cursorRawPosition.current = {

‎packages/webui/src/client/ui/util/useSetDocumentClass.ts‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,31 +38,22 @@ export function useOwnedElementClassToggle(
3838
const ownedElementRef = useRef<HTMLElement | null>(null)
3939

4040
useEffect(() => {
41+
removedByHookRef.current = false
42+
ownedElementRef.current = null
43+
4144
const element = document.querySelector(selector)
4245
if (element instanceof HTMLElement && element.isConnected && removeOnMount) {
4346
removedByHookRef.current = element.classList.contains(className)
4447
if (removedByHookRef.current) {
4548
element.classList.remove(className)
4649
ownedElementRef.current = element
47-
} else {
48-
ownedElementRef.current = null
4950
}
5051
}
5152

5253
return () => {
53-
let cleanupElement = ownedElementRef.current
54-
55-
if (!cleanupElement || !cleanupElement.isConnected) {
56-
const fallbackElement = document.querySelector(selector)
57-
cleanupElement = fallbackElement instanceof HTMLElement ? fallbackElement : null
58-
}
59-
60-
if (
61-
removedByHookRef.current &&
62-
cleanupElement &&
63-
cleanupElement.isConnected
64-
) {
65-
cleanupElement.classList.add(className)
54+
const ownedElement = ownedElementRef.current
55+
if (removedByHookRef.current && ownedElement && ownedElement.isConnected) {
56+
ownedElement.classList.add(className)
6657
}
6758

6859
removedByHookRef.current = false

0 commit comments

Comments
 (0)