TypeScript definitions improvements

[use-strict]

This PR follows #1413. I will open up some wounds here, so please excuse me if I missed any of the points in the previous conversation.

Signature of dispatch

The problem that I still have is with the signature of dispatch. any just isn't type-safe at all. It will propagate silently in the app without the developer even realizing. w.r.t what @Pajn and @joshuacc said, there is a difference between consciously making a choice to type-cast and getting any by default. With this in mind, it's even better to use {} instead of any (which btw is what type parameters default to anyway).

Given the nature of dispatch and it being wrapped by middlewares, it isn't possible to properly type it to account for every possibility. (goes with what @xogeny said in previous PR) The default behavior should be to receive Action and return Action. When middlewares are involved, the middleware decides what the action is and what is returned. Unless someone finds of a way to augment the dispatch signature to work auto-magically with specific middlewares, I think it's better to let the user cast explicitly, because it is something known only by him, given the combination of middlewares that he applied.

I understand that explicit cast is more verbose and some may even be tempted to say it's even annoying, but think of it this way: it isn't any different than using Promise or Map or Set. Only the user know what exactly he's dispatching and what he expects to be returned.

Signature of StoreCreator and StoreEnhancer

I'm not sure I fully understand how these work, but the signature looks different to me than what results from the source code: https://github.com/reactjs/redux/blob/master/src/createStore.js#L49

Should be something like:

    interface StoreEnhancer<TState> {
         (createStore: StoreCreator<TState>): (reducer: Reducer<TState>, initialState?: TState) => Store<TState>;
    }

In this case StoreEnhancer should be generic. This leads to another problem. If it's typed as a generic interface, then StoreCreator should be generic as well, in which case we can no longer type the createStore variable. The only way I see to fix this would be to create to interfaces that are basically copy-pastes of each other:

export interface StoreCreatorInferrable {
  <S>(reducer: Reducer<S>, enhancer?: StoreEnhancer): Store<S>;
  <S>(reducer: Reducer<S>, initialState: S,
      enhancer?: StoreEnhancer): Store<S>;
}
export interface StoreCreator<S> {
  (reducer: Reducer<S>, enhancer?: StoreEnhancer<S>): Store<S>;
  (reducer: Reducer<S>, initialState: S,
      enhancer?: StoreEnhancer<S>): Store<S>;
}

[use-strict]

cc @aikoven , @igorbt

[Igorbek]

it will not infer types automatically. can you define the function that will not require providing types explicitly?
If I call it without types, TS will infer {} as a TMiddlewareAction.

[use-strict]

@Igorbek , I don't think it's possible to infer the types here unless you know exactly what middlewares you added. That's runtime behavior.

Just like my comparison with Promise, you don't know beforehand what that Promise is supposed to resolve to, until someone actually returns something. The middleware case is more or less the same since it might return something or even nothing at all.

[Igorbek]

You know what you can expect from dispatch if you know what configuration to use.
Ideally, if I had only standard configuration without thunk, dispatch would only expect an action and return it:

export interface Dispatch {
    (action: Action): Action;
}

So then you have no choice but using this signature.
But if you include thunk, you can augment this interface with new supported signatures:

module "redux" { // module augmentation
  export interface Dispatch { // interface augmentation
    (actionCreator: (dispatch: Dispatch) => void): Promise<any>; // whatever it returns
  }
}

So then you can you any of signatures were defined. I believe, TS definition for thunk should be in its package.

[Igorbek]

I forgot, it takes getState also:

module "redux" { // module augmentation
  export interface Dispatch { // interface augmentation
    (actionCreator: (dispatch: Dispatch, getState: () => any) => void): Promise<any>; // whatever it returns
  }
}

If we define Dispatch to be typed by state< it'd be more useful:

module "redux" { // module augmentation
  export interface Dispatch<TState> { // interface augmentation
    (actionCreator: (dispatch: Dispatch, getState: () => TState) => void): Promise<any>; // whatever it returns
  }
}

And then store must refer to its's state type in dispatch method.

[Igorbek]

Ok, I've found what it returns. So the thunks's signature would be:

module "redux" {
  export interface Dispatch<TState> {
    <TResult>(asyncAction: (dispatch: Dispatch, getState: () => TState) => TResult): TResult;
  }
}

[Igorbek]

I've added PR with my thoughts to your PR use-strict#1 :)

[use-strict]

@Igorbek , any ideas about the StoreEnhancer in my first post? That's the only thing remaining that I haven't quite figured out.

[Igorbek]

@use-strict for me, StoreEnhancer looks good as it is now. However, according to its using it doesn't need to support this complex signature of StoreCreator. The function produced by an enhancer can strictly expect initialState were passed and other enhancer weren't passed:

export type StoreEnhancerStoreCreator<S> = (reducer: Reducer<S>, initialState: S) => Store<S>;
export type StoreEnhancer<S> = (next: StoreCreator) => StoreEnhancerStoreCreator<S>;

As you suggested, a typed version of StoreCreator could be added, but not replaced since createStore must be generic. I'll create other PR to express it.

[aikoven]

@Igorbek how's its going with PR? Should we wait for it before releasing #1413?

[buzinas]

@aikoven Please, if possible, make a release of #1413 that's already merged asap, and then we discuss about the this and #1537 for the next release.

[Igorbek]

My PR is ready to merge. However the reviewers have asked a few questions which I answered, but didn't hear back from them that is enough or if they have some other concerns.
My PR introduces changes to Dispatch that is more desirable to have earlier rather than after a version without it. I'd say it's worth to wait it merged. I hope it wound't take to much to wait.

[aikoven]

@Igorbek I meant another PR that you mentioned above, with changes to StoreEnhancer/StoreCreator. I can't find it in the PR list.

[Igorbek]

Oh. I've completely forgot about it and misunderstood you meant it. It wouldn't take too much to do this, but I can do it in evening only (it's morning now for me). So, optimistically, with one day review from you guys, it takes 2 days in total. I know you want to release new features and it might delay, does it work for you?

[gaearon]

Out in 3.4.0.