TypeScript: usage of DeepPartial for preloadedState is too deep

[OliverJAsh]

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Since #2679, we use DeepPartial for the type of preloadedState.

This means that, given a state tree, all properties at any level in the tree are optional.

What is the expected behavior?

Given a state tree, only properties whose state is computed by a reducer should be optional. This is because reducers have initial state, but the state within them may be required (unless those properties are explicitly marked as optional or they represent reducers).

Which versions of Redux, and which browser and OS are affected by this issue? Did this work in previous versions of Redux?

N/A

---

/cc @aikoven @dawnmist

I'm not quite sure how we can write the type definition to match the expected behaviour. Any ideas?

Related #2552

[aikoven]

I'm not sure it's possible without some kind of black magic.
But it might be possible to solve with the upcoming TS 2.8 feature — Conditional Types.

The crucial part here is combineReducers. The preloadedState type should be the same as the type of the first argument of the reducer passed to createStore. It should also be a subtype of DeepPartial<S>.

Given some type T where DeepPartial<S> <= T <= S we should be able to produce Reducer type that accepts T as its first argument, and returns S. combineReducers should preserve it when mapping over ReducersMapObject.

[Shakeskeyboarde]

There’s a comment in #2679 that it needs to be a deep partial, and not just partial. But hunting through the issues and PRs I’m not seeing why? Partial would have fixed the issue without adding new issues... even in the case of nested combined reducers. What am I missing? What’s the scenario that required deep partial?

[dawnmist]

Partial<State> would have only fixed one level of the issues - if you have a nested tree of reducers the inner ones would again require being fully defined or the entire tree left out entirely.
e.g.

{
   reportA
        => options
             => optionA
             => optionB
             => optionC
   reportB,
   ...
}

With only using Partial<State>, only the reportA and reportB reducers would be marked as optional - their children would remain required as per the normal State definition so that if reportA was provided all of its children were required. This then means that you could not supply an initial value for the optionB reducer without being required to supply one for the optionA & optionC reducers and any other items inside the reportA reducer - the contents of the reportA reducer become all-or-nothing. What is desired/required is that each layer be optional - and that's where the problem is, because there wasn't a way to determine if the output type was from a combination of reducers (i.e. therefore all fields should be optional) or from a single reducer (where all fields should be required).

DeepPartial is the closest we could come up with at the time - but has the issue that if a reducer returns an object rather than a single/array value it extends the optional setting one level too far.. :(

[Shakeskeyboarde]

Ah, I had read that before, but it didn't compute until I did a little more experimenting.

I can think of two solutions.

1. Make combineReducers() return a Reducer<Partial<S>, ...> instead of just Reducer<S, ...>, thereby baking the "partiality" into the generated state. Then createReducer() would simply have an optional preloadedState parameter (not DeeplyPartial or even Partial).

2. Make combineReducers() attach a symbol to the generated state to indicate that it's a combined state root object CombinedState type, which would allow DeeplyPartial to be replaced with a CombinedPartial which could selectively apply Partial to CombinedState objects.

What do you think?

I'm leaning toward option 2, because store.getState() shouldn't indicate the combined reducer state shape has optional properties.

[Shakeskeyboarde]

I've opened a pull request for this issue. #3485