Skip to content
This repository has been archived by the owner on Nov 10, 2021. It is now read-only.

Commit

Permalink
Don't run stale listeners (#22)
Browse files Browse the repository at this point in the history
Per the [Redux docs](https://redux.js.org/api/store#subscribe):

> The subscriptions are snapshotted just before every dispatch() call. If you subscribe or unsubscribe while the listeners are being invoked, this will not have any effect on the dispatch() that is currently in progress.

Since a component can be unmounted during dispatch, and Redux won't remove the listener until afterward, we need to manually guard against it being called after unsubscribing.
  • Loading branch information
gaearon authored and ianobermiller committed Feb 1, 2019
1 parent 61832d8 commit 8cab489
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
27 changes: 27 additions & 0 deletions src/__tests__/index-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as React from 'react';
import * as ReactDOM from 'react-dom';
import {Store} from 'redux';
import {StoreContext, useMappedState} from '..';
import {isatty} from 'tty';

interface IAction {
type: 'add todo';
Expand Down Expand Up @@ -170,6 +171,32 @@ describe('redux-react-hook', () => {

expect(getText()).toBe('foo 45');
});

it("doesn't try to update after unmounting during dispatch", () => {
let mapStateCalls = 0;
const Component = () => {
const mapState = React.useCallback((s: IState) => {
mapStateCalls++;
return s.foo;
}, []);
const foo = useMappedState(mapState);
return <div>{foo}</div>;
};

render(<Component />);

flushEffects();

ReactDOM.unmountComponentAtNode(reactRoot);

const consoleErrorSpy = jest.spyOn(console, 'error');
state = {...state, foo: 'foo'};
subscriberCallback();

// mapState is called during and after the first render
expect(mapStateCalls).toBe(2);
expect(consoleErrorSpy).not.toHaveBeenCalled();
});
});

// https://github.com/kentcdodds/react-testing-library/commit/11a41ce3ad9e9695f4b1662a5c67b890fc304894
Expand Down
16 changes: 14 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,36 @@ export function useMappedState<TState, TResult>(

useEffect(
() => {
let didUnsubscribe = false;

// Run the mapState callback and if the result has changed, make the
// component re-render with the new state.
const checkForUpdates = () => {
if (didUnsubscribe) {
// Don't run stale listeners.
// Redux doesn't guarantee unsubscriptions happen until next dispatch.
return;
}

const newDerivedState = runMapState();
if (!shallowEqual(newDerivedState, lastRenderedDerivedState.current)) {
setDerivedState(newDerivedState);
}
};

// Pull data from the store on first render.
// Pull data from the store after first render in case the store has
// changed since we began.
checkForUpdates();

// Subscribe to the store to be notified of subsequent changes.
const unsubscribe = store.subscribe(checkForUpdates);

// The return value of useEffect will be called when unmounting, so
// we use it to unsubscribe from the store.
return unsubscribe;
return () => {
didUnsubscribe = true;
unsubscribe();
};
},
[store, mapState],
);
Expand Down

0 comments on commit 8cab489

Please sign in to comment.