Skip to content

Commit b92891f

Browse files
authored
Disable camera when not in use (#940)
* Disabled camera when not in use * Added useSafeTimeout custom hook * Fixed memory leak from setTimeout
1 parent fc9a463 commit b92891f

File tree

9 files changed

+277
-1
lines changed

9 files changed

+277
-1
lines changed

configs/test-utils/src/__mocks__/@monkvision/common.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,4 +147,5 @@ export = {
147147
alpha: 0,
148148
requestCompassPermission: jest.fn(() => Promise.resolve()),
149149
})),
150+
useSafeTimeout: jest.fn(() => jest.fn()),
150151
};

packages/camera-web/src/Camera/hooks/useUserMedia.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ export function useUserMedia(
203203
constraints: MediaStreamConstraints,
204204
videoRef: RefObject<HTMLVideoElement> | null,
205205
): UserMediaResult {
206+
const streamRef = useRef<MediaStream | null>(null);
206207
const [stream, setStream] = useState<MediaStream | null>(null);
207208
const [dimensions, setDimensions] = useState<PixelDimensions | null>(null);
208209
const [isLoading, setIsLoading] = useState(false);
@@ -276,6 +277,7 @@ export function useUserMedia(
276277
str?.addEventListener('inactive', onStreamInactive);
277278
if (isMounted()) {
278279
setStream(str);
280+
streamRef.current = str;
279281
setDimensions(getStreamDimensions(str, true));
280282
setIsLoading(false);
281283
setAvailableCameraDevices(deviceDetails.availableDevices);
@@ -332,6 +334,14 @@ export function useUserMedia(
332334
}
333335
}, [stream, videoRef]);
334336

337+
useEffect(() => {
338+
return () => {
339+
streamRef.current?.getTracks().forEach((track) => {
340+
track.stop();
341+
});
342+
};
343+
}, []);
344+
335345
return useObjectMemo({
336346
getUserMedia,
337347
stream,

packages/camera-web/test/Camera/hooks/useUserMedia.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,4 +528,64 @@ describe('useUserMedia hook', () => {
528528
});
529529
unmount();
530530
});
531+
532+
it('should stop all tracks when the component unmounts', async () => {
533+
const videoRef = { current: {} } as RefObject<HTMLVideoElement>;
534+
const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef });
535+
536+
await waitFor(() => {
537+
expect(result.current.stream).toEqual(gumMock?.stream);
538+
});
539+
540+
expect(gumMock?.tracks).toHaveLength(1);
541+
expect(gumMock?.tracks[0].stop).toBeDefined();
542+
543+
unmount();
544+
545+
gumMock?.tracks.forEach((track) => {
546+
expect(track.stop).toHaveBeenCalledTimes(1);
547+
});
548+
});
549+
550+
it('should stop tracks when unmounting even if stream is null', () => {
551+
const videoRef = { current: {} } as RefObject<HTMLVideoElement>;
552+
553+
// Create a mock stream that returns null for getTracks
554+
const nullStreamMock = {
555+
getTracks: jest.fn(() => null),
556+
addEventListener: jest.fn(),
557+
removeEventListener: jest.fn(),
558+
} as unknown as MediaStream;
559+
560+
mockGetUserMedia({
561+
createMock: () => jest.fn(() => Promise.resolve(nullStreamMock)),
562+
});
563+
564+
const { unmount } = renderUseUserMedia({ constraints: {}, videoRef });
565+
566+
// Unmount should not throw even with null tracks
567+
expect(() => unmount()).not.toThrow();
568+
});
569+
570+
it('should call stop on each track exactly once during cleanup', async () => {
571+
const videoRef = { current: {} } as RefObject<HTMLVideoElement>;
572+
const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef });
573+
574+
await waitFor(() => {
575+
expect(result.current.stream).toEqual(gumMock?.stream);
576+
});
577+
578+
// Verify initial state - no tracks stopped yet
579+
gumMock?.tracks.forEach((track) => {
580+
expect(track.stop).not.toHaveBeenCalled();
581+
});
582+
583+
// Unmount the component
584+
unmount();
585+
586+
// Verify each track was stopped exactly once
587+
gumMock?.tracks.forEach((track) => {
588+
expect(track.stop).toHaveBeenCalledTimes(1);
589+
});
590+
});
531591
});

packages/common-ui-web/src/components/TakePictureButton/hooks.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { InteractiveStatus } from '@monkvision/types';
22
import { CSSProperties, useState } from 'react';
3+
import { useSafeTimeout } from '@monkvision/common';
34
import { styles, TAKE_PICTURE_BUTTON_COLORS } from './TakePictureButton.styles';
45

56
/**
@@ -33,11 +34,12 @@ export function useTakePictureButtonStyle(
3334
params: MonkTakePictureButtonStyleParams,
3435
): TakePictureButtonStyles {
3536
const [isPressed, setIsPressed] = useState(false);
37+
const setSafeTimeout = useSafeTimeout();
3638
const borderWidth = (params.size * (1 - INNER_BUTTON_SIZE_RATIO)) / 4;
3739

3840
const animateClick = () => {
3941
setIsPressed(true);
40-
setTimeout(() => setIsPressed(false), PRESS_ANIMATION_DURATION_MS);
42+
setSafeTimeout(() => setIsPressed(false), PRESS_ANIMATION_DURATION_MS);
4143
};
4244

4345
const buttonStyles = {

packages/common-ui-web/test/components/TakePictureButton.test.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
jest.mock('@monkvision/common');
2+
13
import { fireEvent, render, screen } from '@testing-library/react';
24
import {
35
expectComponentToPassDownClassNameToHTMLElement,

packages/common/README/HOOKS.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,19 @@ This hook returns takes a `ResponsiveStyleProperties` declarations object (see t
265265
`@monkvision/types` package for more details) containing a media query and returns either the CSSProperties contained in
266266
the type, or `null` if the query conditions are not met. Note that if there are no query, the style will be applied.
267267

268+
269+
### useSafeTimeout
270+
```tsx
271+
import { sights } from '@monkvision/sights';
272+
import { useSafeTimeout } from '@monkvision/common';
273+
274+
function TestComponent() {
275+
const setSafeTimeout = useSafeTimeout();
276+
setSafeTimeout(() => console.log('test'), 1000);
277+
}
278+
```
279+
Custom hook that provides a safe way to use setTimeout.
280+
268281
### useSearchParams
269282
```tsx
270283
import { sights } from '@monkvision/sights';

packages/common/src/hooks/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ export * from './useObjectMemo';
1313
export * from './useForm';
1414
export * from './useIsMounted';
1515
export * from './useDeviceOrientation';
16+
export * from './useSafeTimeout';
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { useCallback, useEffect, useRef } from 'react';
2+
3+
/**
4+
* A custom hook that provides a safe way to use setTimeout.
5+
*/
6+
export function useSafeTimeout() {
7+
const isMounted = useRef(true);
8+
const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
9+
10+
useEffect(() => {
11+
return () => {
12+
isMounted.current = false;
13+
if (timeoutRef.current) {
14+
clearTimeout(timeoutRef.current);
15+
}
16+
};
17+
}, []);
18+
19+
return useCallback((callback: () => void, delay: number) => {
20+
timeoutRef.current = setTimeout(() => {
21+
if (isMounted.current) {
22+
callback();
23+
}
24+
}, delay);
25+
}, []);
26+
}
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
import { renderHook, act } from '@testing-library/react-hooks';
2+
import { useSafeTimeout } from '../../src';
3+
4+
jest.useFakeTimers();
5+
6+
describe('useSafeTimeout hook', () => {
7+
afterEach(() => {
8+
jest.clearAllTimers();
9+
});
10+
11+
afterAll(() => {
12+
jest.useRealTimers();
13+
});
14+
15+
it('should execute callback after specified delay', () => {
16+
const { result } = renderHook(() => useSafeTimeout());
17+
const callback = jest.fn();
18+
19+
act(() => {
20+
result.current(callback, 1000);
21+
});
22+
23+
expect(callback).not.toHaveBeenCalled();
24+
25+
act(() => {
26+
jest.advanceTimersByTime(1000);
27+
});
28+
29+
expect(callback).toHaveBeenCalledTimes(1);
30+
});
31+
32+
it('should not execute callback if component unmounts before delay', () => {
33+
const { result, unmount } = renderHook(() => useSafeTimeout());
34+
const callback = jest.fn();
35+
36+
act(() => {
37+
result.current(callback, 1000);
38+
});
39+
40+
unmount();
41+
42+
act(() => {
43+
jest.advanceTimersByTime(1000);
44+
});
45+
46+
expect(callback).not.toHaveBeenCalled();
47+
});
48+
49+
it('should allow multiple timeouts to run if not overridden', () => {
50+
const { result } = renderHook(() => useSafeTimeout());
51+
const callback1 = jest.fn();
52+
const callback2 = jest.fn();
53+
54+
act(() => {
55+
result.current(callback1, 1000);
56+
});
57+
58+
act(() => {
59+
result.current(callback2, 500);
60+
});
61+
62+
act(() => {
63+
jest.advanceTimersByTime(500);
64+
});
65+
66+
expect(callback2).toHaveBeenCalledTimes(1);
67+
expect(callback1).not.toHaveBeenCalled();
68+
69+
act(() => {
70+
jest.advanceTimersByTime(500);
71+
});
72+
73+
expect(callback1).toHaveBeenCalledTimes(1);
74+
expect(callback2).toHaveBeenCalledTimes(1);
75+
});
76+
77+
it('should handle multiple timeouts in sequence', () => {
78+
const { result } = renderHook(() => useSafeTimeout());
79+
const callback1 = jest.fn();
80+
const callback2 = jest.fn();
81+
82+
act(() => {
83+
result.current(callback1, 500);
84+
});
85+
86+
act(() => {
87+
jest.advanceTimersByTime(500);
88+
});
89+
90+
expect(callback1).toHaveBeenCalledTimes(1);
91+
92+
act(() => {
93+
result.current(callback2, 300);
94+
});
95+
96+
act(() => {
97+
jest.advanceTimersByTime(300);
98+
});
99+
100+
expect(callback2).toHaveBeenCalledTimes(1);
101+
});
102+
103+
it('should handle zero delay', () => {
104+
const { result } = renderHook(() => useSafeTimeout());
105+
const callback = jest.fn();
106+
107+
act(() => {
108+
result.current(callback, 0);
109+
});
110+
111+
act(() => {
112+
jest.advanceTimersByTime(0);
113+
});
114+
115+
expect(callback).toHaveBeenCalledTimes(1);
116+
});
117+
118+
it('should clean up timeout on unmount', () => {
119+
const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout');
120+
const { result, unmount } = renderHook(() => useSafeTimeout());
121+
const callback = jest.fn();
122+
123+
act(() => {
124+
result.current(callback, 1000);
125+
});
126+
127+
unmount();
128+
129+
expect(clearTimeoutSpy).toHaveBeenCalled();
130+
clearTimeoutSpy.mockRestore();
131+
});
132+
133+
it('should return the same function reference across renders', () => {
134+
const { result, rerender } = renderHook(() => useSafeTimeout());
135+
const firstReference = result.current;
136+
137+
rerender();
138+
const secondReference = result.current;
139+
140+
expect(firstReference).toBe(secondReference);
141+
});
142+
143+
it('should handle callback that throws an error', () => {
144+
const { result } = renderHook(() => useSafeTimeout());
145+
const errorCallback = jest.fn(() => {
146+
throw new Error('Test error');
147+
});
148+
149+
act(() => {
150+
result.current(errorCallback, 100);
151+
});
152+
153+
expect(() => {
154+
act(() => {
155+
jest.advanceTimersByTime(100);
156+
});
157+
}).toThrow('Test error');
158+
159+
expect(errorCallback).toHaveBeenCalledTimes(1);
160+
});
161+
});

0 commit comments

Comments
 (0)