upcastingReducer example doesn't show the need for an initial state

[dcurletti]

The example in the Readme of the upcastingReducer doesn't show that the final "reducer" should have an initial state passed to it in order for it not to error.

Reason: someone copy and pasting that example and exporting the final function as the reducer will get an error when they try to run the code or test it.

I can submit a PR for this later today, but the fix is just creating an INITIAL_STATE variable that is given as the default to the final "reducer".

[dphilipson]

Hi @dcurletti, thanks very much for opening this issue. You're totally right that the example is broken. I have some somewhat rambling thoughts about how to handle it.

I think this is a docs problem, not a library problem. The only thing wrong with the upcasting reducer example is that the function reducer in the example no longer satisfies the Reducer interface after Redux changed it at some point (much more detail below). Simply changing the documentation so that the function reducer() has an initial state will fix the docs, and this isn't a problem in practice because the incorrect example will fail typechecking when initializing the store or calling combineReducers with it.

I'm not sure that giving upcastingReducer an initial state is the right fix. It's really hard to make typings with regard to initial state that make everyone happy. Here's a short history of the issues involved:

• Originally, the official Redux typings declared a reducer as (State, Action) => State, without allowing for the possibility of passing undefined state, and this library followed suit with the same definition. This was a technically incorrect definition, since reducers need to handle undefined as being initial state in the standard Redux usage, so users could define functions satisfying the Reducer type which could not actually be used with Redux. However, this definition has the advantage for this library that both reducerWithInitialState(initialState) and reducerWithoutInitialState() could return the type Reducer and soundly satisfy its type definition. See the former definition here.

• At some point, the official Redux typings changed to define a reducer as (State?, Action) => State, as in the current definition as of today. This is more correct, but unfortunately makes reducerWithoutInitialState awkward for this library, as it can no longer honestly say it is returning a function meeting the Reducer interface. This gives this library a choice:

  • We could continue to claim that reducerWithoutInitialState returns a Reducer. This is unsound, because if passed to a store which does not have an initial state then it will fail.
  • We could truthfully say that the function it returns does not accept undefined state. This makes it type-incompatible with Redux, which defeats the point of this library.

You might ask, why have reducerWithoutInitialState() at all in that case? It's actually a pretty useful way to structure some apps. In many applications, the initial state depends on some environmental factors, such as cookies or local settings, or perhaps it doesn't make sense to create the store at all until certain data is retrieved from the server. In this case, it doesn't make sense for "dumb" reducers to produce the same initial state every time- it makes more sense to programatically construct an initial state and pass it to your store as you create the store. The Redux authors have argued that putting the initial state in the reducers is better because it ensures that the shape of the state matches the shape of the reducers, but one might argue that we don't have to worry about that, because we use TypeScript!

Because of this, despite the typings around reducerWithoutInitialState being unsound, the function is still pragmatically useful and so will remain in the library.

Back to the original point: as Reducers are now required to accept undefined as their first argument, the example written in the README will not typecheck and so this isn't a problem with the library but with the docs. I'll change the docs and consider this fixed, unless you have an argument for why making upcastingReducer take an initial state is the right solution instead.

[dcurletti]

Will leave a longer response as to why tomorrow, but I agree that upcastingReducer shouldn't take an initial state.

Yea, sorry, I should've been more explicit that this was only a docs problem.