Updated the definition of 'Reducer' and 'Dispatch' for TypeScript 2.3+ #2479
Conversation
index.d.ts
Outdated
| @@ -43,7 +43,7 @@ export interface Action { | |||
| * | |||
| * @template S State object type. | |||
| */ | |||
| export type Reducer<S> = <A extends Action>(state: S | undefined, action: A) => S; | |||
| export type Reducer<S, A extends Action = Action> = (state: S, action: A) => S; | |||
There was a problem hiding this comment.
Why is S | undefined removed? Every reducer must accept undefined as the first arg to initialize the state.
There was a problem hiding this comment.
I don't think so.
For each Reducer, they may need to have the same structure of State, includes the initial state.
And we used to define Reducer with defaults like:
const someReducer: Reducer<State> = (state = {} as State /* this is may not undefined */, action) => stateThere was a problem hiding this comment.
Redux itself passes undefined to the reducer on Store init. Typings must reflect this.
There was a problem hiding this comment.
Sure, I have reverted this change.
index.d.ts
Outdated
| @@ -93,8 +93,8 @@ export function combineReducers<S>(reducers: ReducersMapObject<S>): Reducer<S>; | |||
| * transform, delay, ignore, or otherwise interpret actions or async actions | |||
| * before passing them to the next middleware. | |||
| */ | |||
| export interface Dispatch<S> { | |||
| <A extends Action>(action: A): A; | |||
| export interface Dispatch<A extends Action = Action> { | |||
There was a problem hiding this comment.
I'm not sure this is correct. Redux store is able to dispatch any possible action, so we can't constrain it to a specific subtype.
Also, here's the rationale for making state type a type parameter for Dispatch: #1537
However, for the whole year, I've only seen it being used in redux-thunk. Moreover, it would give type-checking benefits only when dispatch is called directly via store.dispatch(...), but if we e.g. use connect from react-redux, we'd still have to declare state type manually.
In light of the above, I tend to agree that we should consider removing the S type parameter from Dispatch.
There was a problem hiding this comment.
Yes, redux-thunk and redux-promise have different type for Dispatch.
But I think they should be re-defined in that packages.
Or just use any here :-)
There was a problem hiding this comment.
How about declare Dispatch like this:
export interface Dispatch<TInput = Action, TReturn = Action> {
(input: TInput): TReturn;
}for redux-thunk user, can use like store.dispatch<() => Action>(() => ({ type: "action" }))
for redux-promise user, can use like store.dispatch<Action, Promise<Action>>(someAction)
There was a problem hiding this comment.
The correct typing for redux-promise is something like
type Dispatch = <T>(action: PromiseAction<T>) => Promise<T>;Unfortunately there's no way to express this via type parameters. Currently we augment Dispatch signature in third-party middleware typings via module augmentation, and it works fine. There's a chance that with TS 2.4 we could extend Dispatch during store enhancement. I'm going to make some experiments when I have time.
| @@ -43,7 +43,7 @@ export interface Action { | |||
| * | |||
| * @template S State object type. | |||
| */ | |||
| export type Reducer<S> = <A extends Action>(state: S | undefined, action: A) => S; | |||
| export type Reducer<S, A extends Action = Action> = (state: S | undefined, action: A) => S; | |||
There was a problem hiding this comment.
I think this should conform with #2467
|
I am sorry for nagging, in my experience, it is very hard to fix typings without introducing a breaking change, so I just want to make sure we come up with the best typings possible for Redux v4. Otherwise, we would have to wait forever for the next major. |
No description provided.