Make typings compatible with Redux 4.0.0

[Cryrivers]

Partially fixes #169.

BREAKING: It only supports TypeScript 2.4.0 or later.

Changed ThunkAction<R, S, E> to ThunkAction<R, S, E, A>, where R is the return type of the thunk function, S is the type of root state, E is the type of extra arguments, A should be all actions that can be dispatched.

Due to the limitation of current implementation, I'm not sure if R, S, E, A could be inferred. (UPDATE: I changed the implementation a little bit. I believe R, S, E, could be inferred in store.dispatch(). Changes needed in type definition for redux to make A inferable. )So a potentially better solution is to create a type for thunk actions by yourself and specify root states and all actions that can be dispatched.

Example:

type SideEffect<T> = ThunkAction<Promise<T>, RootState, {}, RootAction>

Here I assume all my thunk functions return a promise of something. Then my thunk function returns a SideEffect<T>.

export function asyncFunc(): SideEffect<string> {
  return async (dispatch, getState) => {
    dispatch({ type: ActionTypes.ASYNC });
    await timeout(1000);
    dispatch({ type: ActionTypes.ASYNC_FULFILLED });
    await timeout(1000);
    dispatch({ type: 'INVALID_ACTION_TYPE' });
    return 'NICE!';
  };
}

Then you can check the typings of dispatch and getState.

Naturally it also makes dispatching type-safe, too.

[Cryrivers]

E could be inferred in withExtraArgument. S could be inferred in Middleware. Now the problem is A is not inferable. I'm thinking of opening a PR in https://github.com/reactjs/redux to pass action types to Middleware

EDIT: I'm wrong I guess.

[esiqveland]

Any way for me to try out these changes?

[0livare]

@esiqveland I did the following and everything seems to be working swimmingly:

• Cloned @Cryrivers fork
• Checked out his dist branch
• Copied the following from the dist branch to the node_modules/redux-thunk directory of my project
  • dist directory
  • src directory
  • index.d.ts file

[Cryrivers]

@esiqveland @zposten I guess you could run npm install git://github.com/Cryrivers/redux-thunk .git#dist --save

[0livare]

@Cryrivers I realize this is a WIP, but I thought it could be helpful if I presented what was happening for me with these changes.

When I try to dispatch my thunk action to the store like this:

type ThunkResult<T> = ThunkAction<Promise<T>, RootState, {}, RootAction>


function myThunkAction() : ThunkResult<ISauce> {
  return async (dispatch, getState) => {
    const secretSauce = await api.getSecretSauce()
    // Below is the regular action creator
    dispatch(getSecretSauceSuccess(secretSauce))
    return secretSauce
  }
}

const store: Store = configureStore()
store.dispatch(myThunkAction()) // <-- error occurs here

I get the following error:

Argument of type 'ThunkAction<Promise<ISauce>, RootState, {}, RootAction>' is not assignable to parameter of type 'AnyAction'.
  Property 'type' is missing in type 'ThunkAction<Promise<ISauce>, RootState, {}, RootAction>'.

That sort of makes sense to me because the definition of ThunkAction from index.d.ts does not have a type property, but I don't know how the middleware is supposed to account for that.

Am I doing something wrong?

[Cryrivers]

@zposten no, you are not doing wrong at all. I changed it to WIP status because I need to deal with dispatch thunk action (the case you are encountering, currently not supported yet i guess) and redo the testing for typescript.

please allow me to spend sometime to finish them. thanks.

[Cryrivers]

@zposten I just fixed the test cases and the definitions. I guess you might want to refer to the test cases file for usage of typings.

[0livare]

Won't this cause a stack overflow? I think second simple method that returned a ThunkResult would solve this

function testGetState(): ThunkResult<void> {
  return (dispatch, getState) => {
    const state = getState();
    const foo: string = state.foo;
    dispatch({ type: 'FOO' });
    // typings:expect-error
    dispatch({ type: 'BAR'});
    dispatch({ type: 'BAR', result: 5 });
    // typings:expect-error
    dispatch({ type: 'BAZ'});
    // Can dispatch another thunk action
    dispatch(getStringThunk()); // <-- Updated this method call
  };
}

function getStringThunk(): ThunkResult<string> {
    return (dispatch, getState) =>  'foobar';
}

[Cryrivers]

Yes if you run it. but this file goes through type checking only, wont be executed by test runner.

[0livare]

But don't you think this serves as a nice example file that someone might try running?

[Cryrivers]

I think you are right. We could change it to another thunk action.

[Cryrivers]

Fixed.

[0livare]

@Cryrivers I very much appreciate the work you did here! This is currently working wonderfully in my project 😄

One issue I had, that I think might be worth documenting somewhere (unless it's just a me thing) is the fact that you cannot type your store to be of type { Store } from 'redux', otherwise you will get a typescript error when attempting to dispatch thunks

const store: Store = createStore(/*params*/);    // <-- Typing this to Store
store.dispatch((dispatch, getState) => 'foobar') // <-- Causes this to error

Argument of type 'ThunkAction' is not assignable to parameter of type 'AnyAction'.
Property 'type' is missing in type 'ThunkAction'.

[Cryrivers]

I guess there's no need to declare store as Store because Store is a generic type. If you use Store alone without specifying types, default dispatch signature would be used. However, createStore returns a Store where most generic constraints could be specified, which should be sounder in your case.

[duro]

@Cryrivers I've been testing this, and it seems to not be working when the return type is a promise.

I have Thunk creator that looks like this:

type PromisedThunkResult<R> = ThunkAction<
  Promise<R>,
  IAppState,
  undefined,
  ReturnType<typeof promiseAction>
>

export function promisifyAsyncAction<T extends AsyncActionCreator>(
  actionCreator: T
) {
  return (
    payload: ReturnType<T['request']>['payload']
  ): PromisedThunkResult<ReturnType<T['success']>['payload']> => dispatch =>
    new Promise((resolve, reject) =>
      dispatch(
        promiseAction({
          payload,
          actionCreator,
          resolve,
          reject
        })
      )
    )
}

I then build the creator like so:

const mapDispatchToProps = {
  setSessionToken: SA.setToken,
  setTokenAsync: promisifyAsyncAction(SA.setTokenAsync)
}

And call it like so:

const result = await props.setTokenAsync({token: 'whwhw'})

The type that gets assigned to result is defintely not the return type of the promise.

Am I doing something wrong, or is this any issue?

[Cryrivers]

@duro could you post the implementation of AsyncActionCreator and a minimum viable implmentation of setToken so we can reproduce the issue?

what's the type of result inferred by TS?

[Kimahriman]

I'm having an issue with the changes to ThunkDispatch. I wrote a bindThunk that works like bindActionCreators for an individual thunk action creator. This is what it looks like pre 4.0.0:

export type Thunk<R> = ThunkAction<R, GlobalState, void>

export function bindThunk<R>(thunkActionCreator: () => Thunk<R>, dispatch: Dispatch<GlobalState>): () => R
export function bindThunk<R, A1>(thunkActionCreator: (a1: A1) => Thunk<R>, dispatch: Dispatch<GlobalState>): (a1: A1) => R
export function bindThunk<R, A1, A2>(thunkActionCreator: (a1: A1, a2: A2) => Thunk<R>, dispatch: Dispatch<GlobalState>): (a1: A1, a2: A2) => R
export function bindThunk<R>(thunkActionCreator: (...args: any[]) => Thunk<R>, dispatch: Dispatch<GlobalState>) {
    return (...args: any[]) => dispatch(thunkActionCreator(...args))
}

This was how I was able to get proper type support for binding these functions. However I am unable to do this now because ThunkDispatch isn't exported. Any reason not to export that?

Also if there's a better way to do what I'm trying to do that would be great too. I'm still trying to figure out how to get a similar thing working on an object like bindActionCreators does.

[derekclair]

These changes did the trick for me. Thank you for putting this PR together! I did not run any of the test (just looked them over) so I can't really speak to those. But the index.d.ts are playing nicely. Only input is, as @Kimahriman stated, unless there is a specific reason that I'm unaware of and would love to know, I think it would be helpful to export interface ThunkDispatch for those use-cases where that definition is needed. Again, thank you, and let's get this merged STAT!

[Ky6uk]

I am also agree with moving typings away of this repository. Right now this is only major blocker for using TS + Redux 4.0.

[timdorr]

I'm all for less complexity. It will still need a release to actually come out of the package.

Are there sufficient typings in DT now? I don't want to break everyone's apps 😄

[jacobstern]

@markerikson Regardless of your opinion on TypeScript, this repository contains TypeScript code and that code is broken.

Is there a PR open to add types in DefinitelyTyped?

[jimsugg]

Our project is blocked on moving to redux v4 until this is resolved. I have tested the redux-thunk type definitions in this pull request, and they work well in our case. Thanks @timdorr for working to get this in soon.

[timdorr]

I don't see sufficient types in DT yet. I can work on moving them over. The easiest thing to do is take these on now in 2.x, get them into DT, and then bump to 3.x to remove them here (since that's breaking). Sound good?

[NathanNZ]

@timdorr any progress on this?

We could always accept this pull request for now, and then move the types across to DT at a later time. I really feel like we should focus on having something useable now even if it isn't perfect - and then the migration is a different story that can be figured out at a less urgent pace.

[timdorr]

Nope, sorry. I've been caught in a big project at work, so I haven't had time for this. I've also got a release of React Router to do. And I wanted to get my new React Redux implementation out for folks to play around with. Busy busy busy!

I'm fine with that strategy for now. I'm not familiar with DT's processes, so it would take some time to get the typings set up over there as well (and make it clear I'm not the subject matter expert! 😄). So getting these out would be better than nothing for the moment.

[Ky6uk]

@timdorr will you plan to make a new package with this fix? Thanks.

[yuqingc]

Finally merged! Congrats!!!

[timdorr]

Yes, I'll release it some time soon. I need to make sure I can build things correctly. I'll punt on revamping the build process until 3.0, where we'll drop the built-in types too.

[neodescis]

@timdorr Thanks very much for merging this; look forward to the release so I can migrate to redux 4. I would like to point out though, that I believe this is a breaking change (at least for TS users)? If so, it'll probably need a major version bump. Up to you though.

[jawadst]

@timdorr This was actually a breaking change and our builds started failing with the following error:
error TS2315: Type 'Middleware' is not generic.

Maybe this would have been better as a major release. It's not a big deal (we fixed it by using 2.2.0 instead of ^2.2.0) but figured I'd document this here if anyone has the same issue.

[Cryrivers]

@jawadst i guess it references a older version of redux which Middleware is not generic. Type of Middleware should be generic since redux 4.0.0

[timdorr]

Correct, you need to use redux 4.0 with redux-thunk 2.3.0. This kind of version conflict is why I'm getting rid of them with the next major :)

[deini]

@Cryrivers Are return types working as expected?

Taking code from the typescript tests

function anotherThunkAction(): ThunkResult<string> {
  return (dispatch, getState) => {
    dispatch({ type: 'FOO' });
    return 'hello';
  }
}

In theory I should be able to do something like...

const test = anotherThunkAction();

And expect test to be of type string? Or am I missing something?

[Kimahriman]

@deini

const test = dispatch(anotherThunkAction())

Then test should be of type string

[deini]

Thanks for pointing that out @Kimahriman

Following the test examples if I do

const test = store.dispatch(anotherThunkAction());

test is of type any, probably something else I'm missing 🤔

Edit:
My bad, was not using ThunkDispatch, all good 👍

[Kimahriman]

I'm not sure @deini, it types it as string for me when I just edit the test file directly. Must be missing something else. Make sure the middleware is applied with the as ThunkMiddleware<State, Actions> cast