-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
Add TypeScript definitions #1413
Changes from 1 commit
889030c
c01501f
4b09a3b
6b4f7f6
b19ebc1
a6b4d80
56ab522
1fa6636
6b61fca
16325a2
6317727
a5d44fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
export interface ActionCreator { | ||
(...args: any[]): any; | ||
} | ||
|
||
export type Reducer<S> = (state: S, action: any) => S; | ||
|
||
export type Dispatch = (action: any) => any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't avoid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can. Just make it generic:
or if dispatch returns output changed by middleware:
This is one of typings that is necessary for strict typing of any redux based app. If dispatch can return anything and can't be parametrised then static typing is of no value here… There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do we use it? Considering first option we'd have: export interface Store<S> {
dispatch: Dispatch<any>;
// ...
} So the only place where type parameters would be useful is the app code, e.g. @connect(
state => /* ... */,
(dispatch: Dispatch<Action>) => {
return {
someActionCreator: () => dispatch(someActionCreator)
}
}
) But nothing prevents you to set up custom type MyDispatch = (action: MyAction) => MyAction; Then you can use your strict IMO if we add type parameters here, we'd have to write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see how your proposal can be useful: if we put type Dispatch<A, O> = (action: A) => O; Then original interface Store<S> {
dispatch: Dispatch<Action, Action>;
} Then we can strengthen interface Middleware {
<S, A, O>(api: MiddlewareAPI<S>):
<D extends Dispatch<any, any>>(next: D) => D | Dispatch<A, O>;
} Thus we could always have strong typings for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's what I've come up with: type StoreEnhancer<NewDispatch extends Dispatch<any, any>> =
<D extends Dispatch<any, any>>(next: StoreCreator<D>) =>
StoreCreator<D | NewDispatch>; It takes store creator with original dispatch and returns one where dispatch is either original or new. But I couldn't get it to work with enhancers that don't add new dispatch signature, e.g. logger middleware: const logger: Middleware</* ? */> = ({getState}) => {
return <D extends Dispatch<any, any>>(next: D) =>
(action: /* ? */) => {
console.log('will dispatch', action)
let returnValue = next(action)
console.log('state after dispatch', getState())
return returnValue
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fine for default dispatch (no middleware): type DefaultDispatch = <A extends Action>(action: A) => A; The problem here is that dispatch doesn't always return its input. Consider store.dispatch(dispatch => 42) // returns 42 After applying thunk middleware type ThunkDispatch = <O>(thunk: (dispatch, getState) => O) => O;
typeof store.dispatch == DefaultDispatch | ThunkDispatch; Things get worse when we try to define types for type ThunkDispatch =
<O, S>(thunk: (dispatch: DefaultDispatch, getState: () => S) => O) => O; But what if we had e.g. Promise middleware in front of type PromiseDispatch = <P extends Promise<any>>(promise: P): P;
type ThunkDispatch =
<O, S>(thunk: (dispatch: DefaultDispatch | PromiseDispatch, getState: () => S) => O) => O; This means that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tricky situation - JS (and other dynamically typed langs) encourages to build structures that accept anything and handle type inside (runtime reflection) while static typing is about splitting it to handle different inputs/outputs in separate functions/methods. Thunk (and probably many other) middleware is so elastic, that best solution is to use just: interface Dispatch extends Function {
<A>(action: A) => A;
<A, B>(action: A) => B;
} And use it with thunk: const dispatch: Dispatch;
…
… dispatch<any>() … There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also consider this: type ThunkDispatch =
<O, S>(thunk: (dispatch: DefaultDispatch | PromiseDispatch, getState: () => S) => O; is exactly same as: type Thunk<S, O> = (dispatch: Dispatch, getState: () => S) => O;
type ThunkDispatch = <S, O>(thunk: Thunk<S, O>) => O; Which could be replaced with: type ThunkDispatch = <Th, O>(thunk: Th) => O;
// Just set type parameter `Th` as `Thunk<SomeState, TheOutput>` and `O` as `Output` Which is exactly same as second overload here (: interface Dispatch extends Function {
<A>(action: A) => A;
<A, B>(action: A) => B;
} Proposed promise dispatch is actually dispatch: type PromiseDispatch = <P extends Promise<any>>(promise: P): P;
// what is
const dispatch: Dispatch;
… dispatch<Promise<any>>(promise); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes more sense for me now. Thanks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right. I think it's the best we can do - will satisfy both strict typing freaks (like me) and elasticity lovers. I think the overloaded one does the job in a best possible way: interface Dispatch extends Function {
<A>(action: A): A;
<T, O>(action: T): O;
} |
||
|
||
export interface MiddlewareAPI<S> { | ||
dispatch: Dispatch; | ||
getState: () => S; | ||
} | ||
|
||
export interface Middleware { | ||
<S>(api: MiddlewareAPI<S>): (next: Dispatch) => Dispatch; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Middleware definition below seems to be enough, but… interface Middleware<S> extends Function {
(store: MiddlewareAPI<S>): (next: Dispatch) => Dispatch;
} It actually does not return a dispatch but function mapping input of type A to output of type B: interface Middleware<S, A, B> extends Function {
(store: MiddlewareAPI<S>): (next: Dispatch) => (action: A) => B;
} But in this case interface Middleware extends Function {
<S, A, B>(store: MiddlewareAPI<S>): (next: Dispatch) => (action: A) => B;
} It's not always so easy to add static type definitions to code written in dynamically typed language… ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's try to implement const thunkMiddleware = ({dispatch, getState}) =>
(next) => (action) => {
return typeof action === function ? action(dispatch, getState) : next(action)
} Now what types can we add here? Keep in mind that there may me other middlewares applied before There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // in our typings
interface Middleware<S, A, B> extends Function {
(store: MiddlewareAPI<S>): (next: Dispatch) => (action: A) => B;
} import { MyState } from './wherever-my-state-is-declared'
type ThunkAction = ((d: Dispatch, gs: () => MyState) => ThunkAction) | Object;
const thunkMiddleware: Middleware<MyStore, ThunkAction, ThunkAction> =
({dispatch, getState}) =>
(next) => (action) => {
return typeof action === function ? action(dispatch, getState) : next(action)
} Does it do the job ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It shows that elasticity of import { MyState } from './wherever-my-state-is-declared';
const thunkMiddleware: Middleware<MyStore, any, any> =
({dispatch, getState}) =>
(next) => (action) => {
return typeof action === function ? action(dispatch, getState) : next(action)
} ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is — |
||
|
||
export class Store<S> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not a class because you can not import it from Redux (or 'redux') lib… Trying this:
or just
will compile but throw an error in runtime - this is makes static typing a bit useless :( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. Not sure how it became a class here. |
||
dispatch: Dispatch; | ||
getState: () => S; | ||
subscribe: (listener: () => void) => () => void; | ||
replaceReducer: (reducer: Reducer<S>) => void; | ||
} | ||
|
||
export type StoreCreator<S> = (reducer: Reducer<S>, initialState?: S) => Store<S>; | ||
|
||
export type StoreEnhancer = <S>(next: StoreCreator<S>) => StoreCreator<S>; | ||
|
||
export function createStore<S>(reducer: Reducer<S>, initialState?: S, | ||
enhancer?: StoreEnhancer): Store<S>; | ||
|
||
export function bindActionCreators<T extends ActionCreator|{[key: string]: ActionCreator}>(actionCreators: T, dispatch: Dispatch): T; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Action creators:I think that overloading is much better than union: // or maybe "<A extends Action>" but due to middleware issues better without "extends Action"
export interface ActionCreator<A> extends Function {
(...args: any[]): A;
}
export interface ActionCreatorsMapObject {
[key: string]: <A>(...args: any[]) => A;
}
export interface ActionCreatorsBinder {
<A>(actionCreator: ActionCreator<A>, dispatch: Dispatch): ActionCreator<A>;
<M extends ActionCreatorsMapObject>(actionCreators: M, dispatch: Dispatch): M;
(actionCreators: ActionCreatorsMapObject, dispatch: Dispatch): ActionCreatorsMapObject;
}
export function bindActionCreators: ActionCreatorsBinder; I've tested it with such piece of useless code: const oneCreatorA = bindActionCreators<Action>(a => a, store.dispatch);
const oneCreatorB = bindActionCreators(oneCreatorA, store.dispatch);
const oneCreatorC = bindActionCreators(oneCreatorB, store.dispatch);
interface Ott extends ActionCreatorsMapObject {
one: IActionCreator<Action>;
two: IActionCreator<Action>;
three: IActionCreator<Action>;
four: IActionCreator<Action>;
}
const xCreatorA = bindActionCreators({
one: a => a,
two(b: Action) {return b;},
three: oneCreatorA
}, store.dispatch);
const cMap: Ott = {
one: a => a,
two(b: Action) {return b;},
three: oneCreatorA,
four: oneCreatorC
};
const xCreatorB: ActionCreatorsMapObject = bindActionCreators(xCreatorA, store.dispatch);
const xCreatorC: Ott = bindActionCreators(cMap, store.dispatch); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Third overload seems redundant here, we can omit type parameter in second overload and it will be inferred as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also consider the difference between <A>(actionCreator: ActionCreator<A>, dispatch: Dispatch): ActionCreator<A>; and <A extends ActionCreator<any>>(actionCreator: A, dispatch: Dispatch): A; I guess the latter is stronger because let's you constraint not only return type of ActionCreator, but its full signature including argument types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Second one here: <A extends ActionCreator<any>>(actionCreator: A, dispatch: Dispatch): A; is not typesafe as it enforces I think we should find better way… There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aikoven also you said
And that is great :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand. This signature allows doing bindActionCreators<ActionCreator<MyAction>>(...) and even bindActionCreators<(text: string) => MyAction>(...) So type parameter is not enforced. In contrast, if we used this signature: <A>(actionCreator: ActionCreator<A>, dispatch: Dispatch): ActionCreator<A>; to bind action creator of type Also, I found that we don't cover cases when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, you're right. My mistake: <A extends ActionCreator<any>>(actionCreator: A, dispatch: Dispatch): A; Is totally ok.
So maybe third overload: <A extends ActionCreator<any>, B extends ActionCreator<any>>(
actionCreator: A,
dispatch: Dispatch
): B; |
||
export function combineReducers<S>(reducers: {[key: string]: Reducer<any>}): Reducer<S>; | ||
export function applyMiddleware<S>(...middlewares: Middleware[]): StoreEnhancer; | ||
|
||
// from DefinitelyTyped/compose-function | ||
// Hardcoded signatures for 2-4 parameters | ||
export function compose<A, B, C>(f1: (b: B) => C, | ||
f2: (a: A) => B): (a: A) => C; | ||
export function compose<A, B, C, D>(f1: (b: C) => D, | ||
f2: (a: B) => C, | ||
f3: (a: A) => B): (a: A) => D; | ||
export function compose<A, B, C, D, E>(f1: (b: D) => E, | ||
f2: (a: C) => D, | ||
f3: (a: B) => C, | ||
f4: (a: A) => B): (a: A) => E; | ||
|
||
// Minimal typing for more than 4 parameters | ||
export function compose<Result>(f1: (a: any) => Result, | ||
...functions: Function[]): (a: any) => Result; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it can be even better if we decide what should be the input type of composed function: export function compose<R, I>(fn1: (arg: any) => R,
...functions: Function[]): (arg: I) => R;
// I for input, R for Result |
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.
It would be great if try to not use too much
any
- it's just like working without support of typscript… :(Consider more strict approach like:
Or even better
so anyone can implement his own strict actions:
or dynamic ones:
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.
Why isn't this sufficient?
If you want to have strict actions you can just do:
And it will be assignable to
Action
.Also, reducer should not be constrained to accept only some selected actions. It should accept any possible action and bypass ones it doesn't need to handle.