From f482096dc54638fb2211d673d762f3d051fa271e Mon Sep 17 00:00:00 2001 From: Boris Sergeyev Date: Sat, 3 Nov 2018 19:20:07 +0100 Subject: [PATCH 1/4] [react-redux] implemented proper typings for thunk action creators in TDispatchProps --- types/react-redux/index.d.ts | 38 +++++++++++++++++++++---- types/react-redux/react-redux-tests.tsx | 23 +++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/types/react-redux/index.d.ts b/types/react-redux/index.d.ts index 5cbde2e8db..0676941ce9 100644 --- a/types/react-redux/index.d.ts +++ b/types/react-redux/index.d.ts @@ -1,4 +1,4 @@ -// Type definitions for react-redux 6.0.4 +// Type definitions for react-redux 6.0.5 // Project: https://github.com/reduxjs/react-redux // Definitions by: Qubo , // Kenzie Togami , @@ -11,6 +11,7 @@ // Valentin Descamps // Johann Rakotoharisoa // Anatoli Papirovski +// Boris Sergeyev // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped // TypeScript Version: 2.8 @@ -102,7 +103,7 @@ type ConnectedComponentClass = ComponentClass { - >>>( + >>>( component: C ): ConnectedComponentClass, keyof Shared>> & TNeedsProps> } @@ -113,6 +114,19 @@ export interface InferableComponentEnhancerWithProps = InferableComponentEnhancerWithProps +// redux-thunk middleware returns thunk's return value from dispatch call +type WithThunkActionCreators = + TDispatchProps extends object + ? { + [C in keyof TDispatchProps]: C extends (...args: any[]) => any + ? ReturnType extends (...args: any[]) => any + ? ReturnType> + : C + : never + } + : TDispatchProps + + /** * Connects a React component to a Redux store. * @@ -142,12 +156,18 @@ export interface Connect { ( mapStateToProps: null | undefined, mapDispatchToProps: MapDispatchToPropsParam - ): InferableComponentEnhancerWithProps; + ): InferableComponentEnhancerWithProps< + WithThunkActionCreators, + TOwnProps + >; ( mapStateToProps: MapStateToPropsParam, mapDispatchToProps: MapDispatchToPropsParam - ): InferableComponentEnhancerWithProps; + ): InferableComponentEnhancerWithProps< + TStateProps & WithThunkActionCreators, + TOwnProps + >; ( mapStateToProps: MapStateToPropsParam, @@ -185,14 +205,20 @@ export interface Connect { mapDispatchToProps: MapDispatchToPropsParam, mergeProps: null | undefined, options: Options<{}, TStateProps, TOwnProps> - ): InferableComponentEnhancerWithProps; + ): InferableComponentEnhancerWithProps< + WithThunkActionCreators, + TOwnProps + >; ( mapStateToProps: MapStateToPropsParam, mapDispatchToProps: MapDispatchToPropsParam, mergeProps: null | undefined, options: Options - ): InferableComponentEnhancerWithProps; + ): InferableComponentEnhancerWithProps< + TStateProps & WithThunkActionCreators, + TOwnProps + >; ( mapStateToProps: MapStateToPropsParam, diff --git a/types/react-redux/react-redux-tests.tsx b/types/react-redux/react-redux-tests.tsx index 3b25b25843..a5f8c2b4f7 100644 --- a/types/react-redux/react-redux-tests.tsx +++ b/types/react-redux/react-redux-tests.tsx @@ -99,6 +99,29 @@ namespace MapDispatch { const verifyUndefined = } +namespace MapDispatchWithThunkActionCreators { + interface OwnProps { foo: string } + interface DispatchProps { + onClick(): void, + thunkAction(): Promise + } + + class TestComponent extends Component {} + + const mapDispatchToProps = () => ({ + onClick: () => {}, + thunkAction: () => async () => {} + }) + + const Test = connect( + null, + mapDispatchToProps, + )(TestComponent) + + const verify = +} + + namespace MapStateAndDispatchObject { interface ClickPayload { count: number } const onClick: ActionCreator = () => ({ count: 1 }); From 57ba1bcd755c8bc2aac0e74dadc50215d9127bed Mon Sep 17 00:00:00 2001 From: Boris Sergeyev Date: Mon, 5 Nov 2018 23:09:07 +0100 Subject: [PATCH 2/4] [react-redux] fixed error in typings for thunk action creators As noted in https://github.com/DefinitelyTyped/DefinitelyTyped/pull/30255#issuecomment-435814014 the key was used where value was expected. The test didn't catched that because `never` does not interact with `Shared` higher-order type in a way that I've expected. So I've avoided using `never` and edited test a bit. --- types/react-redux/index.d.ts | 20 +++++++++++--------- types/react-redux/react-redux-tests.tsx | 9 ++++----- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/types/react-redux/index.d.ts b/types/react-redux/index.d.ts index 0676941ce9..d1889b54fe 100644 --- a/types/react-redux/index.d.ts +++ b/types/react-redux/index.d.ts @@ -114,17 +114,19 @@ export interface InferableComponentEnhancerWithProps = InferableComponentEnhancerWithProps +type HandleThunkActionCreator = + TActionCreator extends (...args: any[]) => (...args: any[]) => any + ? ReturnType + : TActionCreator + // redux-thunk middleware returns thunk's return value from dispatch call +// https://github.com/reduxjs/redux-thunk#composition type WithThunkActionCreators = - TDispatchProps extends object - ? { - [C in keyof TDispatchProps]: C extends (...args: any[]) => any - ? ReturnType extends (...args: any[]) => any - ? ReturnType> - : C - : never - } - : TDispatchProps + TDispatchProps extends { [key: string]: any } + ? { + [C in keyof TDispatchProps]: HandleThunkActionCreator + } + : TDispatchProps /** diff --git a/types/react-redux/react-redux-tests.tsx b/types/react-redux/react-redux-tests.tsx index a5f8c2b4f7..a2fad70440 100644 --- a/types/react-redux/react-redux-tests.tsx +++ b/types/react-redux/react-redux-tests.tsx @@ -99,14 +99,13 @@ namespace MapDispatch { const verifyUndefined = } + namespace MapDispatchWithThunkActionCreators { - interface OwnProps { foo: string } - interface DispatchProps { + class TestComponent extends Component<{ + foo: string, onClick(): void, thunkAction(): Promise - } - - class TestComponent extends Component {} + }>{} const mapDispatchToProps = () => ({ onClick: () => {}, From ea41934fb03b6253b574329347192fa34c7a185e Mon Sep 17 00:00:00 2001 From: Boris Sergeyev Date: Tue, 20 Nov 2018 22:41:35 +0100 Subject: [PATCH 3/4] [react-redux] improved linting --- types/react-redux/index.d.ts | 110 ++--- types/react-redux/react-redux-tests.tsx | 567 +++++++++++------------- types/react-redux/tslint.json | 80 +--- 3 files changed, 302 insertions(+), 455 deletions(-) diff --git a/types/react-redux/index.d.ts b/types/react-redux/index.d.ts index d1889b54fe..2f919581f4 100644 --- a/types/react-redux/index.d.ts +++ b/types/react-redux/index.d.ts @@ -1,4 +1,4 @@ -// Type definitions for react-redux 6.0.5 +// Type definitions for react-redux 6.0 // Project: https://github.com/reduxjs/react-redux // Definitions by: Qubo , // Kenzie Togami , @@ -43,15 +43,14 @@ import { } from 'redux'; // Omit taken from https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-8.html -type Omit = Pick>; +export type Omit = Pick>; export interface DispatchProp { dispatch: Dispatch; } -interface AdvancedComponentDecorator { - (component: ComponentType): ComponentClass; -} +export type AdvancedComponentDecorator = + (component: ComponentType) => ComponentClass; /** * A property P will be present if: @@ -65,7 +64,7 @@ interface AdvancedComponentDecorator { * - if property P is present in InjectedProps but does not extend the * DecorationTargetProps[P] definition, its definition will be that of InjectedProps[P] */ -type Matching = { +export type Matching = { [P in keyof DecorationTargetProps]: P extends keyof InjectedProps ? InjectedProps[P] extends DecorationTargetProps[P] ? DecorationTargetProps[P] @@ -83,7 +82,7 @@ type Matching = { * required by the decorated (right hand side) component. * But any property required by the decorated component must be satisfied by the injected property. */ -type Shared< +export type Shared< InjectedProps, DecorationTargetProps extends Shared > = { @@ -91,43 +90,41 @@ type Shared< }; // Infers prop type from component C -type GetProps = C extends ComponentType ? P : never; +export type GetProps = C extends ComponentType ? P : never; // Applies LibraryManagedAttributes (proper handling of defaultProps // and propTypes), as well as defines WrappedComponent. -type ConnectedComponentClass = ComponentClass> & { +export type ConnectedComponentClass = ComponentClass> & { WrappedComponent: C; -} +}; // Injects props and removes them from the prop requirements. // Will not pass through the injected props if they are passed in during // render. Also adds new prop requirements from TNeedsProps. -export interface InferableComponentEnhancerWithProps { - >>>( +export type InferableComponentEnhancerWithProps = + >>>( component: C - ): ConnectedComponentClass, keyof Shared>> & TNeedsProps> -} + ) => ConnectedComponentClass, keyof Shared>> & TNeedsProps>; // Injects props and removes them from the prop requirements. // Will not pass through the injected props if they are passed in during // render. export type InferableComponentEnhancer = - InferableComponentEnhancerWithProps + InferableComponentEnhancerWithProps; -type HandleThunkActionCreator = +export type HandleThunkActionCreator = TActionCreator extends (...args: any[]) => (...args: any[]) => any ? ReturnType - : TActionCreator + : TActionCreator; // redux-thunk middleware returns thunk's return value from dispatch call // https://github.com/reduxjs/redux-thunk#composition -type WithThunkActionCreators = +export type WithThunkActionCreators = TDispatchProps extends { [key: string]: any } ? { [C in keyof TDispatchProps]: HandleThunkActionCreator } - : TDispatchProps - + : TDispatchProps; /** * Connects a React component to a Redux store. @@ -149,6 +146,7 @@ type WithThunkActionCreators = * @param options */ export interface Connect { + // tslint:disable:no-unnecessary-generics (): InferableComponentEnhancer; ( @@ -193,6 +191,7 @@ export interface Connect { mapStateToProps: MapStateToPropsParam, mapDispatchToProps: MapDispatchToPropsParam, mergeProps: MergeProps, + options?: Options ): InferableComponentEnhancerWithProps; ( @@ -221,48 +220,37 @@ export interface Connect { TStateProps & WithThunkActionCreators, TOwnProps >; - - ( - mapStateToProps: MapStateToPropsParam, - mapDispatchToProps: MapDispatchToPropsParam, - mergeProps: MergeProps, - options: Options - ): InferableComponentEnhancerWithProps; + // tslint:enable:no-unnecessary-generics } /** * The connect function. See {@type Connect} for details. */ -export declare const connect: Connect; +export const connect: Connect; -interface MapStateToProps { - (state: State, ownProps: TOwnProps): TStateProps; -} +export type MapStateToProps = + (state: State, ownProps: TOwnProps) => TStateProps; -interface MapStateToPropsFactory { - (initialState: State, ownProps: TOwnProps): MapStateToProps; -} +export type MapStateToPropsFactory = + (initialState: State, ownProps: TOwnProps) => MapStateToProps; -type MapStateToPropsParam = MapStateToPropsFactory | MapStateToProps | null | undefined; +export type MapStateToPropsParam = MapStateToPropsFactory | MapStateToProps | null | undefined; -interface MapDispatchToPropsFunction { - (dispatch: Dispatch, ownProps: TOwnProps): TDispatchProps; -} +export type MapDispatchToPropsFunction = + (dispatch: Dispatch, ownProps: TOwnProps) => TDispatchProps; -type MapDispatchToProps = +export type MapDispatchToProps = MapDispatchToPropsFunction | TDispatchProps; -interface MapDispatchToPropsFactory { - (dispatch: Dispatch, ownProps: TOwnProps): MapDispatchToProps; -} +export type MapDispatchToPropsFactory = + (dispatch: Dispatch, ownProps: TOwnProps) => MapDispatchToProps; -type MapDispatchToPropsParam = MapDispatchToPropsFactory | MapDispatchToProps; +export type MapDispatchToPropsParam = MapDispatchToPropsFactory | MapDispatchToProps; -interface MergeProps { - (stateProps: TStateProps, dispatchProps: TDispatchProps, ownProps: TOwnProps): TMergedProps; -} +export type MergeProps = + (stateProps: TStateProps, dispatchProps: TDispatchProps, ownProps: TOwnProps) => TMergedProps; -interface Options extends ConnectOptions { +export interface Options extends ConnectOptions { /** * If true, implements shouldComponentUpdate and shallowly compares the result of mergeProps, * preventing unnecessary updates, assuming that the component is a “pure” component @@ -303,11 +291,12 @@ interface OptionsselectorFactory in the factoryOptions argument. */ -export declare function connectAdvanced( +export function connectAdvanced( + // tslint:disable-next-line no-unnecessary-generics selectorFactory: SelectorFactory, connectOptions?: ConnectOptions & TFactoryOptions ): AdvancedComponentDecorator; @@ -320,13 +309,10 @@ export declare function connectAdvancedselector to return that * previous object when appropriate. */ -export interface SelectorFactory { - (dispatch: Dispatch, factoryOptions: TFactoryOptions): Selector -} +export type SelectorFactory = + (dispatch: Dispatch, factoryOptions: TFactoryOptions) => Selector; -export interface Selector { - (state: S, ownProps: TOwnProps): TProps -} +export type Selector = (state: S, ownProps: TOwnProps) => TProps; export interface ConnectOptions { /** @@ -336,13 +322,13 @@ export interface ConnectOptions { * @default name => 'ConnectAdvanced('+name+')' * @param componentName */ - getDisplayName?: (componentName: string) => string + getDisplayName?: (componentName: string) => string; /** * Shown in error messages. Usually overridden by wrapper functions. * * @default 'connectAdvanced' */ - methodName?: string + methodName?: string; /** * If defined, a property named this value will be added to the props passed to the wrapped component. Its value * will be the number of times the component has been rendered, which can be useful for tracking down unnecessary @@ -350,27 +336,27 @@ export interface ConnectOptions { * * @default undefined */ - renderCountProp?: string + renderCountProp?: string; /** * Controls whether the connector component subscribes to redux store state changes. If set to false, it will only * re-render on componentWillReceiveProps. * * @default true */ - shouldHandleStateChanges?: boolean + shouldHandleStateChanges?: boolean; /** * The key of props/context to get the store. You probably only need this if you are in the inadvisable position of * having multiple stores. * * @default 'store' */ - storeKey?: string + storeKey?: string; /** * If true, stores a ref to the wrapped component instance and makes it available via getWrappedInstance() method. * * @default false */ - withRef?: boolean + withRef?: boolean; } export interface ProviderProps { @@ -392,4 +378,4 @@ export class Provider extends Component {} + class TestComponent extends React.Component {} - const Test = connect()(TestComponent) + const Test = connect()(TestComponent); - const verify = + const verify = ; } -namespace MapState { - interface OwnProps { foo: string } - interface StateProps { bar: number } +function MapState() { + interface OwnProps { foo: string; } + interface StateProps { bar: number; } - class TestComponent extends Component {} + class TestComponent extends React.Component {} const mapStateToProps = (_: any) => ({ bar: 1 - }) + }); const Test = connect( mapStateToProps - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } -namespace MapStateWithDispatchProp { - interface OwnProps { foo: string } - interface StateProps { bar: number, dispatch: Dispatch } +function MapStateWithDispatchProp() { + interface OwnProps { foo: string; } + interface StateProps { bar: number; dispatch: Dispatch; } - class TestComponent extends Component {} + class TestComponent extends React.Component {} const mapStateToProps = (_: any) => ({ bar: 1 - }) + }); const Test = connect( mapStateToProps - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } -namespace MapStateFactory { - interface OwnProps { foo: string } - interface StateProps { bar: number } +function MapStateFactory() { + interface OwnProps { foo: string; } + interface StateProps { bar: number; } - class TestComponent extends Component {} + class TestComponent extends React.Component {} const mapStateToProps = () => () => ({ bar: 1 - }) + }); const Test = connect( mapStateToProps - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } -namespace MapDispatch { - interface OwnProps { foo: string } - interface DispatchProps { onClick: () => void } +function MapDispatch() { + interface OwnProps { foo: string; } + interface DispatchProps { onClick: () => void; } - class TestComponent extends Component {} + class TestComponent extends React.Component {} const mapDispatchToProps = () => ({ onClick: () => {} - }) + }); const TestNull = connect( null, mapDispatchToProps, - )(TestComponent) + )(TestComponent); - const verifyNull = + const verifyNull = ; const TestUndefined = connect( undefined, mapDispatchToProps, - )(TestComponent) + )(TestComponent); - const verifyUndefined = + const verifyUndefined = ; } - -namespace MapDispatchWithThunkActionCreators { - class TestComponent extends Component<{ +function MapDispatchWithThunkActionCreators() { + class TestComponent extends React.Component<{ foo: string, onClick(): void, thunkAction(): Promise - }>{} + }> {} const mapDispatchToProps = () => ({ onClick: () => {}, thunkAction: () => async () => {} - }) + }); const Test = connect( null, mapDispatchToProps, - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } - -namespace MapStateAndDispatchObject { - interface ClickPayload { count: number } +function MapStateAndDispatchObject() { + interface ClickPayload { count: number; } const onClick: ActionCreator = () => ({ count: 1 }); const dispatchToProps = { onClick, }; - interface OwnProps { foo: string } - interface StateProps { bar: number } - interface DispatchProps { onClick: ActionCreator } + interface OwnProps { foo: string; } + interface StateProps { bar: number; } + interface DispatchProps { onClick: ActionCreator; } const mapStateToProps = (_: any, __: OwnProps): StateProps => ({ bar: 1 - }) + }); - class TestComponent extends Component {} + class TestComponent extends React.Component {} const Test = connect( mapStateToProps, dispatchToProps, - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } -namespace MapDispatchFactory { - interface OwnProps { foo: string } - interface DispatchProps { onClick: () => void } +function MapDispatchFactory() { + interface OwnProps { foo: string; } + interface DispatchProps { onClick: () => void; } - class TestComponent extends Component {} + class TestComponent extends React.Component {} const mapDispatchToPropsFactory = () => () => ({ onClick: () => {} - }) + }); const TestNull = connect( null, mapDispatchToPropsFactory, - )(TestComponent) + )(TestComponent); - const verifyNull = + const verifyNull = ; const TestUndefined = connect( undefined, mapDispatchToPropsFactory, - )(TestComponent) + )(TestComponent); - const verifyUndefined = + const verifyUndefined = ; } -namespace MapStateAndDispatch { - interface OwnProps { foo: string } - interface StateProps { bar: number } - interface DispatchProps { onClick: () => void } +function MapStateAndDispatch() { + interface OwnProps { foo: string; } + interface StateProps { bar: number; } + interface DispatchProps { onClick: () => void; } - class TestComponent extends Component {} + class TestComponent extends React.Component {} const mapStateToProps = () => ({ bar: 1 - }) + }); const mapDispatchToProps = () => ({ onClick: () => {} - }) + }); const Test = connect( mapStateToProps, mapDispatchToProps, - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } -namespace MapStateFactoryAndDispatch { - interface OwnProps { foo: string } - interface StateProps { bar: number } - interface DispatchProps { onClick: () => void } +function MapStateFactoryAndDispatch() { + interface OwnProps { foo: string; } + interface StateProps { bar: number; } + interface DispatchProps { onClick: () => void; } - const mapStateToPropsFactory = () => () =>({ + const mapStateToPropsFactory = () => () => ({ bar: 1 - }) + }); const mapDispatchToProps = () => ({ onClick: () => {} - }) + }); - class TestComponent extends Component {} + class TestComponent extends React.Component {} const Test = connect( mapStateToPropsFactory, mapDispatchToProps, - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } -namespace MapStateFactoryAndDispatchFactory { - interface OwnProps { foo: string } - interface StateProps { bar: number } - interface DispatchProps { onClick: () => void } +function MapStateFactoryAndDispatchFactory() { + interface OwnProps { foo: string; } + interface StateProps { bar: number; } + interface DispatchProps { onClick: () => void; } - const mapStateToPropsFactory = () => () =>({ + const mapStateToPropsFactory = () => () => ({ bar: 1 - }) + }); const mapDispatchToPropsFactory = () => () => ({ onClick: () => {} - }) + }); - class TestComponent extends Component {} + class TestComponent extends React.Component {} const Test = connect( mapStateToPropsFactory, mapDispatchToPropsFactory, - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } -namespace MapStateAndDispatchAndMerge { - interface OwnProps { foo: string } - interface StateProps { bar: number } - interface DispatchProps { onClick: () => void } +function MapStateAndDispatchAndMerge() { + interface OwnProps { foo: string; } + interface StateProps { bar: number; } + interface DispatchProps { onClick: () => void; } - class TestComponent extends Component {} + class TestComponent extends React.Component {} const mapStateToProps = () => ({ bar: 1 - }) + }); const mapDispatchToProps = () => ({ onClick: () => {} - }) + }); const mergeProps = (stateProps: StateProps, dispatchProps: DispatchProps) => ( - Object.assign({}, stateProps, dispatchProps) - ) + { ...stateProps, ...dispatchProps } + ); const Test = connect( mapStateToProps, mapDispatchToProps, mergeProps, - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } -namespace MapStateAndOptions { +function MapStateAndOptions() { interface State { state: string; } - interface OwnProps { foo: string } - interface StateProps { bar: number } - interface DispatchProps { dispatch: Dispatch } + interface OwnProps { foo: string; } + interface StateProps { bar: number; } + interface DispatchProps { dispatch: Dispatch; } - class TestComponent extends Component {} + class TestComponent extends React.Component {} const mapStateToProps = (state: State) => ({ bar: 1 - }) + }); const areStatePropsEqual = (next: StateProps, current: StateProps) => true; @@ -290,17 +290,17 @@ namespace MapStateAndOptions { pure: true, areStatePropsEqual, } - )(TestComponent) + )(TestComponent); - const verify = + const verify = ; } interface CounterState { counter: number; } -declare var increment: Function; +declare var increment: () => { type: string }; -class Counter extends Component { +class Counter extends React.Component { render() { return (