Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript: combineReducers returns reducer that accepts partial state #2809

Conversation

OliverJAsh
Copy link
Contributor

combineReducers returns a reducer that accepts partial state, not the whole state.

@@ -86,7 +86,7 @@ export type ReducersMapObject<S = any, A extends Action = Action> = {
* object, and builds a state object with the same shape.
*/
export function combineReducers<S>(reducers: ReducersMapObject<S, any>): Reducer<S>;
export function combineReducers<S, A extends Action = AnyAction>(reducers: ReducersMapObject<S, A>): Reducer<S, A>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a shame to break apart from the Reducer type here, but without adding an extra generic to Reducer, I don't see a way around it?

@aikoven
Copy link
Collaborator

aikoven commented Feb 5, 2018

We could consider adding a type

type CombinedReducer<S, A extends Action = AnyAction> = 
    (state: Partial<S> | undefined, action: A) => S

It would be assignable to Reducer<S, A>, so you'd be able to use it anywhere like a regular Reducer.

Although when you pass it to a generic function with S as a type parameter, it is inferred to Reducer<Partial<S>>. Particularly, when nesting combineReducers:

declare const reducer: Reducer<number>;

const combined = combineReducers({
  parent: combineReducers({
    child: reducer,
  }),
});

typeof combined === (
  state: Partial<{parent: Partial<{ child: number }>}> | undefined,
  action: AnyAction
) => {parent: Partial<{ child: number }>}

combined(undefined, {type: ''}).parent.child  // number | undefined, but expected number

@OliverJAsh
Copy link
Contributor Author

Thinking about this some more, I think the state parameter type needs to use something like DeepPartial, to allow partial state to be passed in when using nested combineReducers.

Although DeepPartial is problematic in itself: #2808

@timdorr
Copy link
Member

timdorr commented Dec 13, 2018

Since this stalled, I'm going to close it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants