Add state type to dispatch interface

[Igorbek]

This PR is an alternate to #1526
I've based it on what @use-strict proposed and augmented with the following:

• Made Dispatch interface be parameterized with state type S. Because dispatch is a part of store which has a state type, we can do it.
• Made Dispatch to be an interface, so that allows to augment it in other modules, such as middlewares.
• Dispatch exposes only one call signature at the the moment. It takes an action and returns it. It aligned with default behavior of redux. Any extension middleware could extend it by adding other signatures. For instance, redux-thunk can add a call signature that takes a function, and redux-promise can add one that takes a promise. So that any end interface shape will be determined by set of modules imported. This "module augmentation" approach is shown in dispatch.ts test.
• Middleware now is a function that returns a function that gets a dispatch and returns new dispatch.

Definitions have been tested with TypeScript 1.8.7.

[use-strict]

Looks OK. The S is a bit awkward and more of a workaround, but I don't find it too disturbing.

Wouldn't this play along with <TMiddlewareAction, TMiddlewareActionResult>(action: TMiddlewareAction): TMiddlewareActionResult; ? Can't we apply the same augmentation method but leave the generic by default? It better expresses the shape of API.

[Igorbek]

@aikoven @ulfryk please review.

[aikoven]

Sorry, I don't have much time right now to do a review.
At first sight this looks good to me, except for interface augmentation, because it's global and dispatch signature is changed only after middleware is applied. Still I admit that it's better than any.

I will check back in a few days.

[ulfryk]

Here Dispatch<S> is defined as a paramtrized type (generic) but the time parameter from signature is not used. @Igorbek can you explain the purpose?

Also I think that after a rather long discussion we have decided to drop type parameters from <A extends Action>(action: A): A; due to middleware issues.

See my initial proposition -> https://gist.github.com/ulfryk/69981ccfb488647b6ee3

[Igorbek]

Having state type parameter S allows middlewares be more accurate in typings. For example, thunk function takes a dispatch and a getState(): S, so without S it couldn't be well-typed. See the example of this here below https://github.com/Igorbek/redux/blob/ts-def-improv/test/typescript/dispatch.ts#L9-L16

Regarding second question, I think this signature express exactly what initial (without midllewares) dispatch does. It take an action and returns it. If I used less generic signature (action: Action) => Action, due to TypeScript rules it would fail on object literals, such as dispatch({ type: 'ADD_TODO', text: 'todo' }) (Object literals may only specify known properties). Moreover, it returns more precise result - typed action that was passed.

[Pajn]

Would the external module augmentation use case work for multiple middlewares?

Unfortunately, I don't have time to test for myself right now.

[Igorbek]

It will. Module can be augmented by any number of external modules.

-----Original Message-----
From: "Rasmus Eneman" [email protected]
Sent: ‎2016-‎03-‎31 8:28 AM
To: "reactjs/redux" [email protected]
Cc: "Igor Oleinikov" [email protected]
Subject: Re: [reactjs/redux] Add state type to dispatch interface (#1537)

Would the external module augmentation use case work for multiple middlewares?
Unfortunately, I don't have time to test for myself right now.
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[Pajn]

@Igorbek How would the final type definition for dispatch look if you say combined redux-thunk and redux-promise (just as an example with easy signatures, I know that using promise when you have thunk is pretty useless)?

[Igorbek]

Sure. It might be like that:

// node_modules/redux-promise/index.d.ts
declare module "redux" {
  export interface Dispatch<S> {
    (promiseAction:  Promise<Action>): Promise<Action>; // it supports promise of action
    <T>(fsa: { payload: Promise<T> }): Promise<T>;  //  it supports Flux standard actions
  }
}

// app.ts
import * as redux from "redux";
import "redux-promise";
import "redux-thunk";  //  why not

...
store.dispatch(ajax("...").then(d => ...));

To be honest, these signatures might not be able to express more complicated scenarios, like when promise resolves to other prmoise or a thunk function. It's still possible but would be slightly harder to use or define.

[aikoven]

Thanks @Igorbek, @use-strict

[gaearon]

Out in 3.4.0.

[AndyMoreland]

I acknowledge that this is somewhat late, but:

I'm working on upgrading several applications and libraries to the new version of redux because we like dropping dependencies on DefinitelyTyped type packages when npm libraries start shipping types directly.

However, the Dispatch<S> type is giving us some trouble. We don't use redux-thunk or redux-promise, so we don't have a use for the <S> parameter. Additionally, because some of our libraries are intended to be used across many applications and are therefore application-state agnostic, there isn't really a sane thing to put for a lot of the <S> arguments. As a result, the conversion results in Dispatch<any> littered everywhere in our codebase which is pretty sad.

Is there any way that <S> could be dropped from the core redux Dispatch interface? It seems strange that the mainline types have a nonsense type parameter for these external libraries.

[aikoven]

@AndyMoreland I agree that this is not very convenient in some cases, but it is the best (strongest) we could come up with.

I too don't use redux-thunk nor redux-promise in my projects so I tend to agree with you. Also, I don't have a lot of places where I use dispatch as a Store method; mostly as a standalone function passed via argument, e.g. react-redux. But IMO there's not much difference between passing state type as type parameter for Dispatch:

// SomeComponent.tsx

import {Dispatch} from 'redux'
connect(
  store => {...},
  (dispatch: Dispatch<MyState>) => {...}
)

or having a concrete Dispatch type declared in your project:

// configureStore.ts

import {ThunkDispatch} from 'redux-thunk'

export type MyDispatch = Dispatch | ThunkDispatch<MyState>;

// SomeComponent.tsx

import {MyDispatch} from '../configureStore'
connect(
  store => {...},
  (dispatch: MyDispatch) => {...}
)

Therefore it seems to me that having state type parameter for Dispatch is not very useful unless you use store.dispatch() directly. However I don't know which case is more important for the majority.

There are also other options, you could have something like this in your code:

import {Dispatch as ReduxDispatch} from 'redux';

export type Dispatch = ReduxDispatch<any>;

Or wait for optional type parameters (microsoft/TypeScript#2175), although it doesn't have any milestone set.

[aikoven]

@AndyMoreland I'd like to add that you're not late; I think everyone agrees that our goal is to bring the most convenient typings, so your feedback is appreciated.