Skip to content

Commit 970df1e

Browse files
authored
refactor(plugins): Migrate to reactive streams and signal-based state (#4254)
* refactor(plugins): migrate to reactive streams and signal-based state - Refactor Hover plugin to use fromEvent() and effect() for reactive event handling - Refactor Timeline plugin to react to state.duration changes automatically - Refactor Zoom plugin to integrate with state.zoom and state.duration signals - Refactor Minimap plugin to react to state.audioBuffer changes - Replace manual event listener cleanup with unified subscriptions array - Eliminate custom unsubscribe functions in favor of BasePlugin pattern - Integrate plugins with WaveSurfer reactive state signals Benefits: - Consistent reactive patterns across all plugins - Declarative state-driven updates - Automatic cleanup prevents memory leaks - Better TypeScript integration with signals - Reduced boilerplate code All tests pass. No breaking changes to public APIs. * fix(Timeline): prevent timeline disappearing on scroll The scroll listener was being re-registered on every initTimeline() call, causing multiple listeners to reference stale timeline DOM elements. Solution: - Register scroll listener once in onInit() instead of in initTimeline() - Store reference to current timeline element - Update scroll listener to use the stored reference This prevents memory leaks and ensures the timeline remains visible when scrolling. * fix(Minimap): restore rendering by using duration state instead of audioBuffer The audioBuffer state is never set in WaveSurfer core, so the reactive effect was never triggered. The minimap requires decoded data to initialize, which is available after the duration is set. Solution: - React to state.duration changes instead of state.audioBuffer - Check that decoded data exists before initializing minimap - This effectively replaces the 'decode' event with reactive state observation The minimap now renders correctly on audio load. * refactor(Zoom): convert addEventListener to reactive fromEvent streams Replace manual event listener management with reactive event streams: - Use fromEvent() for wheel, touchstart, touchmove, touchend, touchcancel - Use effect() to react to event stream changes - Remove manual removeEventListener calls in destroy() - Cleanup now handled automatically through subscriptions array Benefits: - Consistent reactive pattern across all DOM event handling - Automatic cleanup prevents memory leaks - More declarative event handling * fix(Minimap): revert to using decode event instead of reactive state The previous reactive approach had a race condition where state.duration could be set before decoded data was available, causing the minimap to not render. The 'decode' event is the correct trigger as it fires exactly when audio data is decoded and ready. Changes: - Revert from effect(state.duration) back to on('decode') - Remove unused effect import - Simplify onScroll to use getDuration() directly This restores the original working behavior while maintaining the subscriptions array pattern for automatic cleanup.
1 parent 2dc0845 commit 970df1e

File tree

4 files changed

+159
-79
lines changed

4 files changed

+159
-79
lines changed

src/plugins/hover.ts

Lines changed: 69 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
import BasePlugin, { type BasePluginEvents } from '../base-plugin.js'
66
import createElement from '../dom.js'
7+
import { fromEvent } from '../reactive/event-streams.js'
8+
import { effect } from '../reactive/store.js'
79

810
export type HoverPluginOptions = {
911
/** The hover cursor color (playback cursor and progress mask colors used as falllback in this order)
@@ -53,7 +55,6 @@ class HoverPlugin extends BasePlugin<HoverPluginEvents, HoverPluginOptions> {
5355
private wrapper: HTMLElement
5456
private label: HTMLElement
5557
private lastPointerPosition: { clientX: number; clientY: number } | null = null
56-
private unsubscribe: () => void = () => undefined
5758

5859
constructor(options?: HoverPluginOptions) {
5960
super(options || {})
@@ -109,66 +110,82 @@ class HoverPlugin extends BasePlugin<HoverPluginEvents, HoverPluginOptions> {
109110
const container = this.wavesurfer.getWrapper()
110111
container.appendChild(this.wrapper)
111112

112-
// When zoom or scroll happens, re-run the pointer move logic
113-
// with the last known mouse position
113+
// Get reactive state
114+
const state = this.wavesurfer.getState()
115+
116+
// Create event streams for pointer events
117+
const pointerMove = fromEvent(container, 'pointermove')
118+
const pointerLeave = fromEvent(container, 'pointerleave')
119+
120+
// React to pointer movement
121+
this.subscriptions.push(
122+
effect(() => {
123+
const e = pointerMove.value
124+
if (!e || !this.wavesurfer) return
125+
126+
// Store only the position data needed for zoom/scroll updates
127+
this.lastPointerPosition = { clientX: e.clientX, clientY: e.clientY }
128+
129+
// Position
130+
const bbox = this.wavesurfer.getWrapper().getBoundingClientRect()
131+
const { width } = bbox
132+
const offsetX = e.clientX - bbox.left
133+
const relX = Math.min(1, Math.max(0, offsetX / width))
134+
const posX = Math.min(width - this.options.lineWidth - 1, offsetX)
135+
this.wrapper.style.transform = `translateX(${posX}px)`
136+
this.wrapper.style.opacity = '1'
137+
138+
// Timestamp
139+
const duration = state.duration.value
140+
this.label.textContent = this.options.formatTimeCallback(duration * relX)
141+
const labelWidth = this.label.offsetWidth
142+
const transformCondition = this.options.labelPreferLeft ? posX - labelWidth > 0 : posX + labelWidth > width
143+
this.label.style.transform = transformCondition ? `translateX(-${labelWidth + this.options.lineWidth}px)` : ''
144+
145+
// Emit a hover event with the relative X position
146+
this.emit('hover', relX)
147+
}, [pointerMove, state.duration]),
148+
)
149+
150+
// React to pointer leave
151+
this.subscriptions.push(
152+
effect(() => {
153+
const e = pointerLeave.value
154+
if (!e) return
155+
156+
this.wrapper.style.opacity = '0'
157+
this.lastPointerPosition = null
158+
}, [pointerLeave]),
159+
)
160+
161+
// When zoom or scroll happens, re-run the pointer move logic with the last known mouse position
114162
const onUpdate = () => {
115-
if (this.lastPointerPosition) {
116-
// Recreate a minimal PointerEvent-like object
117-
this.onPointerMove(this.lastPointerPosition as PointerEvent)
163+
if (this.lastPointerPosition && this.wavesurfer) {
164+
// Position
165+
const bbox = this.wavesurfer.getWrapper().getBoundingClientRect()
166+
const { width } = bbox
167+
const offsetX = this.lastPointerPosition.clientX - bbox.left
168+
const relX = Math.min(1, Math.max(0, offsetX / width))
169+
const posX = Math.min(width - this.options.lineWidth - 1, offsetX)
170+
this.wrapper.style.transform = `translateX(${posX}px)`
171+
172+
// Timestamp
173+
const duration = state.duration.value
174+
this.label.textContent = this.options.formatTimeCallback(duration * relX)
175+
const labelWidth = this.label.offsetWidth
176+
const transformCondition = this.options.labelPreferLeft ? posX - labelWidth > 0 : posX + labelWidth > width
177+
this.label.style.transform = transformCondition ? `translateX(-${labelWidth + this.options.lineWidth}px)` : ''
118178
}
119179
}
120180

121-
// Create unsubscribe function for wavesurfer events
122-
const unsubscribeZoom = this.wavesurfer.on('zoom', onUpdate)
123-
const unsubscribeScroll = this.wavesurfer.on('scroll', onUpdate)
124-
125-
// Attach pointer events and create complete unsubscribe function
126-
container.addEventListener('pointermove', this.onPointerMove)
127-
container.addEventListener('pointerleave', this.onPointerLeave)
128-
129-
this.unsubscribe = () => {
130-
container.removeEventListener('pointermove', this.onPointerMove)
131-
container.removeEventListener('pointerleave', this.onPointerLeave)
132-
unsubscribeZoom()
133-
unsubscribeScroll()
134-
}
135-
}
136-
137-
private onPointerMove = (e: PointerEvent) => {
138-
if (!this.wavesurfer) return
139-
140-
// Store only the position data needed for zoom/scroll updates
141-
this.lastPointerPosition = { clientX: e.clientX, clientY: e.clientY }
142-
143-
// Position
144-
const bbox = this.wavesurfer.getWrapper().getBoundingClientRect()
145-
const { width } = bbox
146-
const offsetX = e.clientX - bbox.left
147-
const relX = Math.min(1, Math.max(0, offsetX / width))
148-
const posX = Math.min(width - this.options.lineWidth - 1, offsetX)
149-
this.wrapper.style.transform = `translateX(${posX}px)`
150-
this.wrapper.style.opacity = '1'
151-
152-
// Timestamp
153-
const duration = this.wavesurfer.getDuration() || 0
154-
this.label.textContent = this.options.formatTimeCallback(duration * relX)
155-
const labelWidth = this.label.offsetWidth
156-
const transformCondition = this.options.labelPreferLeft ? posX - labelWidth > 0 : posX + labelWidth > width
157-
this.label.style.transform = transformCondition ? `translateX(-${labelWidth + this.options.lineWidth}px)` : ''
158-
159-
// Emit a hover event with the relative X position
160-
this.emit('hover', relX)
161-
}
162-
163-
private onPointerLeave = () => {
164-
this.wrapper.style.opacity = '0'
165-
this.lastPointerPosition = null
181+
// Subscribe to zoom and scroll events
182+
this.subscriptions.push(this.wavesurfer.on('zoom', onUpdate))
183+
this.subscriptions.push(this.wavesurfer.on('scroll', onUpdate))
166184
}
167185

168186
/** Unmount */
169187
public destroy() {
170188
super.destroy()
171-
this.unsubscribe()
172189
this.wrapper.remove()
173190
}
174191
}

src/plugins/minimap.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ class MinimapPlugin extends BasePlugin<MinimapPluginEvents, MinimapPluginOptions
244244
private initWaveSurferEvents() {
245245
if (!this.wavesurfer) return
246246

247+
// Subscribe to decode, scroll and redraw events
247248
this.subscriptions.push(
248249
this.wavesurfer.on('decode', () => {
249250
this.initMinimap()

src/plugins/timeline.ts

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import BasePlugin, { type BasePluginEvents } from '../base-plugin.js'
66
import createElement from '../dom.js'
7+
import { effect } from '../reactive/store.js'
78

89
export type TimelinePluginOptions = {
910
/** The height of the timeline in pixels, defaults to 20 */
@@ -56,9 +57,9 @@ export type TimelinePluginEvents = BasePluginEvents & {
5657

5758
class TimelinePlugin extends BasePlugin<TimelinePluginEvents, TimelinePluginOptions> {
5859
private timelineWrapper: HTMLElement
59-
private unsubscribeNotches: (() => void)[] = []
6060
protected options: TimelinePluginOptions & typeof defaultOptions
6161
private notchElements: Map<HTMLElement, { start: number; width: number; wasVisible: boolean }> = new Map()
62+
private currentTimeline: HTMLElement | null = null
6263

6364
constructor(options?: TimelinePluginOptions) {
6465
super(options || {})
@@ -95,17 +96,37 @@ class TimelinePlugin extends BasePlugin<TimelinePluginEvents, TimelinePluginOpti
9596
container.appendChild(this.timelineWrapper)
9697
}
9798

99+
// Get reactive state
100+
const state = this.wavesurfer.getState()
101+
102+
// React to duration changes and redraw events to initialize timeline
103+
this.subscriptions.push(
104+
effect(() => {
105+
const duration = state.duration.value
106+
if (duration > 0 || this.options.duration) {
107+
this.initTimeline()
108+
}
109+
}, [state.duration]),
110+
)
111+
98112
this.subscriptions.push(this.wavesurfer.on('redraw', () => this.initTimeline()))
99113

114+
// Add single scroll listener for all notches (register once, not on every redraw)
115+
this.subscriptions.push(
116+
this.wavesurfer.on('scroll', (_start, _end, scrollLeft, scrollRight) => {
117+
if (this.currentTimeline) {
118+
this.updateVisibleNotches(scrollLeft, scrollRight, this.currentTimeline)
119+
}
120+
}),
121+
)
122+
100123
if (this.wavesurfer?.getDuration() || this.options.duration) {
101124
this.initTimeline()
102125
}
103126
}
104127

105128
/** Unmount */
106129
public destroy() {
107-
this.unsubscribeNotches.forEach((unsubscribe) => unsubscribe())
108-
this.unsubscribeNotches = []
109130
this.timelineWrapper.remove()
110131
super.destroy()
111132
}
@@ -188,8 +209,6 @@ class TimelinePlugin extends BasePlugin<TimelinePluginEvents, TimelinePluginOpti
188209
}
189210

190211
private initTimeline() {
191-
this.unsubscribeNotches.forEach((unsubscribe) => unsubscribe())
192-
this.unsubscribeNotches = []
193212
this.notchElements.clear()
194213

195214
const duration = this.wavesurfer?.getDuration() ?? this.options.duration ?? 0
@@ -272,15 +291,7 @@ class TimelinePlugin extends BasePlugin<TimelinePluginEvents, TimelinePluginOpti
272291

273292
this.timelineWrapper.innerHTML = ''
274293
this.timelineWrapper.appendChild(timeline)
275-
276-
// Add single scroll listener for all notches (instead of one per notch)
277-
if (this.wavesurfer) {
278-
this.unsubscribeNotches.push(
279-
this.wavesurfer.on('scroll', (_start, _end, scrollLeft, scrollRight) => {
280-
this.updateVisibleNotches(scrollLeft, scrollRight, timeline)
281-
}),
282-
)
283-
}
294+
this.currentTimeline = timeline
284295

285296
this.emit('ready')
286297
}

src/plugins/zoom.ts

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
*/
2323

2424
import { BasePlugin, BasePluginEvents } from '../base-plugin.js'
25+
import { effect } from '../reactive/store.js'
26+
import { fromEvent } from '../reactive/event-streams.js'
2527

2628
export type ZoomPluginOptions = {
2729
/**
@@ -95,16 +97,72 @@ class ZoomPlugin extends BasePlugin<ZoomPluginEvents, ZoomPluginOptions> {
9597
}
9698
this.container = this.wrapper.parentElement as HTMLElement
9799

98-
this.container.addEventListener('wheel', this.onWheel)
99-
this.container.addEventListener('touchstart', this.onTouchStart, { passive: false, capture: true })
100-
this.container.addEventListener('touchmove', this.onTouchMove, { passive: false, capture: true })
101-
this.container.addEventListener('touchend', this.onTouchEnd, { passive: false, capture: true })
102-
this.container.addEventListener('touchcancel', this.onTouchEnd, { passive: false, capture: true })
103-
104100
if (typeof this.options.maxZoom === 'undefined') {
105101
this.options.maxZoom = this.container.clientWidth
106102
}
107103
this.endZoom = this.options.maxZoom
104+
105+
// Get reactive state
106+
const state = this.wavesurfer?.getState()
107+
108+
// React to zoom state changes to update internal state
109+
if (state) {
110+
this.subscriptions.push(
111+
effect(() => {
112+
const zoom = state.zoom.value
113+
if (zoom > 0 && this.startZoom === 0 && this.options.exponentialZooming) {
114+
const duration = state.duration.value
115+
if (duration > 0 && this.container) {
116+
this.startZoom = this.container.clientWidth / duration
117+
}
118+
}
119+
}, [state.zoom, state.duration]),
120+
)
121+
}
122+
123+
// Create event streams
124+
const wheelStream = fromEvent(this.container, 'wheel')
125+
const touchStartStream = fromEvent(this.container, 'touchstart')
126+
const touchMoveStream = fromEvent(this.container, 'touchmove')
127+
const touchEndStream = fromEvent(this.container, 'touchend')
128+
const touchCancelStream = fromEvent(this.container, 'touchcancel')
129+
130+
// React to wheel events
131+
this.subscriptions.push(
132+
effect(() => {
133+
const e = wheelStream.value
134+
if (e) this.onWheel(e)
135+
}, [wheelStream]),
136+
)
137+
138+
// React to touch events
139+
this.subscriptions.push(
140+
effect(() => {
141+
const e = touchStartStream.value
142+
if (e) this.onTouchStart(e)
143+
}, [touchStartStream]),
144+
)
145+
146+
this.subscriptions.push(
147+
effect(() => {
148+
const e = touchMoveStream.value
149+
if (e) this.onTouchMove(e)
150+
}, [touchMoveStream]),
151+
)
152+
153+
this.subscriptions.push(
154+
effect(() => {
155+
const e = touchEndStream.value
156+
if (e) this.onTouchEnd(e)
157+
}, [touchEndStream]),
158+
)
159+
160+
this.subscriptions.push(
161+
effect(() => {
162+
const e = touchCancelStream.value
163+
if (e) this.onTouchEnd(e)
164+
}, [touchCancelStream]),
165+
)
108166
}
109167

110168
private onWheel = (e: WheelEvent) => {
@@ -248,13 +306,6 @@ class ZoomPlugin extends BasePlugin<ZoomPluginEvents, ZoomPluginOptions> {
248306
}
249307

250308
destroy() {
251-
if (this.container) {
252-
this.container.removeEventListener('wheel', this.onWheel)
253-
this.container.removeEventListener('touchstart', this.onTouchStart)
254-
this.container.removeEventListener('touchmove', this.onTouchMove)
255-
this.container.removeEventListener('touchend', this.onTouchEnd)
256-
this.container.removeEventListener('touchcancel', this.onTouchEnd)
257-
}
258309
super.destroy()
259310
}
260311
}

0 commit comments

Comments
 (0)