memoize the wrappedReducer function

[chrisdhanaraj]

Resolves #18

Essentially matches the useMemo/useCallback usage that the other wrapped functions (initialStateAndEffects / wrappedDispatch) already have

[davidkpiano]

Thanks for this! Can you add a quick test to demonstrate the fixed behavior?

[chrisdhanaraj]

Okay, so this is somewhat of a goof on me. My solve does fix the problem I was having, but it could also be fixed in user land. I wrote a test for this and did some variations and saw my error - here's the codesandbox with all three different variations.

https://codesandbox.io/s/red-https-bxpo0?file=/src/App.tsx

Basically, boils down to using a Map inside the state object. Starting from a state shape that looks like

interface State {
  selectedItems: Map<string, string[]>
}

and an acton that adds an item into an array, if...

1. When you handle the event in the reducer, if you operate on the original map in state, then create a new Map and use that in the returned state, you run into the double dispatch problem.

2. But memoing the wrapped reducer in the useEffectReducer solves this, as I believe this keeps the identity the same. This matches how you would solve the same issue in useReducer (i.e., either memo the reducer function or move it out of the React component).

3. But but you can also avoid the double dispatch problem by handling Maps better. Instead of operating on the original Map and then cloning as you updated state, you can instead clone the original Map then operate on it. If you do that, I believe you still maintain identity and the double dispatch doesn't happen.

I can update the PR with the test that checks this, but I'm not sure if you want something that could be fixed in user land here?

[ssorallen]

I think the example of mutating the Map differently happens to get around the bug that is caused by passing a new reducer to useReducer on each render. The bug still exists, although I can't track down exactly why the "CorrectMap" example works.

The reducer not being memoized is definitely causing a double dispatch for me too, it's a side effect of passing a different function to useReducer on each render.

This PR should include effectReducer in the dependency list, and that actually surfaces an issue in the Codesandbox: the reducer function passed to useEffectReducer must either be a module-level variable or be useMemo'd itself otherwise each render creates a new reducer function and gets back into this bug.

Updated Codesandbox with the reducer moved to a module function and with effectReducer added to the deps array: https://codesandbox.io/s/elated-williams-fzbmg

[chrisdhanaraj]

@davidkpiano - hello! I know it's been a while, but checking to see if there was something I could do to get this merged. I swapped to this version in an internal codebase and one thing that probably should be called out is the need to memoize the effectsMap object in userland as well.

i.e., now that the useMemo has the [effectsMap..] in it, users should be doing something like

const effectsMap = useMemo(() => {
 return {}
}, [whateverDepsRequired])