fix(initialArgs): pass initialState arg to initializer, not internal state

[chrisdhanaraj]

Right now when you use the initializer argument, you receive

{
  sideEffects: [],
  state: [].
}

Instead of the second parameter (initialState) like you would in a normal useReducer. This fixes that - I also flipped the logic a little bit in the argument for creating the initState. IMO, putting the early return in there reads cleaner.

Also added in a prettier file - I matched the majority style of the file, although there was actually some discrepancy (sometimes trailing commas, sometimes not).

[jamesplease]

I think this change makes sense. A consequence of it is that the following useReducer init:

useReducer(reducer, initialState, v => v);

which currently would still be v => v, now looks like this:

useReducerWithSideEffects(reducer, initialState, v => Update(v));

but I think that makes sense and would be expected from folks.

👍! Thanks @chrisdhanaraj !

[chrisdhanaraj]

@jamesplease - hmm, are you sure that's the case? Doesn't the mergeState + the return inside the init function here handle actually updating the state?

https://github.com/conorhastings/use-reducer-with-side-effects/pull/33/files#diff-1fdf421c05c1140f6d71444ea2b27638R82

[jamesplease]

Ohhh, I misread what this commit does. You're right, it would still be v => v.

Would this refactor prevent folks from returning side effects in init? I think that's an important case to support, and I regularly do with when using this lib (I opened up #14 to support this use case; more discussion is also in #9 ).

[jamesplease]

Ignore this lol

Misread da code

[chrisdhanaraj]

Ahhh yes, it would - the goal was to mimic the behavior of the original useReducer. I can def see the positives of being able to load up side effects on the initial render though, maybe I'll just wrap my use case to expose only state

[chrisdhanaraj]

I think what I've been doing is useEffect(() => { ... }. [] paired with useReducer - do you just not do that one at all

[jamesplease]

That makes sense.

I could make something like that work, I think, but it wouldn't be as clean as what I'm currently doing. Let me explain my use case a little bit more: I have a few reducers that I expose as libraries. So, in an app, one might do:

// Under-the-hood this calls useReducerWithSideEffect
import useLibReducer from 'lib-reducer';

const [state, dispatch] = useLibReducer(options);

Within these libraries, a pattern I often use is a utility that computes the new state, and the associated side effects, from a set of inputs. So, internally, I might have:

function computeNewState(options) { ... }
// => returns { sideEffects, state }

For each action type, the reducer ultimately call this util, and so does init.

The previous state, new state, and side effects are so tightly linked that I don't think I'd be able to split this out into separate computeNewState and computeNewSideEffects utils. One thing I could do is run the util twice: once in init to get the state, and a second time in useEffect to get the effects. Or I could use closures to pass the effects outside of init to then access them in a useEffect. Neither of these seem as ideal as what I'm currently doing, though, but maybe there are other options I'm not thinking of?

I'm open to suggestions if you have any thoughts on how this could be better, or if you both think the API in this PR is better either way, then I'm OK with that, too!

[chrisdhanaraj]

Nah I like yours! I actually thought this was just a goof, not intentional.