Skip to content

Commit 8d101e9

Browse files
committed
fix: remove audio elements when VoiceRecordingPreview and AudioRecordingPreview are unmounted
1 parent 86eeec4 commit 8d101e9

File tree

8 files changed

+151
-28
lines changed

8 files changed

+151
-28
lines changed

src/components/AudioPlayer/AudioPlayer.ts

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export type AudioPlayerState = {
3030
canPlayRecord: boolean;
3131
/** Current playback speed. Initiated with the first item of the playbackRates array. */
3232
currentPlaybackRate: number;
33-
elementRef: HTMLAudioElement;
33+
elementRef: HTMLAudioElement | null;
3434
isPlaying: boolean;
3535
playbackError: Error | null;
3636
/** An array of fractional numeric values of playback speed to override the defaults (1.0, 1.5, 2.0) */
@@ -135,7 +135,7 @@ export class AudioPlayer {
135135
}
136136

137137
get src() {
138-
return this.elementRef.src;
138+
return this.elementRef?.src;
139139
}
140140

141141
get secondsElapsed() {
@@ -154,6 +154,12 @@ export class AudioPlayer {
154154
return this._mimeType;
155155
}
156156

157+
private ensureElementRef(): HTMLAudioElement {
158+
if (!this.elementRef) {
159+
this.setRef(new Audio(this.src));
160+
}
161+
return this.elementRef as HTMLAudioElement;
162+
}
157163
private setPlaybackStartSafetyTimeout = () => {
158164
clearTimeout(this.playTimeout);
159165
this.playTimeout = setTimeout(() => {
@@ -181,15 +187,25 @@ export class AudioPlayer {
181187
if (durationSeconds !== this._durationSeconds) {
182188
this._durationSeconds = durationSeconds;
183189
}
184-
if (this.elementRef.src !== src) {
185-
this.elementRef.src = src;
190+
const elementRef = this.ensureElementRef();
191+
if (elementRef.src !== src) {
192+
elementRef.src = src;
193+
elementRef.load();
194+
}
195+
}
196+
197+
private prepareElementRefForGarbageCollection() {
198+
this.stop();
199+
if (this.elementRef) {
200+
this.elementRef.src = '';
186201
this.elementRef.load();
202+
this.setRef(null);
187203
}
188204
}
189205

190-
private setRef = (elementRef: HTMLAudioElement) => {
206+
private setRef = (elementRef: HTMLAudioElement | null) => {
191207
if (elementIsPlaying(this.elementRef)) {
192-
this.pause();
208+
this.prepareElementRefForGarbageCollection();
193209
}
194210
this.state.partialNext({ elementRef });
195211
};
@@ -217,6 +233,7 @@ export class AudioPlayer {
217233
!!(mimeType && this.elementRef?.canPlayType(mimeType));
218234

219235
play = async (params?: AudioPlayerPlayAudioParams) => {
236+
const elementRef = this.ensureElementRef();
220237
if (elementIsPlaying(this.elementRef)) {
221238
if (this.isPlaying) return;
222239
this.state.partialNext({ isPlaying: true });
@@ -234,12 +251,12 @@ export class AudioPlayer {
234251
return;
235252
}
236253

237-
this.elementRef.playbackRate = currentPlaybackRate ?? this.currentPlaybackRate;
254+
elementRef.playbackRate = currentPlaybackRate ?? this.currentPlaybackRate;
238255

239256
this.setPlaybackStartSafetyTimeout();
240257

241258
try {
242-
await this.elementRef.play();
259+
await elementRef.play();
243260
this.state.partialNext({
244261
currentPlaybackRate,
245262
isPlaying: true,
@@ -266,7 +283,7 @@ export class AudioPlayer {
266283
stop = () => {
267284
this.pause();
268285
this.setSecondsElapsed(0);
269-
this.elementRef.currentTime = 0;
286+
if (this.elementRef) this.elementRef.currentTime = 0;
270287
};
271288

272289
togglePlay = async () => (this.isPlaying ? this.pause() : await this.play());
@@ -309,7 +326,12 @@ export class AudioPlayer {
309326
this.plugins.forEach(({ onError }) => onError?.({ player: this, ...params }));
310327
};
311328

312-
onRemove = () => {
329+
/**
330+
* Removes the audio element reference, event listeners and audio player from the player pool.
331+
* Helpful when only a single AudioPlayer instance is to be removed from the AudioPlayerPool.
332+
*/
333+
requestRemoval = () => {
334+
this.prepareElementRefForGarbageCollection();
313335
this.unsubscribeEventListeners?.();
314336
this.unsubscribeEventListeners = null;
315337
this.plugins.forEach(({ onRemove }) => onRemove?.({ player: this }));
@@ -318,7 +340,7 @@ export class AudioPlayer {
318340
registerSubscriptions = () => {
319341
this.unsubscribeEventListeners?.();
320342

321-
const audioElement = this.elementRef;
343+
const audioElement = this.ensureElementRef();
322344

323345
const handleEnded = () => {
324346
this.state.partialNext({

src/components/AudioPlayer/AudioPlayerPool.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { AudioPlayer, type AudioPlayerOptions } from './AudioPlayer';
2+
import { audioPlayerRequestRemovalFromPoolPluginFactory } from './plugins/AudioPlayerRequestRemovalPlugin';
23

34
export class AudioPlayerPool {
45
pool = new Map<string, AudioPlayer>();
@@ -10,7 +11,16 @@ export class AudioPlayerPool {
1011
getOrAdd = (params: AudioPlayerOptions) => {
1112
let player = this.pool.get(params.id);
1213
if (player) return player;
13-
player = new AudioPlayer(params);
14+
const requestRemovalPlugin = audioPlayerRequestRemovalFromPoolPluginFactory({
15+
players: this,
16+
});
17+
player = new AudioPlayer({
18+
...params,
19+
plugins: [
20+
...(params.plugins ?? []).filter((p) => p.id !== requestRemovalPlugin.id),
21+
requestRemovalPlugin,
22+
],
23+
});
1424
player.registerSubscriptions();
1525
this.pool.set(params.id, player);
1626
return player;
@@ -19,11 +29,10 @@ export class AudioPlayerPool {
1929
remove = (id: string) => {
2030
const player = this.pool.get(id);
2131
if (!player) return;
22-
player.stop();
23-
player.elementRef.src = '';
24-
player.elementRef.load();
25-
player.onRemove();
26-
this.pool.delete(id);
32+
player.requestRemoval();
33+
if (this.pool.has(id)) {
34+
this.pool.delete(id);
35+
}
2736
};
2837

2938
clear = () => {

src/components/AudioPlayer/__tests__/AudioPlayer.test.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,4 +320,38 @@ describe('AudioPlayer', () => {
320320
endedSpy.mockReturnValue(true);
321321
expect(elementIsPlaying(el)).toBe(false);
322322
});
323+
324+
it('requestRemoval clears element (load called) and nulls elementRef, notifies plugins', () => {
325+
const onRemove = jest.fn();
326+
const player = makePlayer({ plugins: [{ id: 'TestOnRemove', onRemove }] });
327+
328+
const el = createdAudios[0];
329+
const loadSpy = jest.spyOn(el, 'load');
330+
331+
expect(player.elementRef).toBe(el);
332+
333+
player.requestRemoval();
334+
335+
expect(loadSpy).toHaveBeenCalled();
336+
expect(player.elementRef).toBeNull();
337+
expect(onRemove).toHaveBeenCalledWith(expect.objectContaining({ player }));
338+
});
339+
340+
it('play() after requestRemoval recreates a fresh HTMLAudioElement via ensureElementRef', async () => {
341+
jest.spyOn(HTMLMediaElement.prototype, 'canPlayType').mockReturnValue('maybe');
342+
const player = makePlayer();
343+
344+
const firstEl = createdAudios[0];
345+
expect(player.elementRef).toBe(firstEl);
346+
347+
player.requestRemoval();
348+
expect(player.elementRef).toBeNull();
349+
350+
await player.play();
351+
352+
const secondEl = player.elementRef;
353+
expect(secondEl).toBeTruthy();
354+
expect(secondEl).not.toBe(firstEl);
355+
expect(createdAudios.length).toBe(2);
356+
});
323357
});

src/components/AudioPlayer/__tests__/WithAudioPlayback.test.js

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ describe('WithAudioPlayback + useAudioPlayer', () => {
287287
unmount();
288288

289289
// cannot do "expect(player.elementRef.src).toBe('');" as player.elementRef.src in JSDOM normalizes to "http://localhost/"
290-
expect(player.elementRef.getAttribute('src')).toBe('');
290+
expect(player.elementRef).toBeNull();
291291
expect(loadSpy).toHaveBeenCalled();
292292
});
293293

@@ -318,4 +318,35 @@ describe('WithAudioPlayback + useAudioPlayer', () => {
318318
expect.arrayContaining(['ended', 'error', 'timeupdate']),
319319
);
320320
});
321+
322+
it('re-mounting provider with same props creates a fresh player and cleans previous element', () => {
323+
const params = { mimeType: 'audio/mpeg', src: 'https://example.com/a.mp3' };
324+
325+
let firstPlayer;
326+
const { unmount } = renderWithProvider(
327+
<RegisterPlayer onReady={(p) => (firstPlayer = p)} params={params} />,
328+
);
329+
330+
// first mount created one element
331+
expect(createdAudios.length).toBe(1);
332+
const firstEl = createdAudios[0];
333+
expect(firstPlayer).toBeTruthy();
334+
expect(firstPlayer.elementRef).toBe(firstEl);
335+
336+
unmount();
337+
338+
// After unmount, player was cleaned
339+
expect(firstPlayer.elementRef).toBeNull();
340+
341+
// New provider -> new pool -> new player + new <audio>
342+
let secondPlayer;
343+
renderWithProvider(
344+
<RegisterPlayer onReady={(p) => (secondPlayer = p)} params={params} />,
345+
);
346+
347+
expect(secondPlayer).toBeTruthy();
348+
expect(secondPlayer).not.toBe(firstPlayer);
349+
expect(createdAudios.length).toBe(2);
350+
expect(secondPlayer.elementRef).not.toBe(firstEl);
351+
});
321352
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import type { AudioPlayerPlugin } from './AudioPlayerPlugin';
2+
import type { AudioPlayerPool } from '../AudioPlayerPool';
3+
4+
export const audioPlayerRequestRemovalFromPoolPluginFactory = ({
5+
players,
6+
}: {
7+
players: AudioPlayerPool;
8+
}): AudioPlayerPlugin => ({
9+
id: 'AudioPlayerRequestRemovalFromPoolPlugin',
10+
onRemove: ({ player }) => {
11+
players.pool.delete(player.id);
12+
},
13+
});

src/components/MediaRecorder/AudioRecorder/AudioRecordingPreview.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from 'react';
1+
import React, { useEffect } from 'react';
22
import { PauseIcon, PlayIcon } from '../../MessageInput/icons';
33
import { RecordingTimer } from './RecordingTimer';
44
import { WaveProgressBar } from '../../Attachment';
@@ -36,6 +36,13 @@ export const AudioRecordingPreview = ({
3636

3737
const displayedDuration = secondsElapsed || durationSeconds;
3838

39+
useEffect(
40+
() => () => {
41+
audioPlayer?.requestRemoval();
42+
},
43+
[audioPlayer],
44+
);
45+
3946
if (!audioPlayer) return;
4047

4148
return (

src/components/MediaRecorder/AudioRecorder/__tests__/AudioRecorder.test.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
import { generateDataavailableEvent } from '../../../../mock-builders/browser/events/dataavailable';
3434
import { AudioRecorder } from '../AudioRecorder';
3535
import { MediaRecordingState } from '../../classes';
36+
import { WithAudioPlayback } from '../../../AudioPlayer';
3637

3738
const PERM_DENIED_NOTIFICATION_TEXT =
3839
'To start recording, allow the microphone access in your browser';
@@ -424,13 +425,15 @@ const DEFAULT_RECORDING_CONTROLLER = {
424425
const renderAudioRecorder = (controller = {}) =>
425426
render(
426427
<ChannelActionProvider value={{}}>
427-
<MessageInputContextProvider
428-
value={{
429-
recordingController: { ...DEFAULT_RECORDING_CONTROLLER, ...controller },
430-
}}
431-
>
432-
<AudioRecorder />
433-
</MessageInputContextProvider>
428+
<WithAudioPlayback>
429+
<MessageInputContextProvider
430+
value={{
431+
recordingController: { ...DEFAULT_RECORDING_CONTROLLER, ...controller },
432+
}}
433+
>
434+
<AudioRecorder />
435+
</MessageInputContextProvider>
436+
</WithAudioPlayback>
434437
</ChannelActionProvider>,
435438
);
436439

@@ -463,7 +466,9 @@ describe('AudioRecorder', () => {
463466
recording: generateVoiceRecordingAttachment(),
464467
recordingState: MediaRecordingState.STOPPED,
465468
});
466-
expect(container).toMatchSnapshot();
469+
await waitFor(() => {
470+
expect(container).toMatchSnapshot();
471+
});
467472
});
468473

469474
it.each([MediaRecordingState.PAUSED, MediaRecordingState.RECORDING])(

src/components/MessageInput/AttachmentPreviewList/VoiceRecordingPreview.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from 'react';
1+
import React, { useEffect } from 'react';
22
import { PlayButton } from '../../Attachment';
33
import { RecordingTimer } from '../../MediaRecorder';
44
import { CloseIcon, LoadingIndicatorIcon, RetryIcon } from '../icons';
@@ -33,6 +33,8 @@ export const VoiceRecordingPreview = ({
3333
const { isPlaying, secondsElapsed } =
3434
useStateStore(audioPlayer?.state, audioPlayerStateSelector) ?? {};
3535

36+
useEffect(() => () => audioPlayer?.requestRemoval(), [audioPlayer]);
37+
3638
if (!audioPlayer) return null;
3739

3840
return (

0 commit comments

Comments
 (0)