Skip to content

Commit

Permalink
Make listen and stop get latest state/props (#32)
Browse files Browse the repository at this point in the history
Issue #31: `listen` and `stop` were being regenerated on every render
so if you called them from inside a closure, you could end up using a
stale version that referenced old values of state and props. This led
to confusing behavior when calling `listen` or `stop` from inside a
closure, e.g.

    const { listen, stop } = useSpeechRecognition();

    const handleClick = () => {
      setTimeout(stop, 1000);
      listen();
    };

    return <button onClick={handleClick}>Listen for 1 second</button>;

Clicking the button would listen indefinitely because the version of
`stop` inside the `handleClick` callback referenced the old value of
`listening`, before `listen` was called.

This commit makes it so `listen` and `stop` always reference the latest
values of state and props so they can be safely called from within
closures.
  • Loading branch information
maxwellhaydn authored Jul 4, 2020
1 parent 9c09578 commit e6db84e
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 5 deletions.
39 changes: 34 additions & 5 deletions src/useSpeechRecognition.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,33 @@
import { useRef, useEffect, useState } from 'react';
import { useRef, useEffect, useState, useCallback } from 'react';

/**
* Custom hook similar to useCallback, but for callbacks where the dependencies
* change frequently. Ensures that references to state and props inside the
* callback always get the latest values. Used to keep the `listen` and `stop`
* functions in sync with the latest values of the `listening` and `supported`
* state variables. See this issue for an example of why this is needed:
*
* https://github.com/MikeyParton/react-speech-kit/issues/31
*
* Implementation taken from "How to read an often-changing value from
* useCallback?" in the React hooks API reference:
*
* https://reactjs.org/docs/hooks-faq.html#how-to-read-an-often-changing-value-from-usecallback
*/
const useEventCallback = (fn, dependencies) => {
const ref = useRef(() => {
throw new Error('Cannot call an event handler while rendering.');
});

useEffect(() => {
ref.current = fn;
}, [fn, ...dependencies]);

return useCallback((args) => {
const fn = ref.current;
return fn(args);
}, [ref]);
};

const useSpeechRecognition = (props = {}) => {
const { onEnd = () => {}, onResult = () => {}, onError = () => {} } = props;
Expand All @@ -23,7 +52,7 @@ const useSpeechRecognition = (props = {}) => {
onError(event);
};

const listen = (args = {}) => {
const listen = useEventCallback((args = {}) => {
if (listening || !supported) return;
const {
lang = '',
Expand All @@ -46,17 +75,17 @@ const useSpeechRecognition = (props = {}) => {
// We want it to keep going until we tell it to stop
recognition.current.onend = () => recognition.current.start();
recognition.current.start();
};
}, [listening, supported, recognition]);

const stop = () => {
const stop = useEventCallback(() => {
if (!listening || !supported) return;
recognition.current.onresult = () => {};
recognition.current.onend = () => {};
recognition.current.onerror = () => {};
setListening(false);
recognition.current.stop();
onEnd();
};
}, [listening, supported, recognition, onEnd]);

useEffect(() => {
if (typeof window === 'undefined') return;
Expand Down
61 changes: 61 additions & 0 deletions test/useSpeechRecognition.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,26 @@ describe('useSpeechRecognition', () => {
return <TestComponent {...props} />;
};

// A test component with `listen` and `stop` in closures to test that they
// reference the latest value of `listening`
const ClosureExample = () => {
const props = useSpeechRecognition({
onResult: mockOnResult,
onEnd: mockOnEnd,
onError: mockOnError,
});

props.listenHandler = () => {
setTimeout(() => props.listen(), 1000);
};

props.stopHandler = () => {
setTimeout(() => props.stop(), 1000);
};

return <TestComponent {...props} />;
};

beforeEach(() => {
jest.clearAllTimers();
jest.clearAllMocks();
Expand Down Expand Up @@ -122,6 +142,25 @@ describe('useSpeechRecognition', () => {
expect(MockRecognition.start.mock.calls.length).toBe(1);
});
});

describe('when already listening, in a closure', () => {
it('does not call start on the window.speechSynthesis instance again', () => {
jest.clearAllMocks();
const wrapper = mount(<ClosureExample />);
act(() => {
wrapper.find(TestComponent).props().listen();
wrapper.find(TestComponent).props().listenHandler();
});

wrapper.update();
act(() => {
jest.advanceTimersByTime(1000);
});

wrapper.update();
expect(MockRecognition.start.mock.calls.length).toBe(1);
});
});
});

describe('stop()', () => {
Expand Down Expand Up @@ -154,5 +193,27 @@ describe('useSpeechRecognition', () => {
expect(mockOnEnd.mock.calls.length).toBe(0);
});
});

describe('while listening, in a closure', () => {
it('calls stop on the window.speechSynthesis instance and the provided onEnd prop, then passes listening: false', () => {

const wrapper = mount(<ClosureExample />);
act(() => {
wrapper.find(TestComponent).props().listen();
wrapper.find(TestComponent).props().stopHandler();
});

wrapper.update();

act(() => {
jest.advanceTimersByTime(1000);
});

wrapper.update();
expect(MockRecognition.stop.mock.calls.length).toBe(1);
expect(mockOnEnd.mock.calls.length).toBe(1);
expect(wrapper.find(TestComponent).props().listening).toBe(false);
});
});
});
});

0 comments on commit e6db84e

Please sign in to comment.