Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 127 additions & 1 deletion src/__tests__/memory-leaks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
*/

import WaveSurfer from '../wavesurfer.js'
import RegionsPlugin from '../plugins/regions.js'

// Mock audio context
// Mock audio context and matchMedia
beforeAll(() => {
global.AudioContext = jest.fn().mockImplementation(() => ({
createMediaElementSource: jest.fn(() => ({
Expand All @@ -22,6 +23,21 @@ beforeAll(() => {
destination: {},
close: jest.fn(),
}))

// Mock matchMedia for drag-stream
Object.defineProperty(window, 'matchMedia', {
writable: true,
value: jest.fn().mockImplementation((query) => ({
matches: false,
media: query,
onchange: null,
addListener: jest.fn(),
removeListener: jest.fn(),
addEventListener: jest.fn(),
removeEventListener: jest.fn(),
dispatchEvent: jest.fn(),
})),
})
})

describe('Memory Leak Detection', () => {
Expand Down Expand Up @@ -150,6 +166,116 @@ describe('Memory Leak Detection', () => {
})
})

describe('Regions plugin memory leak (#4243)', () => {
it('should cleanup region event listeners when removed', () => {
const ws = WaveSurfer.create({ container })
const regions = ws.registerPlugin(RegionsPlugin.create())

// Mock duration so regions are saved immediately
jest.spyOn(ws, 'getDuration').mockReturnValue(10)
jest.spyOn(ws, 'getDecodedData').mockReturnValue({ numberOfChannels: 1 } as any)

// Create a region
const region = regions.addRegion({ start: 0, end: 1 })

// Track if cleanup is happening
const clickHandler = jest.fn()
region.on('click', clickHandler)

// Remove the region
region.remove()

// After removal, the region element should be null
expect(region.element).toBeNull()

// Cleanup
ws.destroy()
})

it('should not retain regions in memory after removal', () => {
const ws = WaveSurfer.create({ container })
const regions = ws.registerPlugin(RegionsPlugin.create())

// Mock duration so regions are saved immediately
jest.spyOn(ws, 'getDuration').mockReturnValue(10)
jest.spyOn(ws, 'getDecodedData').mockReturnValue({ numberOfChannels: 1 } as any)

// Create multiple regions
const region1 = regions.addRegion({ start: 0, end: 1 })
const region2 = regions.addRegion({ start: 2, end: 3 })
const region3 = regions.addRegion({ start: 4, end: 5 })

expect(regions.getRegions().length).toBe(3)

// Remove regions
region1.remove()
region2.remove()

// Only one region should remain
expect(regions.getRegions().length).toBe(1)
expect(regions.getRegions()[0]).toBe(region3)

// Remove last region
region3.remove()
expect(regions.getRegions().length).toBe(0)

// Cleanup
ws.destroy()
})

it('should cleanup content event listeners when region is removed', () => {
const ws = WaveSurfer.create({ container })
const regions = ws.registerPlugin(RegionsPlugin.create())

// Mock duration so regions are saved immediately
jest.spyOn(ws, 'getDuration').mockReturnValue(10)
jest.spyOn(ws, 'getDecodedData').mockReturnValue({ numberOfChannels: 1 } as any)

// Create a region with editable content
const region = regions.addRegion({
start: 0,
end: 1,
content: 'Test content',
contentEditable: true,
})

// Remove the region
region.remove()

// Content should be cleaned up
expect(region.element).toBeNull()

// Cleanup
ws.destroy()
})

it('should cleanup DOM event streams on region removal', () => {
const ws = WaveSurfer.create({ container })
const regions = ws.registerPlugin(RegionsPlugin.create())

// Mock duration so regions are saved immediately
jest.spyOn(ws, 'getDuration').mockReturnValue(10)
jest.spyOn(ws, 'getDecodedData').mockReturnValue({ numberOfChannels: 1 } as any)

// Create regions
const createdRegions = []
for (let i = 0; i < 10; i++) {
createdRegions.push(regions.addRegion({ start: i, end: i + 1 }))
}

expect(regions.getRegions().length).toBe(10)

// Remove all regions
createdRegions.forEach((r) => r.remove())

// All regions should be removed
expect(regions.getRegions().length).toBe(0)

// Cleanup
ws.destroy()
})
})

describe('Event listener cleanup', () => {
it('should properly cleanup on destroy', () => {
const ws = WaveSurfer.create({ container })
Expand Down
58 changes: 52 additions & 6 deletions src/plugins/regions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import EventEmitter from '../event-emitter.js'
import createElement from '../dom.js'
import { createDragStream } from '../reactive/drag-stream.js'
import { effect } from '../reactive/store.js'
import { fromEvent, cleanup as cleanupStream } from '../reactive/event-streams.js'

export type RegionsPluginOptions = undefined
export type UpdateSide = 'start' | 'end'
Expand Down Expand Up @@ -279,12 +280,37 @@ class SingleRegion extends EventEmitter<RegionEvents> implements Region {
const { element } = this
if (!element) return

element.addEventListener('click', (e) => this.emit('click', e))
element.addEventListener('mouseenter', (e) => this.emit('over', e))
element.addEventListener('mouseleave', (e) => this.emit('leave', e))
element.addEventListener('dblclick', (e) => this.emit('dblclick', e))
element.addEventListener('pointerdown', () => this.toggleCursor(true))
element.addEventListener('pointerup', () => this.toggleCursor(false))
// Create event streams
const clicks = fromEvent(element, 'click')
const mouseenters = fromEvent(element, 'mouseenter')
const mouseleaves = fromEvent(element, 'mouseleave')
const dblclicks = fromEvent(element, 'dblclick')
const pointerdowns = fromEvent(element, 'pointerdown')
const pointerups = fromEvent(element, 'pointerup')

// Subscribe to streams
const unsubscribeClick = clicks.subscribe((e) => e && this.emit('click', e))
const unsubscribeMouseenter = mouseenters.subscribe((e) => e && this.emit('over', e))
const unsubscribeMouseleave = mouseleaves.subscribe((e) => e && this.emit('leave', e))
const unsubscribeDblclick = dblclicks.subscribe((e) => e && this.emit('dblclick', e))
const unsubscribePointerdown = pointerdowns.subscribe((e) => e && this.toggleCursor(true))
const unsubscribePointerup = pointerups.subscribe((e) => e && this.toggleCursor(false))

// Store cleanup
this.subscriptions.push(() => {
unsubscribeClick()
unsubscribeMouseenter()
unsubscribeMouseleave()
unsubscribeDblclick()
unsubscribePointerdown()
unsubscribePointerup()
cleanupStream(clicks)
cleanupStream(mouseenters)
cleanupStream(mouseleaves)
cleanupStream(dblclicks)
cleanupStream(pointerdowns)
cleanupStream(pointerups)
})

// Drag
const dragStream = createDragStream(element)
Expand Down Expand Up @@ -500,11 +526,31 @@ class SingleRegion extends EventEmitter<RegionEvents> implements Region {
public remove() {
this.isRemoved = true
this.emit('remove')

// Clean up all subscriptions (drag streams, event listeners, etc.)
this.subscriptions.forEach((unsubscribe) => unsubscribe())
this.subscriptions = []

// Clean up content event listeners
if (this.content && this.contentEditable) {
if (this.contentClickListener) {
this.content.removeEventListener('click', this.contentClickListener)
this.contentClickListener = undefined
}
if (this.contentBlurListener) {
this.content.removeEventListener('blur', this.contentBlurListener)
this.contentBlurListener = undefined
}
}

// Remove DOM element
if (this.element) {
this.element.remove()
this.element = null
}

// Clear all event listeners from the EventEmitter
this.unAll()
}
}

Expand Down