Skip to content

Add state type to dispatch interface #1537

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 4, 2016
Merged

Add state type to dispatch interface #1537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74e16aa
Type safety for dispatch signature
Mar 17, 2016
b0d60c4
Add state type to Dispatch interface and improve thunk test with prop…
Igorbek Mar 18, 2016
2d29481
Fix line ending.
Igorbek Mar 18, 2016
e767e13
Improve basic dispatch signature. Fix typescript tests.
Igorbek Mar 22, 2016
658fa6f
Fix typescript tests of middlewares.
Igorbek Mar 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ export function combineReducers<S>(reducers: ReducersMapObject): Reducer<S>;
* transform, delay, ignore, or otherwise interpret actions or async actions
* before passing them to the next middleware.
*/
export type Dispatch = (action: any) => any;
export interface Dispatch<S> {
<A extends Action>(action: A): A;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here Dispatch<S> is defined as a paramtrized type (generic) but the time parameter from signature is not used. @Igorbek can you explain the purpose?

Also I think that after a rather long discussion we have decided to drop type parameters from <A extends Action>(action: A): A; due to middleware issues.

See my initial proposition -> https://gist.github.com/ulfryk/69981ccfb488647b6ee3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having state type parameter S allows middlewares be more accurate in typings. For example, thunk function takes a dispatch and a getState(): S, so without S it couldn't be well-typed. See the example of this here below https://github.com/Igorbek/redux/blob/ts-def-improv/test/typescript/dispatch.ts#L9-L16

Regarding second question, I think this signature express exactly what initial (without midllewares) dispatch does. It take an action and returns it. If I used less generic signature (action: Action) => Action, due to TypeScript rules it would fail on object literals, such as dispatch({ type: 'ADD_TODO', text: 'todo' }) (Object literals may only specify known properties). Moreover, it returns more precise result - typed action that was passed.


/**
* Function to remove listener added by `Store.subscribe()`.
Expand Down Expand Up @@ -136,7 +138,7 @@ export interface Store<S> {
* Note that, if you use a custom middleware, it may wrap `dispatch()` to
* return something else (for example, a Promise you can await).
*/
dispatch: Dispatch;
dispatch: Dispatch<S>;

/**
* Reads the state tree managed by the store.
Expand Down Expand Up @@ -251,7 +253,7 @@ export const createStore: StoreCreator;
/* middleware */

export interface MiddlewareAPI<S> {
dispatch: Dispatch;
dispatch: Dispatch<S>;
getState(): S;
}

Expand All @@ -265,7 +267,7 @@ export interface MiddlewareAPI<S> {
* asynchronous API call into a series of synchronous actions.
*/
export interface Middleware {
<S>(api: MiddlewareAPI<S>): (next: Dispatch) => (action: any) => any;
<S>(api: MiddlewareAPI<S>): (next: Dispatch<S>) => Dispatch<S>;
}

/**
Expand Down Expand Up @@ -337,19 +339,19 @@ export interface ActionCreatorsMapObject {
* creator wrapped into the `dispatch` call. If you passed a function as
* `actionCreator`, the return value will also be a single function.
*/
export function bindActionCreators<A extends ActionCreator<any>>(actionCreator: A, dispatch: Dispatch): A;
export function bindActionCreators<A extends ActionCreator<any>>(actionCreator: A, dispatch: Dispatch<any>): A;

export function bindActionCreators<
A extends ActionCreator<any>,
B extends ActionCreator<any>
>(actionCreator: A, dispatch: Dispatch): B;
>(actionCreator: A, dispatch: Dispatch<any>): B;

export function bindActionCreators<M extends ActionCreatorsMapObject>(actionCreators: M, dispatch: Dispatch): M;
export function bindActionCreators<M extends ActionCreatorsMapObject>(actionCreators: M, dispatch: Dispatch<any>): M;

export function bindActionCreators<
M extends ActionCreatorsMapObject,
N extends ActionCreatorsMapObject
>(actionCreators: M, dispatch: Dispatch): N;
>(actionCreators: M, dispatch: Dispatch<any>): N;


/* compose */
Expand Down
6 changes: 3 additions & 3 deletions test/typescript/actionCreators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ const addTodo: ActionCreator<AddTodoAction> = (text: string) => ({

const addTodoAction: AddTodoAction = addTodo('test');

type AddTodoThunk = (dispatch: Dispatch) => AddTodoAction;
type AddTodoThunk = (dispatch: Dispatch<any>) => AddTodoAction;

const addTodoViaThunk: ActionCreator<AddTodoThunk> = (text: string) =>
(dispatch: Dispatch) => ({
(dispatch: Dispatch<any>) => ({
type: 'ADD_TODO',
text
})

declare const dispatch: Dispatch;
declare const dispatch: Dispatch<any>;

const boundAddTodo = bindActionCreators(addTodo, dispatch);

Expand Down
12 changes: 8 additions & 4 deletions test/typescript/dispatch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import {Dispatch, Action} from "../../index.d.ts";


declare const dispatch: Dispatch;

declare const dispatch: Dispatch<any>;

const dispatchResult: Action = dispatch({type: 'TYPE'});


type Thunk<O> = () => O;
// thunk
declare module "../../index.d.ts" {
export interface Dispatch<S> {
<R>(asyncAction: (dispatch: Dispatch<S>, getState: () => S) => R): R;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ulfryk see dispatch's augmentation here that uses state type S.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to augment Dispatch interface without using ambient modules?
We won't be able to use them e.g. when we add official typings for redux-thunk.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see from microsoft/TypeScript#6213 external module augmentation should work by doing

// redux-thunk/index.d.ts
declare module "redux" {
  export interface Dispatch<S> {
    // ...
  }
}

@Igorbek could you please check this? If it works, I think this PR is good to merge.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Igorbek Sorry, I just realized that this is actually not ambient but external module augmentation introduced in TS 1.8 :)
Still we need to clarify if it works well with Node-style module resolution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right, that was my intent to define additional dispatch signature through module augmentation for "redux" module. And yes, it will work perfectly with node module resolution. I cannot write a test that uses "redux", because within the package we haven't such module.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me :)


const dispatchThunkResult: number = dispatch(() => 42);
const dispatchedTimerId: number = dispatch(d => setTimeout(() => d({type: 'TYPE'}), 1000));
17 changes: 11 additions & 6 deletions test/typescript/middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,26 @@ import {
applyMiddleware, createStore, Dispatch, Reducer, Action
} from "../../index.d.ts";

declare module "../../index.d.ts" {
export interface Dispatch<S> {
<R>(asyncAction: (dispatch: Dispatch<S>, getState: () => S) => R): R;
}
}

type Thunk<S, O> = (dispatch: Dispatch, getState?: () => S) => O;
type Thunk<S, O> = (dispatch: Dispatch<S>, getState: () => S) => O;

const thunkMiddleware: Middleware =
<S>({dispatch, getState}: MiddlewareAPI<S>) =>
(next: Dispatch) =>
<A, B>(action: A | Thunk<S, B>): B =>
(next: Dispatch<S>) =>
<A extends Action, B>(action: A | Thunk<S, B>): B|Action =>
typeof action === 'function' ?
(<Thunk<S, B>>action)(dispatch, getState) :
<B>next(<A>action)
next(<A>action)


const loggerMiddleware: Middleware =
<S>({getState}: MiddlewareAPI<S>) =>
(next: Dispatch) =>
(next: Dispatch<S>) =>
(action: any): any => {
console.log('will dispatch', action)

Expand Down Expand Up @@ -47,7 +52,7 @@ const storeWithThunkMiddleware = createStore(
);

storeWithThunkMiddleware.dispatch(
(dispatch: Dispatch, getState: () => State) => {
(dispatch, getState) => {
const todos: string[] = getState().todos;
dispatch({type: 'ADD_TODO'})
}
Expand Down