Skip to content

Commit

Permalink
Fixes multi hook render cycle issue
Browse files Browse the repository at this point in the history
  • Loading branch information
ctrlplusb committed Jul 23, 2019
1 parent 56c871f commit 42dc2e1
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 12 deletions.
47 changes: 47 additions & 0 deletions src/__tests__/use-store-state.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
useStoreState,
StoreProvider,
useStoreActions,
computed,
} from '../index';

class ErrorBoundary extends React.Component {
Expand Down Expand Up @@ -234,3 +235,49 @@ test('does not throw if state is removed', () => {
// assert
expect(getByTestId('todo').textContent).toEqual('ensure hooks work');
});

test('multiple hooks receive state update in same render cycle', () => {
// arrange
const store = createStore({
items: [],
count: computed(state => state.items.length),
fetch: action(state => {
state.items = ['foo'];
}),
});

const renderSpy = jest.fn();

function App() {
const items = useStoreState(state => state.items);
const count = useStoreState(state => state.count);
renderSpy();
return (
<>
<div data-testid="items">{items.join('')}</div>
<div data-testid="count">{count}</div>
</>
);
}

const { getByTestId } = render(
<StoreProvider store={store}>
<App />
</StoreProvider>,
);

// assert
expect(renderSpy).toHaveBeenCalledTimes(1);
expect(getByTestId('items').textContent).toBe('');
expect(getByTestId('count').textContent).toBe('0');

// act
act(() => {
store.getActions().fetch();
});

// assert
expect(renderSpy).toHaveBeenCalledTimes(2);
expect(getByTestId('items').textContent).toBe('foo');
expect(getByTestId('count').textContent).toBe('1');
});
26 changes: 14 additions & 12 deletions src/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,21 @@ const useIsomorphicLayoutEffect =
typeof window !== 'undefined' ? useLayoutEffect : useEffect;

export function createStoreStateHook(Context) {
return function useStoreState(mapState, dependencies = []) {
return function useStoreState(mapState) {
const store = useContext(Context);
const mappedState = useRef();
const mapStateRef = useRef(mapState);
const stateRef = useRef();
const subscriptionMapStateError = useRef();

const [, forceRender] = useReducer(s => s + 1, 0);

useMemo(() => {
mappedState.current = mapState(store.getState());
}, dependencies);

if (subscriptionMapStateError.current || mappedState.current == null) {
if (
stateRef.current == null ||
mapStateRef.current !== mapState ||
subscriptionMapStateError.current
) {
try {
mappedState.current = mapState(store.getState());
stateRef.current = mapState(store.getState());
} catch (err) {
let errorMessage = `An error occurred trying to map state in a useStoreState hook: ${err.message}.`;
if (subscriptionMapStateError.current) {
Expand All @@ -44,17 +45,18 @@ export function createStoreStateHook(Context) {
}

useIsomorphicLayoutEffect(() => {
mapStateRef.current = mapState;
subscriptionMapStateError.current = undefined;
});

useIsomorphicLayoutEffect(() => {
const checkMapState = () => {
try {
const newState = mapState(store.getState());
if (newState === mappedState.current) {
const newState = mapStateRef.current(store.getState());
if (newState === stateRef.current) {
return;
}
mappedState.current = newState;
stateRef.current = newState;
} catch (err) {
// see https://github.com/reduxjs/react-redux/issues/1179
// There is a possibility mapState will fail due to stale state or
Expand All @@ -69,7 +71,7 @@ export function createStoreStateHook(Context) {
return unsubscribe;
}, []);

return mappedState.current;
return stateRef.current;
};
}

Expand Down

0 comments on commit 42dc2e1

Please sign in to comment.