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

Typings don't work for redux 4 #169

Closed
nenadalm opened this issue Dec 5, 2017 · 17 comments · Fixed by #180
Closed

Typings don't work for redux 4 #169

nenadalm opened this issue Dec 5, 2017 · 17 comments · Fixed by #180

Comments

@nenadalm
Copy link

nenadalm commented Dec 5, 2017

No description provided.

@huestack
Copy link

huestack commented Dec 13, 2017

After updating redux to v4.0.0-beta.1, compilation results in following error:

$ tsc
node_modules/redux-thunk/index.d.ts(4,47): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).
node_modules/redux-thunk/index.d.ts(8,20): error TS2428: All declarations of 'Dispatch' must have identical type parameters.
node_modules/redux/index.d.ts(103,18): error TS2428: All declarations of 'Dispatch' must have identical type parameters.
node_modules/redux/index.d.ts(150,13): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).
node_modules/redux/index.d.ts(271,13): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).
node_modules/redux/index.d.ts(285,59): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).
node_modules/redux/index.d.ts(285,75): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).
node_modules/redux/index.d.ts(357,95): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).
node_modules/redux/index.d.ts(362,33): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).
node_modules/redux/index.d.ts(364,106): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).
node_modules/redux/index.d.ts(369,34): error TS2314: Generic type 'Dispatch<D, S>' requires 2 type argument(s).

My dependencies:

"dependencies": {
		"react": "^16.2.0",
		"react-redux": "^5.0.6",
		"redux": "4.0.0-beta.1",
		"redux-thunk": "^2.2.0"
},
"devDependencies": {
                "@types/react": "^16.0.29",
		"@types/react-native": "^0.51.2",
		"@types/react-redux": "^5.0.14",
		"tslint": "^5.8.0",
		"typescript": "^2.6.2"
}

When I change Dispatch definition to following lines from redux-thunk/index.d.ts, everything works fine:

declare module "redux" {
  export interface Dispatch<D = Action> {
    <R, E>(asyncAction: ThunkAction<R, D, E>): R;
  }
}

@vovkasm
Copy link

vovkasm commented Dec 20, 2017

It is not fixed all cases, moreover, you effectively pass Action instead of State to thunk action defined couple lines earlier:

export type ThunkAction<R, S, E> = (dispatch: Dispatch<S>, getState: () => S,
                                    extraArgument: E) => R;

Also I don't know right solution still... Will try to find.

PS. Author really need comaintainers here... (((

@aikoven
Copy link
Collaborator

aikoven commented Dec 21, 2017

Typings for Redux v4 are not yet final.
Will update redux-thunk typings after v4 is released.

@Cryrivers
Copy link
Contributor

Cryrivers commented Jan 3, 2018

Disclaimer: Typings for Redux v4 are not yet final

import { Middleware, Dispatch } from 'redux';

export type ThunkAction<R, S, E, D> = (dispatch: Dispatch<D, S>, getState: () => S, extraArgument: E) => R;

declare module 'redux' {
  export interface Dispatch<D, S = {}> {
    <R, E>(asyncAction: ThunkAction<R, S, E, D>): R;
  }
}

declare const thunk: Middleware & {
  withExtraArgument(extraArgument: any): Middleware;
};

export default thunk;

Per the definition of Dispatch<D, S> in Redux 4.0, you just need to add one more type param D, which stands for the type of action you are dispatching. And everything should work again. One more benefit you can get is, you can get the type-safe dispatch in (dispatch, getState) callback.

screen shot 2018-01-03 at 3 14 54 pm

s-shin added a commit to s-shin/mem-desktop that referenced this issue Jan 5, 2018
NOTE: redux-thunk doesn't support redux v4 yet.

reduxjs/redux-thunk#169
@lukescott
Copy link

lukescott commented Feb 8, 2018

OK, so I'm just getting into redux-thunk. Just want to make sure I understand this. In Redux 3, my thunk action type currently should look something like this, right?

// using third parameter for environment
export type MyThunk = ThunkAction<void, any, "env1" | "env2">

But with Redux 4, it will look (possibly) something like this (stricter typing possible), right?

export type MyThunk = ThunkAction<void, MyThunkResultAction, "env1" | "env2">

Which then restricts my dispatch() function to dispatch MyThunkResultAction, correct?

If this is the case (which I hope it is), can we do something like this instead:

// third type is the thunk's return, is optional, and defaults to void if not specified
export type MyThunk = ThunkAction<MyThunkResultAction, "env1" | "env2", void>
// so you can do this
export type MyThunk = ThunkAction<MyThunkResultAction, "env1" | "env2">
// or possibly this:
export type MyThunk = ThunkAction<MyThunkResultAction, StateType, "env1" | "env2", ReturnType>

@NathanNZ
Copy link

NathanNZ commented Feb 13, 2018

Edit:
I've updated the snippet based off the information @Cryrivers provided to match with what is currently in redux master (the AnyAction type)
(as of reduxjs/redux#2773)

Looks like it's a constantly moving target at the moment, but I'd say something like this might be the direction to go in to keep the generics in similar order to Redux itself.

import { Middleware, Dispatch, Action, AnyAction } from 'redux';

export type ThunkAction<R, S, A extends Action = AnyAction, E = {}> = {
    (dispatch: Dispatch<A, S>, getState: () => S, extraArgument: E): R
}

declare module 'redux' {
    export interface Dispatch<A extends Action = AnyAction, S = {}> {
        <R, E>(thunk: ThunkAction<R, S, A, E>): R
    }
}

declare const thunk: Middleware & {
    withExtraArgument(extraArgument: any): Middleware;
};

export default thunk;

Changes the contract a little bit, but then again it's a major version of Redux so that's probably not the end of the world.

I don't write Typescript definitions often so feel free to let me know if i'm missing anything.

@NathanNZ
Copy link

Would a contributor be willing to release a beta version of redux-thunk that targets "4.0.0-beta.2" if I raised a pull request?

Apart from TypeScript I doubt we'll see any changes that break the thunk middleware functionality (reduxjs/redux#1342)

@Cryrivers
Copy link
Contributor

Cryrivers commented Feb 26, 2018

@NathanNZ ok, let me try.

EDIT: Pretty much given up, as I found it hard to support type-safe withExtraArguments.

@Cryrivers
Copy link
Contributor

Just created a PR. Please help review. Thank you guys.

@Ky6uk
Copy link

Ky6uk commented Apr 18, 2018

Redux v4 is out. 🎉

@arjanfrans
Copy link

So this basically makes the library unusable, as long as this is not fixed...

@vovkasm
Copy link

vovkasm commented Apr 27, 2018

I just realized, that all sort of this problems exposed by library that is 14 lines of code )))))
I has a plans to return to this problem near weekend and try convert one project... will see how it behave now...

@labs-dlugo
Copy link

Ok guy so sometimes typescript's inference doesn't plain work. Here's the best I could do:

redux-thunk/index.d.ts:

import {Middleware, Dispatch, Action, Store} from "redux";

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

export interface ThunkExt {
  dispatch: <A extends Action, S, E, R>(asyncAction: ThunkAction<A, S, E, R>) => R;
}

declare const thunk: Middleware<ThunkExt> & {
  withExtraArgument(extraArgument: any): Middleware<ThunkExt>;
};

export default thunk;

(original ThunkAction's typings wrong, they have the same template parameter for both the Action typeparam to Redux.Dispatch and the State typeparam for getState())
So first we fix ThunkAction, then we remove Redux.Dispatch redeclaration as Redux V4 has a built in API for dispatch augmentations:

https://github.com/reactjs/redux/blob/3b600993e91d42d1569994964e9a13606edccdf0/index.d.ts#L221-L224

https://github.com/reactjs/redux/blob/3b600993e91d42d1569994964e9a13606edccdf0/index.d.ts#L302-L304

In store.ts however the dispatchMethod doesn't get picked up unless you explicitly pass in the Ext typeparam to the StoreCreator function, that's why we export it in redux-thunk/index.d.ts.
Again, typescript inference should pick up ThunkExt from Middleware passed in to the createStore function, but for some reason it doesn't, it'd be a pretty clean solution if typescript correctly performed the inference:

import {createStore} from 'redux'
import thunk from 'redux-thunk'
const store = (fakeReducer, applyMiddleware(thunk))
store.dispatch(
  makeASandwichWithSecretSauce('Me') // This however raises an error.
);

If we however explicitly pass in the typeparams into the createStore function, it all works:

import {createStore} from 'redux'
import thunk, {ThunkExt} from 'redux-thunk'
const store = createStore<IMyState, MyActions, ThunkExt, {}>(fakeReducer, applyMiddleware(thunk))
store.dispatch(
  makeASandwichWithSecretSauce('Me') // Now even the return value works (see full file below)
);

What I personally did is to type ThunkExt in a different way to "freeze" the State type and Actions type, and forgot about passing it to the Middleware<> type since typescript isn't picking it up there anyways.
redux-thunk/index.d.ts:

import {Middleware, Dispatch, Action, Store} from "redux";

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

export interface ThunkExt<A extends Action, S> {
  dispatch: <E, R>(asyncAction: ThunkAction<A, S, E, R>) => R;
}

declare const thunk: Middleware & {
  withExtraArgument(extraArgument: any): Middleware;
};

export default thunk;

Then in my store.ts:

const store = createStore<IMyState, MyActions, ThunkExt<MyActions, IMyState>, {}>(fakeReducer, applyMiddleware(thunk))
store.dispatch(
  makeASandwichWithSecretSauce('My wife')
).then((makeSandwichAction) => {
  assert(makeSandwichAction.type == 'MAKE_SANDWICH')
});

full store.ts:

import { createStore, Dispatch, applyMiddleware, Reducer } from 'redux';
import thunk, { ThunkAction, ThunkExt } from 'redux-thunk';

interface IMyState { }

interface IMakeSandwichAction {
  type: 'MAKE_SANDWICH'
  forPerson: string
  secretSauce: Response
}

type MyActions = IMakeSandwichAction

const fakeReducer: Reducer<IMyState, MyActions> = (state, action) => {
  if (state) {
    return state
  }
  else {
    return {}
  }
}

const store = createStore<IMyState, MyActions, ThunkExt<MyActions, IMyState>, {}>(fakeReducer, applyMiddleware(thunk))

function fetchSecretSauce(): Promise<Response> {
  return fetch('https://www.google.com/search?q=secret+sauce');
}

function makeASandwich(forPerson: string, secretSauce: Response): IMakeSandwichAction {
  return {
    type: 'MAKE_SANDWICH',
    forPerson,
    secretSauce
  };
}

function makeASandwichWithSecretSauce(forPerson: string): ThunkAction<MyActions, IMyState, undefined, Promise<IMakeSandwichAction>> {
  return function (dispatch: Dispatch<MyActions>) {
    return fetchSecretSauce().then(
      sauce => dispatch(makeASandwich(forPerson, sauce))
    );
  };
}

store.dispatch(
  makeASandwichWithSecretSauce('Me') // typescript error
);

store.dispatch(
  makeASandwichWithSecretSauce('My wife')
).then((makeSandwichAction) => {
  assert(makeSandwichAction.type == 'MAKE_SANDWICH')
});

Another alternative:

const store: Store<IMyState, MyActions> & ThunkExt<MyActions, IMyState> = createStore(fakeReducer, applyMiddleware(thunk))

Hope this is useful for anyone and I might be onto something using redux's built in types for store enhancing which seem correct but typescript isn't picking up all types when it should.

@labs-dlugo
Copy link

Another simple hack that doesn't require messing with messy types or declaration files, I had to use it in order to be able to use react-redux's dispatch function which doesn't have an extension API built in:

However note that your thunk cannot dispatch itself

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

export type FetchBooksThunk =
  ThunkAction<MyAction, IMyStore, undefined, void> & IFetchBooksThunk

interface IFetchBooksThunk {
  type: 'FETCH_BOOKS_THUNK'
}

export type PlainActions = IFetchStart | IFetchComplete | IFetchFailed

// if you do this typescript rightly complains about circular referencing:
// export type PlainActions = IFetchStart | IFetchComplete | IFetchFailed | FetchBooksThunk


const actionify =
  <AT, A extends Action, S, E, R>(actionType: AT, thunk: ThunkAction<A,S,E,R>)
    :Action<AT> & ThunkAction<A,S,E,R> =>
      Object.assign(thunk, {type: actionType})

const fetchCurrentTopic: FetchBooksThunk = actionify<'FETCH_BOOKS_THUNK', PlainActions, IMyStore, undefined, void>('FETCH_BOOKS_THUNK', (dispatch, getState) => {
  dispatch(fetchStart())
  const { topic } = getState()

  fetch(URL + topic)
    .then(res => {
      return res.json()
    })
    .then((json: IAPIResponse) => {
      if(json.error) {
        dispatch(fetchFailed(json.error))
      }
      else {
        dispatch(fetchComplete(json))
      }
    })
    .catch(error => {
      dispatch(fetchFailed(error))
    })
})

@ralexand56
Copy link

Are there any plans to fix this anytime soon?

@typeofweb
Copy link

@danlugo92 do you think you could make a PR?

@nschulzke
Copy link

I just made a pull request that fixes the issue while leaving the interface basically intact.

Pull request: #196

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

Successfully merging a pull request may close this issue.