Skip to content

Commit dd3dac4

Browse files
committed
refactor: replace ReactiveVar in VideoRecorder with Emitter + hook
VideoRecorder used meteor/reactive-var for cameraStarted, but the React consumer never subscribed reactively (no useTracker / useSyncExternalStore), so the send button only re-evaluated its disabled state by accident. Drop meteor/reactive-var from this module and restore reactivity using @rocket.chat/emitter — the pattern already used elsewhere in the client — plus a useSyncExternalStore-based useVideoRecorderCameraStarted hook so VideoMessageRecorder re-renders when the camera starts/stops. Private recording/recordingAvailable flags become plain booleans since they were only read internally.
1 parent 0d68957 commit dd3dac4

3 files changed

Lines changed: 44 additions & 32 deletions

File tree

apps/meteor/app/ui/client/lib/recorderjs/videoRecorder.spec.ts

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,6 @@
11
import { VideoRecorder } from './videoRecorder';
22
import { createDeferredPromise } from '../../../../../tests/mocks/utils/createDeferredMockFn';
33

4-
jest.mock('meteor/reactive-var', () => ({
5-
ReactiveVar: jest.fn().mockImplementation((initialValue) => {
6-
let value = initialValue;
7-
return {
8-
get: jest.fn(() => value),
9-
set: jest.fn((newValue) => {
10-
value = newValue;
11-
}),
12-
};
13-
}),
14-
}));
15-
164
describe('VideoRecorder', () => {
175
let mockStream: MediaStream;
186
let mockVideoTrack: MediaStreamTrack;
@@ -93,7 +81,7 @@ describe('VideoRecorder', () => {
9381
streamDeferred.resolve(mockStream);
9482
await jest.runAllTimersAsync();
9583

96-
expect(VideoRecorder.cameraStarted.get()).toBe(false);
84+
expect(VideoRecorder.cameraStarted).toBe(false);
9785
});
9886

9987
it('should handle multiple start/stop cycles', async () => {
@@ -115,7 +103,7 @@ describe('VideoRecorder', () => {
115103
await jest.runAllTimersAsync();
116104

117105
expect(cb).toHaveBeenCalledWith(true);
118-
expect(VideoRecorder.cameraStarted.get()).toBe(true);
106+
expect(VideoRecorder.cameraStarted).toBe(true);
119107
});
120108

121109
it('should invalidate pending callbacks from previous start when new start is called', async () => {
@@ -155,19 +143,19 @@ describe('VideoRecorder', () => {
155143
await jest.runAllTimersAsync();
156144

157145
expect(cb).toHaveBeenCalledWith(true);
158-
expect(VideoRecorder.cameraStarted.get()).toBe(true);
146+
expect(VideoRecorder.cameraStarted).toBe(true);
159147
});
160148

161149
it('should stop camera tracks', () => {
162150
(VideoRecorder as any).stream = mockStream;
163151
(VideoRecorder as any).started = true;
164-
VideoRecorder.cameraStarted.set(true);
152+
(VideoRecorder as any)._cameraStarted = true;
165153

166154
VideoRecorder.stop();
167155

168156
expect(mockVideoTrack.stop).toHaveBeenCalled();
169157
expect(mockAudioTrack.stop).toHaveBeenCalled();
170-
expect(VideoRecorder.cameraStarted.get()).toBe(false);
158+
expect(VideoRecorder.cameraStarted).toBe(false);
171159
});
172160

173161
it('should return supported mime types', () => {

apps/meteor/app/ui/client/lib/recorderjs/videoRecorder.ts

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
1-
import { ReactiveVar } from 'meteor/reactive-var';
1+
import { Emitter } from '@rocket.chat/emitter';
2+
import { useCallback, useSyncExternalStore } from 'react';
23

3-
class VideoRecorder {
4-
public cameraStarted = new ReactiveVar(false);
4+
type VideoRecorderEvents = {
5+
cameraStartedChange: boolean;
6+
};
7+
8+
class VideoRecorder extends Emitter<VideoRecorderEvents> {
9+
private _cameraStarted = false;
510

611
private started = false;
712

8-
private recording = new ReactiveVar(false);
13+
private recording = false;
914

10-
private recordingAvailable = new ReactiveVar(false);
15+
private recordingAvailable = false;
1116

1217
private videoel: HTMLVideoElement | undefined;
1318

@@ -19,6 +24,18 @@ class VideoRecorder {
1924

2025
private sessionId = 0;
2126

27+
public get cameraStarted(): boolean {
28+
return this._cameraStarted;
29+
}
30+
31+
private setCameraStarted(value: boolean) {
32+
if (this._cameraStarted === value) {
33+
return;
34+
}
35+
this._cameraStarted = value;
36+
this.emit('cameraStartedChange', value);
37+
}
38+
2239
public getSupportedMimeTypes() {
2340
if (window.MediaRecorder.isTypeSupported('video/webm')) {
2441
return 'video/webm; codecs=vp8,opus';
@@ -71,12 +88,12 @@ class VideoRecorder {
7188
this.mediaRecorder = new MediaRecorder(this.stream, { mimeType: this.getSupportedMimeTypes() });
7289
this.mediaRecorder.ondataavailable = (blobev) => {
7390
this.chunks.push(blobev.data);
74-
if (!this.recordingAvailable.get()) {
75-
return this.recordingAvailable.set(true);
91+
if (!this.recordingAvailable) {
92+
this.recordingAvailable = true;
7693
}
7794
};
7895
this.mediaRecorder.start();
79-
return this.recording.set(true);
96+
this.recording = true;
8097
}
8198

8299
private stopStreamTracks(stream: MediaStream) {
@@ -109,7 +126,7 @@ class VideoRecorder {
109126
}
110127

111128
this.started = true;
112-
return this.cameraStarted.set(true);
129+
this.setCameraStarted(true);
113130
}
114131

115132
public stop(cb?: (blob: Blob) => void) {
@@ -128,8 +145,8 @@ class VideoRecorder {
128145

129146
const wasStarted = this.started;
130147
this.started = false;
131-
this.cameraStarted.set(false);
132-
this.recordingAvailable.set(false);
148+
this.setCameraStarted(false);
149+
this.recordingAvailable = false;
133150

134151
if (cb && this.chunks && wasStarted) {
135152
const blob = new Blob(this.chunks);
@@ -141,16 +158,22 @@ class VideoRecorder {
141158
}
142159

143160
public stopRecording() {
144-
if (!this.started || !this.recording || !this.mediaRecorder) {
161+
if (!this.started || !this.mediaRecorder) {
145162
return;
146163
}
147164

148165
this.mediaRecorder.stop();
149-
this.recording.set(false);
166+
this.recording = false;
150167
delete this.mediaRecorder;
151168
}
152169
}
153170

154171
const instance = new VideoRecorder();
155172

156173
export { instance as VideoRecorder };
174+
175+
export const useVideoRecorderCameraStarted = (): boolean =>
176+
useSyncExternalStore(
177+
useCallback((onStoreChange) => instance.on('cameraStartedChange', onStoreChange), []),
178+
() => instance.cameraStarted,
179+
);

apps/meteor/client/views/composer/VideoMessageRecorder/VideoMessageRecorder.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type { AllHTMLAttributes, RefObject } from 'react';
77
import { useRef, useEffect, useState } from 'react';
88

99
import { UserAction, USER_ACTIVITIES } from '../../../../app/ui/client/lib/UserAction';
10-
import { VideoRecorder } from '../../../../app/ui/client/lib/recorderjs/videoRecorder';
10+
import { VideoRecorder, useVideoRecorderCameraStarted } from '../../../../app/ui/client/lib/recorderjs/videoRecorder';
1111
import { useChat } from '../../room/contexts/ChatContext';
1212

1313
type VideoMessageRecorderProps = {
@@ -45,7 +45,8 @@ const VideoMessageRecorder = ({ rid, tmid, reference }: VideoMessageRecorderProp
4545
const [recordingState, setRecordingState] = useState<'idle' | 'loading' | 'recording'>('idle');
4646
const [recordingInterval, setRecordingInterval] = useState<ReturnType<typeof setInterval> | null>(null);
4747
const isRecording = recordingState === 'recording';
48-
const sendButtonDisabled = !(VideoRecorder.cameraStarted.get() && !(recordingState === 'recording'));
48+
const cameraStarted = useVideoRecorderCameraStarted();
49+
const sendButtonDisabled = !(cameraStarted && !isRecording);
4950

5051
const chat = useChat();
5152

0 commit comments

Comments
 (0)