-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix typings for Redux v4 #196
Conversation
Fix typings for Redux v4
The build failed. Could you also fix https://github.com/reduxjs/redux-thunk/blob/master/test/typescript.ts ? |
It looks like now there are dependency issues. The tests pass on my computer, but are failing to build online. Not sure what to do about it, it looks like the new version of typescript-definition-tester has some ES6 code that breaks the build online. |
@@ -8,7 +8,7 @@ store.dispatch(dispatch => { | |||
dispatch({type: 'FOO'}); | |||
}); | |||
|
|||
store.dispatch((dispatch, getState) => { | |||
store.dispatch((dispatch, getState: () => {foo: string}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type for getState
here should not be needed.
Does it not work without it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Redux v4, Dispatch doesn't take any state object as a generic type. So as far as I can tell, there's no way to get the Store's State type into the Thunk arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, okay, I pulled it and now I see the problem.
Yeah, I'll stick to my overrides for now :( Thank you for your time.
My current code:
export type AppStore = Store<AppState> & {
dispatch<R>(asyncAction: AsyncAction<R>): R;
};
export type AsyncAction<R = any> = (dispatch: AppStore['dispatch'], getState: AppStore['getState']) => R;
// store.js
const store = createStore(…) as AppStore; // :(
There is another PR here to make typings compatible for redux 4. Not sure both are trying to do the same. |
Went with #180. Thanks! |
when is an npm release coming? |
Any news on this? |
I just re-wrote it according to my needs and used my own typings (different from this pull request). This library is almost unnecessary, what it does is so simple. Just take a look at the source at write your own. |
shouldn't this be a major release? |
IMHO it's just a bugfix. |
Fix for this issue: #169
The types API should remain identical. I just removed the type specifier on Dispatch in ThunkAction and updated the generics on Dispatch<> in the "redux" module to match v4.