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

Discussion: improving the developer experience for TypeScript users by making changes in the declaration file and adding documentation for types #231

Closed
samjmck opened this issue Jan 13, 2019 · 37 comments

Comments

@samjmck
Copy link

samjmck commented Jan 13, 2019

I think it's clear that Redux Thunk doesn't provide the best experience for TypeScript developers. A Google search for “redux thunk typescript” yields many results with conflicting and outdated information with different ways to do the same thing. As a TypeScript developer, trying to enter this space can become frustrating very quickly. This is why I'd like to propose some changes we can make to improve this situation.

Breaking the single-letter naming convention for generic types

The most used naming convention for generics in the TypeScript community is using single-letters for type names. It's also what's used in the official TypeScript documentation. While this can work well for simple components that don't require much explanation, it can become quite tedious to work with as the component you're working with becomes more complex.

Take this type, for example.

export type ThunkAction<R, S, E, A extends Action> = (
   dispatch: ThunkDispatch<S, E, A>,
   getState: () => S,
   extraArgument: E
) => R;

Without actually looking at the inside code of the type, the only variable type I could guess was S (which is the store state). Only when you start looking at the code, it becomes apparent that R is the result type by looking at which type the function returns, A is an Action by looking at what it's extending and that E is the extra argument by looking at its property name. This also means that if you're using an IDE, the parameter (or generic) info become virtually useless as you'll have to look at the definition anyway because it only works with descriptive names.

This is why I think we should consider using one of these naming conventions:

ThunkAction<ResultType, StateType, ExtraArgumentType, ActionType extends Action>

or

ThunkAction<TResult, TState, TExtraArgument, TAction extends Action>

I personally prefer the second one, as I've seen other codebases use it before and it also prevents you from having to write ActionType, which could conflict with another type you might have in a codebase using Redux.

The main downside of changing the naming convention is that we would be breaking the convention that's used in most TypeScript codebases as well as in official documentation. Ironically, I think it would improve readability.

Writing documentation on how to use the types with real world examples

There are 3 components in the current declaration file: ThunkDispatch, ThunkAction and ThunkMiddleware. While these names are fairly descriptive and self-explanatory, I think new Redux-TypeScript users would benefit greatly from official documentation with some good examples. At times, it can be difficult to use all the types correctly in conjunction with each other due to the way Redux is designed. This documentation could fit well into a separate TypeScript section on the official Redux documentation webpage, or we could keep it simple without going too much in depth and just put comments into the declarations file. The problem is that people might not always look at the declarations file for documentation.

I'm interested to see what everyone thinks of these two proposals, and what else we could do to help the TypeScript community.

@ablakey
Copy link

ablakey commented Feb 4, 2019

The last release of redux-thunk almost a year ago mentions dropping TS entirely, placing it in DefinitelyTyped. Is there an update on that?

Regardless, I think that yes, we need to coalesce meaningful documentation and examples in one sensible location. I've begun dumping examples (like this one: #224 (comment)) throughout PRs, issues, etc. but having a sole, "here's how you use redux-thunk with TypeScript and here's some 'recipes' " document would be fantastic. I'm happy to contribute some time to this.

@Ruffeng
Copy link

Ruffeng commented Feb 15, 2019

Struggled with the same for an entire day.
First I thought to move into Redux-saga. Seems thunk is not providing what we are looking for.
In the end, I am keeping thunk, but removing TS from there.

I have to say that I will move to another library next project I'll set up. It's a pity how simple and great is and because of the combination with TS everything is messed up

@samjmck
Copy link
Author

samjmck commented Feb 17, 2019

What do you guys think of renaming the generics to be slightly more descriptive? I know it's a minor change, but I guess every little bit helps.

@agwells
Copy link
Contributor

agwells commented Mar 6, 2019

+1 for the TReturnType naming scheme for the generics. That's the convention used in the DefinitelyTyped definitions for React-Redux, so it's already out there in the Redux ecosystem. :)

I'd also recommend:

  1. Provide some default values for the generics in ThunkDispatch and ThunkAction. Appropriate defaults could be AnyAction for the action type, and {} or any for most of the others.

  2. Replace the A (Action type) param of ThunkAction, with D extends ThunkDispatch = ThunkDispatch<S, E, AnyAction>. I found it confusing why ThunkAction needs to know my action types, and then I realized it's only so that it can type dispatch: ThunkDispatch correctly. So just providing the type of ThunkDispatch directly makes things more understandable. I guess the downside to that is that the typical typescript project organization for Redux assumes you've already declared an action type that is the union of all your actions. But you could, at the same time, define and export your dispatch type.

  3. It would be great if we could come up with a way to let TypeScript just infer the return type of the thunk, rather than having to pass it explicitly to ThunkAction as a generic param. That'd be one less argument you'd have to pass to ThunkAction. However, I tried a few approaches to this and couldn't get it to work, so perhaps it's not possible under the current TypeScript engine.

@thisissami
Copy link

Just chiming in to say that I am relatively new to both Redux-Thunk and even newer to TypeScript. Today I set out to upgrade all my thunk actions to use proper typescript, and I came to the issues section of this library to see if there was anything related to typescript that could help me out. The current documentation is incredibly confusing, and I fully support the original suggestion in this thread.

I also agree with @agwells that some things aren't very clear at first (e.g. what's the point of A in ThunkAction?). I think adding a few comments explaining the basics for relative n00bs like myself would go a looong way. Many of the libraries that I've been using have detailed explanations that appear when I don't do the types correctly, and this library was the exact opposite experience (no useful type variable names, and no explanations whatsoever).

my $0.02!

@JustFly1984
Copy link

I'm getting weird error while importing react-thunk@2.3.0 in vscode:

import thunk from 'redux-thunk'

and

import { ThunkAction, ThunkDispatch } from 'redux-thunk'

error:

File '/Users/justfly/projects/agrarian-gatsby/node_modules/redux-thunk/index.d.ts' is not a module.ts(2306)

@pfgray
Copy link

pfgray commented Mar 18, 2019

One of the first type errors I ran into was:

myStore.dispatch(myThunkAction)

which gave me:

Argument of type 'ThunkAction<...>' is not assignable to parameter of type 'AnyAction'.

This is because ThunkAction and ThunkDispatch are their own types and are not associated with redux's Dispatch or Action.

You can remedy this by merging redux-thunk's concept of Dispatch with redux's declarations:

~~I think the redux-thunk typings should use this approach (as they do with bindActionCreators), but I'm not sure if there are other implications which prevented this before ~~

see Tim's comment here and if you need to derive the type of your store, see my comment here

@joncys
Copy link

joncys commented Apr 11, 2019

@pfgray You should be using ThunkDispatch from react-redux redux-thunk and not just the regular Dispatch from redux.

Having said that, I would strongly support having the generic type definition names expanded into full names, because at first it is very confusing. I would think, that someone coming into TypeScript without any prior typed language experience will first really run into generics by using redux and redux-thunk.

Also, official documentation on the preferred way to use typings would be very nice as well. There are a lot of conflicting tutorials out in the public, and from my experience they all fall short in really explaining how all the pieces fit together.

If anyone else is struggling with typing their async actions, I suggest to look into here for the time being, this has helped me a lot: https://github.com/reduxjs/redux-thunk/blob/master/test/typescript.ts

[2019-04-25] edit: corrected badly referencing react-redux instead of redux-thunk in suggesting using ThunkDispatch

@pfgray
Copy link

pfgray commented Apr 11, 2019

@joncys , ThunkDispatch and Dispatch are types, not methods.

The dispatch method on any redux store is typed as Dispatch (not ThunkDispatch).

The only way to do this currently (without the aforementioned augmentation) is to typecast the dispatch function, which is unsafe:

(<ThunkDispatch<any, any, any>> myStore.dispatch)(myAction)

@timdorr
Copy link
Member

timdorr commented Apr 11, 2019

@pfgray You add thunks to a store with middleware, which you can use to define the types for ThunkMiddleware at that time. Those carry through to the Dispatch type, which gets changed to a ThunkDispatch: https://github.com/reduxjs/redux-thunk/blob/master/index.d.ts#L29

Here's what you do when you're applyMiddleware-ing: https://github.com/reduxjs/redux-thunk/blob/master/test/typescript.ts#L30

@agwells
Copy link
Contributor

agwells commented Apr 11, 2019

I've put together a pull request that provides more informative names for the type parameters, and adds docblocks above the types, explaining what part of Redux-Thunk they represent.

It should be a 100% non-breaking change, because it only changes comments and type param names. :)

@timdorr
Copy link
Member

timdorr commented Apr 12, 2019

Ok, that takes care of the first part. Who wants to help with documentation?

@pfgray
Copy link

pfgray commented Apr 14, 2019

@timdorr thanks for explaining that!

I tried this in my app, but then realized I'm deriving the state type from my store, after it's created (since my root reducer is often a big nest of combineReducers). For example:

// can't use DerivedAppState yet...
const store = createStore(reducer, applyMiddleware(thunk as ThunkMiddleware<DerivedAppState, AppAction>))

type DerivedAppState = typeof store.getState 

I've talked to a bunch of people who use this pattern, so I think this should be covered in the docs...

One technique would be to instead derive state from the first argument to the root reducer:

type AppAction = {type: 'foo'}

// this could come from a deep nesting of combineReducers
const rootReducer: Reducer<{user: string}, Action> = (s = {user: "bob"}) => s

type First<T> = T extends Array<infer U> ? T[0] : never
// AppState is now derived from the parameters of the reducer
type DerivedAppState = First<Parameters<typeof rootReducer>>

const store = createStore(rootReducer, applyMiddleware(thunk as ThunkMiddleware<DerivedAppState, AppAction>))

@agwells
Copy link
Contributor

agwells commented Apr 14, 2019

@pfgray I do something similar, but I use <Exclude<ReturnType<typeof rootReducer>, undefined>; to get the shape of my state. Here's a slightly simplified version of my whole project's setup, more or less. (My project is split up into "ducks", e.g. Redux modules consisting of a related reducer, actions, and thunks, so this is just the code where I tie them all together into one reducer/store.)

export const rootReducer = combineReducers(/* each duck's reducers */);
export type FullState = Exclude<ReturnType<typeof rootReducer>, undefined>;

// Intersection of actions from each duck
export type AllActions = FooAction | BarAction;

// I inject my i18n manager as an extra argument to thunks
export type ExtraThunkArg = {
  i18n: I18n;
};

export function makeStore(i18n: I18n, initialState?: FullState) {
  // The generic `createStore()` type correctly infers everything, including the actions and the thunk
  // thunk dispatch(), from the supplied arguments.
  return createStore(
    rootReducer,
    initialState,
    applyMiddleware(
      thunk.withExtraArgument({ i18n }),
    )
  );
}

// The type of my store (I sometimes need to reference this in things like unit tests)
export type StandardStore = ReturnType<typeof makeStore>;
// The type of my dispatch() method (for use in places like `mapDispatchToProps()`
export type StandardDispatch = StandardStore['dispatch'];

// The return type of a thunk action creator
export type StandardThunk<TReturnType = any> = ThunkAction<
   // In our project, we make all our thunks asynchronous.
   Promise<TReturnType>,
   FullState,
   ExtraThunkArg,
   AllActions
 >;
 
// An example of a thunk action creator
export function createThing(): StandardThunk {
  // You don't need to implicitly type the params of the inner function, because TS
  // correctly infers them from the outer function's return type of `StandardThunk`
  return async function(dispatch, getState, {i18n}) {
    // do some stuff
  };
}

// Or if you prefer arrow methods...
export const createThing = (): StandardThunk => async (
  dispatch,
  getState,
  { i18n }
) => {
 // do some stuff
};

Now, this is a bit of simplification because I actually use some additional middlewares, one of which seems to interfere with createStore()s ability to correctly infer my dispatch(). So in my real code I manually type my dispatch() method, and manually provide the type params to createStore()

/**
 * The type of our store's dispatch method.
 */
export type StandardDispatch = ThunkDispatch<
  FullState,
  ExtraThunkArg,
  AllActions
>;

// in `makeStore()`
  return createStore<
    FullState,
    AllActions,
    { dispatch: StandardDispatch },
    {}
  >( // ...etc

@threehams
Copy link

A few of us have been working on react-redux hooks typings in DefinitelyTyped/DefinitelyTyped#34913 (comment), and were curious about how much thunk support we should build into that. For some things, we could use Redux types and extend those types in redux-thunk, but beyond the simple useDispatch hook, that gets sketchy.

The existing typings for connect() already had some built-in support for thunks.

Opinions welcome on the best way to handle this, especially in a way that's friendly to other middleware without being overcomplicated to set up.

@timdorr
Copy link
Member

timdorr commented Apr 25, 2019

I don't see why React Redux typings would need to concern itself with a specific middleware. I mean, go right ahead if it's small in scope, but it seems like you'd be missing an opportunity to make any middleware easy to use with your types.

@threehams
Copy link

Really good point. I've been trying to make this work out of the box with thunk while allowing other middleware libraries to add overloads, but I'll see if I can tie everything back to Redux's Dispatch type so everyone can add single overloads there.

@markerikson
Copy link
Contributor

markerikson commented Jun 12, 2019

So, uh... I'm trying to use TS with Redux for the first time, and I'm suddenly agreeing that the single-letter generics here are ridiculous. (Not that I disagreed before, I just hadn't tried to make use of them myself.)

Where does the discussion in this thread actually stand atm?

@timdorr
Copy link
Member

timdorr commented Jun 12, 2019

Those were already changed in #243, just not yet released.

@reduxjs reduxjs deleted a comment from fwh1990 Jul 7, 2019
@threehams
Copy link

threehams commented Jul 8, 2019

I've created a small repo at https://github.com/threehams/react-redux-thunk-typings to experiment with the interactions between redux, redux-thunk, and react-redux. These (of course) aren't the only libraries, but getting these three working together with minimal setup should help a lot.

Tests for all three can be run with yarn test. This should prevent the need to juggle multiple repos and multiple ways of running tests to try out changes.

I'll see about getting a better PR than the one first tried:
#247

This brings up a question, though - how should you use these typings in an ideal world?

Should we assume that redux-thunk is included in the store if it's in package.json? This is the most convenient for use with bindings like react-redux (uses declaration merging), though it prevents multiple (different) middleware setups from being used in a single project.

import { useDispatch } from "react-redux";

const Thing = () => {
  const dispatch = useDispatch();
  // return value is `{ type: "CLEAR" }`, nothing extra needed!
  const result = dispatch((thunkDispatch, getState) => { // getState can't be typed though
    return thunkDispatch({ type: "CLEAR" });
  });
}

If we don't do this, you'd need to set up each of the overloads yourself for each library, which ties into...

What about documenting how to extend these libraries to provide types to each usage of connect() / useSelector's state, thunk's getState, allowable actions into Dispatch? Each user would need to do this, but it's extremely helpful. This prevents needing to manually type each thunk action or each call to useSelector. The typings should be set up to make this as simple as possible, otherwise you're importing and using State types all over for no reason (it's all global).

If you're not using bindings, and using store.dispatch, that's another case. The redux-thunk definition tests are set up for this; we have to support them, but in practice, I don't know how many people combine Redux bindings with direct calls to store.dispatch.

Few things I'm thinking about right now, and will be adding to the repo. Having separate packages makes this more challenging than I'd wish.

@nilshartmann
Copy link

Those were already changed in #243, just not yet released.

Hi @timdorr,

is there anything missing for a new release with the updated types? In #246 you said, you're waiting for the new types, but as I understand, the new types are now merged in #243?

Thanks for clarification and thanks to all contributing to the beautiful new types and docs!!! ❤️

@mayurarora0903
Copy link

I've just started with TypeScript and stumbled upon the following two errors thrown by the TSC:

  • node_modules/redux"' has no exported member 'AnyAction'
  • node_modules/redux-thunk/index.d.ts:14:84 - error TS2315: Type 'Middleware' is not generic.

Is this discussion relevant to the above two. Has anyone been able to solve for these ?

I am on redux-thunk 2.2.9 and redux 3.6.0

@inunotaisho
Copy link

inunotaisho commented Aug 31, 2019

How long am I going to be waiting for you guys to get this fixed?

@markerikson
Copy link
Contributor

markerikson commented Aug 31, 2019

@inunotaisho26 : as long as it takes for us to get around to dealing with it. Comments like that don't help.

We're currently focusing on rewriting Redux itself in TypeScript. That may impact thunks. I know Tim has also said he isn't happy with the current state of types here. So, please be patient.

@inunotaisho
Copy link

@markerikson You're referencing the wrong person.

@markerikson
Copy link
Contributor

Yep, that's what I get for trying to reply on mobile.

Point still stands, though.

@samjmck
Copy link
Author

samjmck commented Sep 1, 2019

@inunotaisho26 : as long as it takes for us to get around to dealing with it. Comments like that don't help.

We're currently focusing on rewriting Redux itself in TypeScript. That may impact thunks. I know Tim has also said he isn't happy with the current state of types here. So, please be patient.

Great to hear this. Looking forward to the future of Redux! Is there anywhere I can follow the progress being made in the transition to TypeScript?

@inunotaisho
Copy link

@inunotaisho26 : as long as it takes for us to get around to dealing with it. Comments like that don't help.
We're currently focusing on rewriting Redux itself in TypeScript. That may impact thunks. I know Tim has also said he isn't happy with the current state of types here. So, please be patient.

Great to hear this. Looking forward to the future of Redux! Is there anywhere I can follow the progress being made in the transition to TypeScript?

Ditto

@markerikson
Copy link
Contributor

See reduxjs/redux#3500 and reduxjs/redux#3536

@ablakey
Copy link

ablakey commented Sep 4, 2019

It's not quite clear to me, having read this thread. Are there known issues with typings for redux-thunk + useDispatch hook? I am having an issue where no matter how I type my thunk action creator, the type returned is a function that returns a promise, rather than the promise, to be chained or awaited itself.

I won't get into a detailed example in this thread as it's leaning off topic. But it sounds like some of you are saying that this mixture of technologies doesn't really work right yet.

@swyxio
Copy link

swyxio commented Sep 4, 2019

for those googling their errors (e.g. TS2315: Type 'Middleware' is not generic.) and just want a quick fix, the answer is that the redux types and redux-thunk types are out of sync at the moment. Please pin versions for now:

redux-thunk: 2.2.0  => redux 3.x

and if you can, pitch in on the redux typescript initiative as that needs to be done first before the redux thunk types are fixed.

@nsmithdev
Copy link

nsmithdev commented Sep 4, 2019

I have a few minimalist helper/alias that have been working well for me so far.
useDispatchTs is the only one that did not already exist in my project but was able to throw one together real quick and it looks like it's working as expected. I'm sure there is a better way but this has been working good for me so far. DispatchAction can also be used for dispatch when injected as prop by parent.

import { AnyAction, Action } from 'redux'
import { ThunkAction, ThunkDispatch } from 'redux-thunk'
import { MyAppStore } from 'reducers'
import { useSelector, TypedUseSelectorHook } from 'react-redux'

export type AsyncAction<R=void> = ThunkAction<Promise<R>, MyAppStore, undefined, AnyAction>
export type DispatchAction<T extends AnyAction = Action> = ThunkDispatch<MyAppStore, undefined, T>
export const useSelectorTs: TypedUseSelectorHook<MyAppStore> = useSelector
export const useDispatchTs: () => DispatchAction = useDispatch

export function getHelp(): AsyncAction<MyReturnObject> {
  return async (dispatch, getState) => {
      const response = await asyncRequest({
        path: 'api/help',
        method: 'GET',
        body: null,
      })
      return response // MyReturnObject
  }
}

const dispatch = useDispatchTs()
var a = await dispatch(getHelp())
a.blabla

@ablakey
Copy link

ablakey commented Sep 4, 2019

@nsmithdev Aha! Custom typing the useDispatch works perfectly. Now I can actually do stuff with the action results at the dispatch site (like local error handling rather than plopping errors onto the redux application state). Thank you!

@akomm
Copy link

akomm commented Jan 24, 2020

You can also keep your actions as-is and instead type useDispatch.

// your custom typings
type ThunkDispatch = {
  <TAction>(action: TAction):
    TAction extends (...args: any[]) => infer TResult ? TResult :
      TAction,
};
// in your component:
const dispatch = useDispatch<ThunkDispatch>();

Important: calling dispatch will infer both normal action creators and thunk-action creators. But you have to be aware of ts type-widening, when you pass literal values into the dispatch, for example dispatch({type: "x", payload: 20}), it will be widened to {type: string, payload: number}.

But normally you should use action creators instead of literal values: dispatch(someAction()). In this case your action creator should return an exact type anyway and then it will be correctly be inferred.

If you want to use literal anyway, you need to pass it like this: {type: "x" as const, payload: 20 as const}.

This does not make any expectations on the allowed actions, etc. since the hook uses context and its opaque, it has to be defined manually, what the dispatch should be allowed to dispatch. So you can make your app's local ThunkDispatch by replacing the generic TAction with your custom set of action types and the dispatch will only allow those or move the function generic <TAction>(action: TAction) to to the type: ThunkDispatch<TAction>:

// your custom typings
type ThunkDispatch<TAction> = {
  (action: TAction):
    TAction extends (...args: any[]) => infer TResult ? TResult :
      TAction,
};
// in your component:
const dispatch = useDispatch<ThunkDispatch<AppActions>>();
// you can also alias
// type AppThunkDispatch = ThunkDispatch<AppActions>
// const dispatch = useDispatch<AppThunkDispatch>();

I guess the example @nsmithdev provided can be simplified using the infer syntax, if you also want to swap the ...args: any[] with specific types.

@nickretallack
Copy link

The latest version has new type annotations, but they haven't been released into an NPM package yet. Can someone make that happen soon?

@timdorr
Copy link
Member

timdorr commented Oct 25, 2021

This has been done, so I'm going to close it out.

@timdorr timdorr closed this as completed Oct 25, 2021
@markerikson
Copy link
Contributor

and 2.4.0 is now live!

https://github.com/reduxjs/redux-thunk/releases/tag/v2.4.0

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

No branches or pull requests