Skip to content

Remove usedevices from uselocaltracks #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timmydoza merged 9 commits into from
May 11, 2021
Merged

Remove usedevices from uselocaltracks #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a4de6cd
Create getDeviceInfo function
May 6, 2021
dad2dcd
Update useDevices to use new getDeviceInfo function
May 6, 2021
dac67fb
Remove usage of useDevices hook from useLocalTracks
May 6, 2021
8f29e7a
Update package-lock to resolve npm vulnerabilities
May 6, 2021
c6e60b4
Update useLocalTracks tests
May 6, 2021
ce9608f
Add comment
May 6, 2021
83e875b
Correct test description
May 6, 2021
fd53113
Update a TS definintion
May 6, 2021
113c140
Merge branch 'master' into remove-usedevices-from-uselocaltracks
May 11, 2021
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
9 changes: 4 additions & 5 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions src/__mocks__/twilio-video.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const mockRoom = new MockRoom();
class MockTrack extends EventEmitter {
kind = '';
stop = jest.fn();
mediaStreamTrack = { getCapabilities: () => ({ deviceId: 'mockDeviceId' }) };

constructor(kind: string) {
super();
Expand All @@ -23,8 +24,11 @@ class MockTrack extends EventEmitter {

const twilioVideo = {
connect: jest.fn(() => Promise.resolve(mockRoom)),
createLocalTracks: jest.fn(() => Promise.resolve([new MockTrack('video'), new MockTrack('audio')])),
createLocalVideoTrack: jest.fn(() => Promise.resolve(new MockTrack('video'))),
createLocalTracks: jest.fn(
// Here we use setTimeout so we can control when this function resolves with jest.runAllTimers()
() => new Promise(resolve => setTimeout(() => resolve([new MockTrack('video'), new MockTrack('audio')])))
),
createLocalVideoTrack: jest.fn(() => new Promise(resolve => setTimeout(() => resolve(new MockTrack('video'))))),
};

export { mockRoom };
Expand Down
106 changes: 62 additions & 44 deletions src/components/VideoProvider/useLocalTracks/useLocalTracks.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,33 @@
import { act, renderHook } from '@testing-library/react-hooks';
import { getDeviceInfo } from '../../../utils';
import { SELECTED_AUDIO_INPUT_KEY, SELECTED_VIDEO_INPUT_KEY, DEFAULT_VIDEO_CONSTRAINTS } from '../../../constants';
import useLocalTracks from './useLocalTracks';
import Video from 'twilio-video';
import useDevices from '../../../hooks/useDevices/useDevices';

jest.mock('../../../hooks/useDevices/useDevices');
const mockUseDevices = useDevices as jest.Mock<any>;
jest.mock('../../../utils');
const mockGetDeviceInfo = getDeviceInfo as jest.Mock<any>;

describe('the useLocalTracks hook', () => {
beforeEach(() => {
Date.now = () => 123456;
mockUseDevices.mockImplementation(() => ({
audioInputDevices: [{ deviceId: 'mockAudioDeviceId', kind: 'audioinput' }],
videoInputDevices: [{ deviceId: 'mockVideoDeviceId', kind: 'videoinput' }],
hasAudioInputDevices: true,
hasVideoInputDevices: true,
}));
mockGetDeviceInfo.mockImplementation(() =>
Promise.resolve({
audioInputDevices: [{ deviceId: 'mockAudioDeviceId', kind: 'audioinput' }],
videoInputDevices: [{ deviceId: 'mockVideoDeviceId', kind: 'videoinput' }],
hasAudioInputDevices: true,
hasVideoInputDevices: true,
})
);
});
afterEach(jest.clearAllMocks);
afterEach(() => window.localStorage.clear());

describe('the getAudioAndVideoTracks function', () => {
it('should create local audio and video tracks', async () => {
const { result, waitForNextUpdate } = renderHook(useLocalTracks);
const { result } = renderHook(useLocalTracks);

await act(async () => {
result.current.getAudioAndVideoTracks();
await waitForNextUpdate();
await result.current.getAudioAndVideoTracks();
});

expect(Video.createLocalTracks).toHaveBeenCalledWith({
Expand All @@ -43,11 +44,10 @@ describe('the useLocalTracks hook', () => {
it('should correctly create local audio and video tracks when selected device IDs are available in localStorage', async () => {
window.localStorage.setItem(SELECTED_VIDEO_INPUT_KEY, 'mockVideoDeviceId');
window.localStorage.setItem(SELECTED_AUDIO_INPUT_KEY, 'mockAudioDeviceId');
const { result, waitForNextUpdate } = renderHook(useLocalTracks);
const { result } = renderHook(useLocalTracks);

await act(async () => {
result.current.getAudioAndVideoTracks();
await waitForNextUpdate();
await result.current.getAudioAndVideoTracks();
});

expect(Video.createLocalTracks).toHaveBeenCalledWith({
Expand All @@ -71,11 +71,10 @@ describe('the useLocalTracks hook', () => {
it('should correctly create local audio and video tracks when selected devices IDs are available in localStorage, but do not correspond to actual devices', async () => {
window.localStorage.setItem(SELECTED_VIDEO_INPUT_KEY, 'otherMockVideoDeviceId');
window.localStorage.setItem(SELECTED_AUDIO_INPUT_KEY, 'otherMockAudioDeviceId');
const { result, waitForNextUpdate } = renderHook(useLocalTracks);
const { result } = renderHook(useLocalTracks);

await act(async () => {
result.current.getAudioAndVideoTracks();
await waitForNextUpdate();
await result.current.getAudioAndVideoTracks();
});

expect(Video.createLocalTracks).toHaveBeenCalledWith({
Expand All @@ -90,18 +89,19 @@ describe('the useLocalTracks hook', () => {
});

it('should create a local audio track when no video devices are present', async () => {
mockUseDevices.mockImplementation(() => ({
audioInputDevices: [{ deviceId: 'mockAudioDeviceId', kind: 'audioinput' }],
videoInputDevices: [],
hasAudioInputDevices: true,
hasVideoInputDevices: false,
}));
mockGetDeviceInfo.mockImplementation(() =>
Promise.resolve({
audioInputDevices: [{ deviceId: 'mockAudioDeviceId', kind: 'audioinput' }],
videoInputDevices: [],
hasAudioInputDevices: true,
hasVideoInputDevices: false,
})
);

const { result, waitForNextUpdate } = renderHook(useLocalTracks);
const { result } = renderHook(useLocalTracks);

await act(async () => {
result.current.getAudioAndVideoTracks();
await waitForNextUpdate();
await result.current.getAudioAndVideoTracks();
});

expect(Video.createLocalTracks).toHaveBeenCalledWith({
Expand All @@ -111,18 +111,19 @@ describe('the useLocalTracks hook', () => {
});

it('should create a local video track when no audio devices are present', async () => {
mockUseDevices.mockImplementation(() => ({
audioInputDevices: [],
videoInputDevices: [{ deviceId: 'mockVideoDeviceId', kind: 'videoinput' }],
hasAudioInputDevices: false,
hasVideoInputDevices: true,
}));
mockGetDeviceInfo.mockImplementation(() =>
Promise.resolve({
audioInputDevices: [],
videoInputDevices: [{ deviceId: 'mockVideoDeviceId', kind: 'videoinput' }],
hasAudioInputDevices: false,
hasVideoInputDevices: true,
})
);

const { result, waitForNextUpdate } = renderHook(useLocalTracks);
const { result } = renderHook(useLocalTracks);

await act(async () => {
result.current.getAudioAndVideoTracks();
await waitForNextUpdate();
await result.current.getAudioAndVideoTracks();
});

expect(Video.createLocalTracks).toHaveBeenCalledWith({
Expand All @@ -137,43 +138,60 @@ describe('the useLocalTracks hook', () => {
});

it('should set isAcquiringLocalTracks to true while acquiring tracks', async () => {
jest.useFakeTimers();
const { result, waitForNextUpdate } = renderHook(useLocalTracks);

expect(result.current.isAcquiringLocalTracks).toBe(false);

act(() => {
await act(async () => {
result.current.getAudioAndVideoTracks();
await waitForNextUpdate();
});

expect(result.current.isAcquiringLocalTracks).toBe(true);

await act(async () => {
jest.runAllTimers();
await waitForNextUpdate();
});

expect(result.current.isAcquiringLocalTracks).toBe(false);
jest.useRealTimers();
});

it('should save the deviceId of the video track to localStorage after it is acquired', async () => {
const { result } = renderHook(useLocalTracks);

await act(async () => {
await result.current.getAudioAndVideoTracks();
});

expect(window.localStorage.getItem(SELECTED_VIDEO_INPUT_KEY)).toBe('mockDeviceId');
});

it('should ignore calls to getAudioAndVideoTracks while isAcquiringLocalTracks is true', async () => {
jest.useFakeTimers();
const { result, waitForNextUpdate } = renderHook(useLocalTracks);

act(() => {
expect(result.current.isAcquiringLocalTracks).toBe(false);
await act(async () => {
result.current.getAudioAndVideoTracks(); // This call is not ignored
await waitForNextUpdate();
});

expect(result.current.isAcquiringLocalTracks).toBe(true);
result.current.getAudioAndVideoTracks(); // This call is ignored

await act(async () => {
jest.runAllTimers();
await waitForNextUpdate();
});

expect(Video.createLocalTracks).toHaveBeenCalledTimes(1);
jest.useRealTimers();
});

it('should not create any tracks when no input devices are present', async () => {
mockUseDevices.mockImplementation(() => []);
mockGetDeviceInfo.mockImplementation(() => Promise.resolve([]));

const { result } = renderHook(useLocalTracks);

Expand All @@ -196,12 +214,12 @@ describe('the useLocalTracks hook', () => {

describe('the removeLocalVideoTrack function', () => {
it('should call videoTrack.stop() and remove the videoTrack from state', async () => {
const { result, waitForNextUpdate } = renderHook(useLocalTracks);
const { result, waitForValueToChange } = renderHook(useLocalTracks);

// First, get tracks
await act(async () => {
result.current.getAudioAndVideoTracks();
await waitForNextUpdate();
await waitForValueToChange(() => result.current.localTracks.length);
Copy link
Contributor

@olipyskoty olipyskoty May 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll need to remember waitForValueToChange(), i haven't seen that before!

Copy link
Contributor Author

@timmydoza timmydoza May 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's nice! I haven't used it much before. There's also this

waitFor(() => result.current.localTracks.length > 0);

It will wait until the condition is true. It only exists in a newer major version of the testing library. I tried upgrading, but it broke things. We can upgrade it at another time.

});

const initialVideoTrack = result.current.localTracks.find(track => track.kind === 'video');
Expand All @@ -219,12 +237,12 @@ describe('the useLocalTracks hook', () => {

describe('the removeLocalAudioTrack function', () => {
it('should call audioTrack.stop() and remove the audioTrack from state', async () => {
const { result, waitForNextUpdate } = renderHook(useLocalTracks);
const { result, waitForValueToChange } = renderHook(useLocalTracks);

// First, get tracks
await act(async () => {
result.current.getAudioAndVideoTracks();
await waitForNextUpdate();
await waitForValueToChange(() => result.current.localTracks.length);
});

const initialAudioTrack = result.current.localTracks.find(track => track.kind === 'audio');
Expand Down
37 changes: 19 additions & 18 deletions src/components/VideoProvider/useLocalTracks/useLocalTracks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { DEFAULT_VIDEO_CONSTRAINTS, SELECTED_AUDIO_INPUT_KEY, SELECTED_VIDEO_INPUT_KEY } from '../../../constants';
import { getDeviceInfo } from '../../../utils';
import { useCallback, useState } from 'react';
import Video, { LocalVideoTrack, LocalAudioTrack, CreateLocalTrackOptions } from 'twilio-video';
import useDevices from '../../../hooks/useDevices/useDevices';

export default function useLocalTracks() {
const [audioTrack, setAudioTrack] = useState<LocalAudioTrack>();
const [videoTrack, setVideoTrack] = useState<LocalVideoTrack>();
const [isAcquiringLocalTracks, setIsAcquiringLocalTracks] = useState(false);
const { audioInputDevices, videoInputDevices, hasAudioInputDevices, hasVideoInputDevices } = useDevices();

const getLocalAudioTrack = useCallback((deviceId?: string) => {
const options: CreateLocalTrackOptions = {};
Expand All @@ -22,9 +21,11 @@ export default function useLocalTracks() {
});
}, []);

const getLocalVideoTrack = useCallback(() => {
const getLocalVideoTrack = useCallback(async () => {
const selectedVideoDeviceId = window.localStorage.getItem(SELECTED_VIDEO_INPUT_KEY);

const { videoInputDevices } = await getDeviceInfo();

const hasSelectedVideoDevice = videoInputDevices.some(
device => selectedVideoDeviceId && device.deviceId === selectedVideoDeviceId
);
Expand All @@ -39,7 +40,7 @@ export default function useLocalTracks() {
setVideoTrack(newTrack);
return newTrack;
});
}, [videoInputDevices]);
}, []);

const removeLocalAudioTrack = useCallback(() => {
if (audioTrack) {
Expand All @@ -55,7 +56,9 @@ export default function useLocalTracks() {
}
}, [videoTrack]);

const getAudioAndVideoTracks = useCallback(() => {
const getAudioAndVideoTracks = useCallback(async () => {
const { audioInputDevices, videoInputDevices, hasAudioInputDevices, hasVideoInputDevices } = await getDeviceInfo();

if (!hasAudioInputDevices && !hasVideoInputDevices) return Promise.resolve();
if (isAcquiringLocalTracks || audioTrack || videoTrack) return Promise.resolve();

Expand All @@ -82,25 +85,23 @@ export default function useLocalTracks() {

return Video.createLocalTracks(localTrackConstraints)
.then(tracks => {
const newVideoTrack = tracks.find(track => track.kind === 'video');
const newAudioTrack = tracks.find(track => track.kind === 'audio');
const newVideoTrack = tracks.find(track => track.kind === 'video') as LocalVideoTrack;
const newAudioTrack = tracks.find(track => track.kind === 'audio') as LocalAudioTrack;
if (newVideoTrack) {
setVideoTrack(newVideoTrack as LocalVideoTrack);
setVideoTrack(newVideoTrack);
// Save the deviceId so it can be picked up by the VideoInputList component. This only matters
// in cases where the user's video is disabled.
window.localStorage.setItem(
SELECTED_VIDEO_INPUT_KEY,
newVideoTrack.mediaStreamTrack.getCapabilities().deviceId ?? ''
);
}
if (newAudioTrack) {
setAudioTrack(newAudioTrack as LocalAudioTrack);
setAudioTrack(newAudioTrack);
}
})
.finally(() => setIsAcquiringLocalTracks(false));
}, [
hasAudioInputDevices,
hasVideoInputDevices,
audioTrack,
videoTrack,
audioInputDevices,
videoInputDevices,
isAcquiringLocalTracks,
]);
}, [audioTrack, videoTrack, isAcquiringLocalTracks]);

const localTracks = [audioTrack, videoTrack].filter(track => track !== undefined) as (
| LocalAudioTrack
Expand Down
Loading