8000 Improve TypeScript types by jednano · Pull Request #3566 · reduxjs/redux · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Improve TypeScript types #3566

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"prettier": "^1.18.2",
"rimraf": "^3.0.0",
"rollup": "^1.20.3",
"redux-thunk": "^2.3.0",
Copy link
Author

Choose a reason for hiding this comment

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

If we're going to be testing thunks, it only makes sense that this dev dependency would aid in that effort.

"rollup-plugin-babel": "^4.3.3",
"rollup-plugin-node-resolve": "^5.2.0",
"rollup-plugin-replace": "^2.2.0",
Expand Down
30 changes: 16 additions & 14 deletions src/applyMiddleware.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import compose from './compose'
import { Middleware, MiddlewareAPI } from './types/middleware'
import { AnyAction } from './types/actions'
import { StoreEnhancer, StoreCreator, Dispatch } from './types/store'
import { Reducer } from './types/reducers'
import {
Dispatch,
StoreEnhancer,
StoreEnhancerStoreCreator
} from './types/store'

/**
* Creates a store enhancer that applies middleware to the dispatch method
Expand All @@ -23,40 +25,40 @@ import { Reducer } from './types/reducers'
* @template Ext Dispatch signature added by a middleware.
* @template S The type of the state supported by a middleware.
*/
export default function applyMiddleware(): StoreEnhancer
export default function applyMiddleware(): StoreEnhancer<{ dispatch: {} }, {}>
export default function applyMiddleware<Ext1, S>(
middleware1: Middleware<Ext1, S, any>
): StoreEnhancer<{ dispatch: Ext1 }>
): StoreEnhancer<{ dispatch: Ext1 }, S>
export default function applyMiddleware<Ext1, Ext2, S>(
middleware1: Middleware<Ext1, S, any>,
middleware2: Middleware<Ext2, S, any>
): StoreEnhancer<{ dispatch: Ext1 & Ext2 }>
): StoreEnhancer<{ dispatch: Ext1 & Ext2 }, S>
export default function applyMiddleware<Ext1, Ext2, Ext3, S>(
middleware1: Middleware<Ext1, S, any>,
middleware2: Middleware<Ext2, S, any>,
middleware3: Middleware<Ext3, S, any>
): StoreEnhancer<{ dispatch: Ext1 & Ext2 & Ext3 }>
): StoreEnhancer<{ dispatch: Ext1 & Ext2 & Ext3 }, S>
export default function applyMiddleware<Ext1, Ext2, Ext3, Ext4, S>(
middleware1: Middleware<Ext1, S, any>,
middleware2: Middleware<Ext2, S, any>,
middleware3: Middleware<Ext3, S, any>,
middleware4: Middleware<Ext4, S, any>
): StoreEnhancer<{ dispatch: Ext1 & Ext2 & Ext3 & Ext4 }>
): StoreEnhancer<{ dispatch: Ext1 & Ext2 & Ext3 & Ext4 }, S>
export default function applyMiddleware<Ext1, Ext2, Ext3, Ext4, Ext5, S>(
middleware1: Middleware<Ext1, S, any>,
middleware2: Middleware<Ext2, S, any>,
middleware3: Middleware<Ext3, S, any>,
middleware4: Middleware<Ext4, S, any>,
middleware5: Middleware<Ext5, S, any>
): StoreEnhancer<{ dispatch: Ext1 & Ext2 & Ext3 & Ext4 & Ext5 }>
): StoreEnhancer<{ dispatch: Ext1 & Ext2 & Ext3 & Ext4 & Ext5 }, S>
export default function applyMiddleware<Ext, S = any>(
...middlewares: Middleware<any, S, any>[]
): StoreEnhancer<{ dispatch: Ext }>
export default function applyMiddleware(
): StoreEnhancer<{ dispatch: Ext }, S>
export default function applyMiddleware<S>(
...middlewares: Middleware[]
): StoreEnhancer<any> {
return (createStore: StoreCreator) => <S, A extends AnyAction>(
reducer: Reducer<S, A>,
): StoreEnhancer<any, S> {
return (createStore: StoreEnhancerStoreCreator<any>) => (
Copy link
Author

Choose a reason for hiding this comment

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

This StoreEnhancerStoreCreator type is new and more accurate than just a plain StoreCreator.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not groking why the StoreEnhancer's Extension here would be an any. We know what apply middleware does (it has a very specific API) and it doesn't do any actual extension of the store's properties. So, at the very least, shouldn't it be {}?

reducer,
...args: any[]
) => {
const store = createStore(reducer, ...args)
Expand Down
8 changes: 2 additions & 6 deletions src/createStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export default function createStore<
8000 throw new Error('Expected the reducer to be a function.')
}

let currentReducer = reducer
let currentReducer: Reducer<any, any> = reducer
let currentState = preloadedState as S
let currentListeners: (() => void)[] | null = []
let nextListeners = currentListeners
Expand Down Expand Up @@ -273,11 +273,7 @@ export default function createStore<
throw new Error('Expected the nextReducer to be a function.')
}

// TODO: do this more elegantly
Copy link
Author

Choose a reason for hiding this comment

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

The elegance is done by line 100, defining a Reducer<any, any>, so that we can keep overwriting it with new reducers w/o issues. This is fine too, because this particular type isn't being returned. It's only used internal to this function.

;((currentReducer as unknown) as Reducer<
NewState,
NewActions
>) = nextReducer
currentReducer = nextReducer

// This action has a similiar effect to ActionTypes.INIT.
// Any reducers that existed in both the new and old rootReducer
Expand Down
65 changes: 31 additions & 34 deletions test/applyMiddleware.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
import { createStore, applyMiddleware, Middleware, AnyAction, Action } from '..'
import {
createStore,
applyMiddleware,
Middleware,
AnyAction,
Action,
Store
} from '../src'
import { rootActions } from './helpers/actionTypes'
import * as reducers from './helpers/reducers'
import { addTodo, addTodoAsync, addTodoIfEmpty } from './helpers/actionCreators'
import { thunk } from './helpers/middleware'

describe('applyMiddleware', () => {
it('warns when dispatching during middleware setup', () => {
function dispatchingMiddleware(store) {
function dispatchingMiddleware(store: Store<any, rootActions>) {
store.dispatch(addTodo('Dont dispatch in middleware setup'))
return next => action => next(action)
}
Expand Down Expand Up @@ -40,8 +48,8 @@ describe('applyMiddleware', () => {
])
})

it('passes recursive dispatches through the middleware chain', () => {
function test(spyOnMethods) {
it('passes recursive dispatches through the middleware chain', async () => {
function test(spyOnMethods: jest.Mock) {
return () => next => action => {
spyOnMethods(action)
return next(action)
Expand All @@ -51,16 +59,11 @@ describe('applyMiddleware', () => {
const spy = jest.fn()
const store = applyMiddleware(test(spy), thunk)(createStore)(reducers.todos)

// the typing for redux-thunk is super complex, so we will use an as unknown hack
Copy link
Author

Choose a reason for hiding this comment

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

Now that the store is properly typed, there's no need to use any "hacks".

const dispatchedValue = (store.dispatch(
addTodoAsync('Use Redux')
) as unknown) as Promise<void>
return dispatchedValue.then(() => {
expect(spy.mock.calls.length).toEqual(2)
})
await store.dispatch(addTodoAsync('Use Redux'))
expect(spy.mock.calls.length).toEqual(2)
})

it('works with thunk middleware', done => {
it('works with thunk middleware', async () => {
const store = applyMiddleware(thunk)(createStore)(reducers.todos)

store.dispatch(addTodoIfEmpty('Hello'))
Expand Down Expand Up @@ -91,27 +94,21 @@ describe('applyMiddleware', () => {
}
])

// the typing for redux-thunk is super complex, so we will use an "as unknown" hack
const dispatchedValue = (store.dispatch(
addTodoAsync('Maybe')
) as unknown) as Promise<void>
dispatchedValue.then(() => {
expect(store.getState()).toEqual([
{
id: 1,
text: 'Hello'
},
{
id: 2,
text: 'World'
},
{
id: 3,
text: 'Maybe'
}
])
done()
})
await store.dispatch(addTodoAsync('Maybe'))
expect(store.getState()).toEqual([
{
id: 1,
text: 'Hello'
},
{
id: 2,
text: 'World'
},
{
id: 3,
text: 'Maybe'
}
])
})

it('passes through all arguments of dispatch calls from within middleware', () => {
Expand Down Expand Up @@ -144,7 +141,7 @@ describe('applyMiddleware', () => {
applyMiddleware(multiArgMiddleware, dummyMiddleware)
)

store.dispatch(spy)
store.dispatch(spy as any)
expect(spy.mock.calls[0]).toEqual(testCallArgs)
})
})
6 changes: 3 additions & 3 deletions test/bindActionCreators.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { bindActionCreators, createStore, ActionCreator } from '..'
import { bindActionCreators, createStore, ActionCreator, Store } from '..'
import { todos } from './helpers/reducers'
import * as actionCreators from './helpers/actionCreators'

describe('bindActionCreators', () => {
let store
let actionCreatorFunctions
let store: Store<typeof todos>
let actionCreatorFunctions: Partial<typeof actionCreators>

beforeEach(() => {
store = createStore(todos)
Expand Down
7 changes: 4 additions & 3 deletions test/createStore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ describe('createStore', () => {

it('throws if action type is missing', () => {
const store = createStore(reducers.todos)
// @ts-ignore
expect(() => store.dispatch({})).toThrow(
/Actions may not have an undefined "type" property/
)
Expand All @@ -519,10 +520,10 @@ describe('createStore', () => {

it('does not throw if action type is falsy', () => {
const store = createStore(reducers.todos)
expect(() => store.dispatch({ type: false })).not.toThrow()
expect(() => store.dispatch({ type: 0 })).not.toThrow()
expect(() => store.dispatch({ type: false } as any)).not.toThrow()
expect(() => store.dispatch({ type: 0 } as any)).not.toThrow()
expect(() => store.dispatch({ type: null })).not.toThrow()
expect(() => store.dispatch({ type: '' })).not.toThrow()
expect(() => store.dispatch({ type: '' } as any)).not.toThrow()
})

it('accepts enhancer as the third argument', () => {
Expand Down
46 changes: 32 additions & 14 deletions test/helpers/actionCreators.ts
Original file line number Diff line number Diff line change
@@ -1,71 +1,89 @@
import { ThunkAction } from 'redux-thunk'

import {
ADD_TODO,
AddTodo,
DISPATCH_IN_MIDDLE,
DispatchInMiddle,
GET_STATE_IN_MIDDLE,
GetStateInMiddle,
SUBSCRIBE_IN_MIDDLE,
SubscribeInMiddle,
UNSUBSCRIBE_IN_MIDDLE,
UnsubscribeInMiddle,
THROW_ERROR,
UNKNOWN_ACTION
ThrowError,
UNKNOWN_ACTION,
UnknownAction
} from './actionTypes'
import { Action, AnyAction, Dispatch } from '../..'

export function addTodo(text: string): AnyAction {
export function addTodo(text: string): AddTodo {
return { type: ADD_TODO, text }
}

export function addTodoAsync(text: string) {
return (dispatch: Dispatch): Promise<void> =>
new Promise(resolve =>
export function addTodoAsync(text: string): ThunkAction<any, any, {}, AddTodo> {
return dispatch =>
new Promise<void>(resolve =>
setImmediate(() => {
dispatch(addTodo(text))
resolve()
})
)
}

export function addTodoIfEmpty(text: string) {
return (dispatch: Dispatch, getState: () => any) => {
export function addTodoIfEmpty(
text: string
): ThunkAction<any, any, {}, AddTodo> {
return (dispatch, getState) => {
if (!getState().length) {
dispatch(addTodo(text))
}
}
}

export function dispatchInMiddle(boundDispatchFn: () => void): AnyAction {
export function dispatchInMiddle(
boundDispatchFn: () => void
): DispatchInMiddle {
return {
type: DISPATCH_IN_MIDDLE,
boundDispatchFn
}
}

export function getStateInMiddle(boundGetStateFn: () => void): AnyAction {
export function getStateInMiddle(
boundGetStateFn: () => void
): GetStateInMiddle {
return {
type: GET_STATE_IN_MIDDLE,
boundGetStateFn
}
}

export function subscribeInMiddle(boundSubscribeFn: () => void): AnyAction {
export function subscribeInMiddle(
boundSubscribeFn: () => void
): SubscribeInMiddle {
return {
type: SUBSCRIBE_IN_MIDDLE,
boundSubscribeFn
}
}

export function unsubscribeInMiddle(boundUnsubscribeFn: () => void): AnyAction {
export function unsubscribeInMiddle(
boundUnsubscribeFn: () => void
): UnsubscribeInMiddle {
return {
type: UNSUBSCRIBE_IN_MIDDLE,
boundUnsubscribeFn
}
}

export function throwError(): Action {
export function throwError(): ThrowError {
return {
type: THROW_ERROR
}
}

export function unknownAction(): Action {
export function unknownAction(): UnknownAction {
return {
type: UNKNOWN_ACTION
}
Expand Down
35 changes: 35 additions & 0 deletions test/helpers/actionTypes.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,42 @@
import { Action } from '../..'

export const ADD_TODO = 'ADD_TODO'
export interface AddTodo extends Action<typeof ADD_TODO> {
text: string
}

export const DISPATCH_IN_MIDDLE = 'DISPATCH_IN_MIDDLE'
export interface DispatchInMiddle extends Action<typeof DISPATCH_IN_MIDDLE> {
boundDispatchFn: () => void
}

export const GET_STATE_IN_MIDDLE = 'GET_STATE_IN_MIDDLE'
export interface GetStateInMiddle extends Action<typeof GET_STATE_IN_MIDDLE> {
boundGetStateFn: () => void
}

export const SUBSCRIBE_IN_MIDDLE = 'SUBSCRIBE_IN_MIDDLE'
export interface SubscribeInMiddle extends Action<typeof SUBSCRIBE_IN_MIDDLE> {
boundSubscribeFn: () => void
}

export const UNSUBSCRIBE_IN_MIDDLE = 'UNSUBSCRIBE_IN_MIDDLE'
export interface UnsubscribeInMiddle
extends Action<typeof UNSUBSCRIBE_IN_MIDDLE> {
boundUnsubscribeFn: () => void
}

export const THROW_ERROR = 'THROW_ERROR'
export type ThrowError = Action<typeof THROW_ERROR>

export const UNKNOWN_ACTION = 'UNKNOWN_ACTION'
export type UnknownAction = Action<typeof UNKNOWN_ACTION>

export type rootActions =
| AddTodo
| DispatchInMiddle
| GetStateInMiddle
| SubscribeInMiddle
| UnsubscribeInMiddle
| ThrowError
| UnknownAction
Loading
0