From 65b176eaefc9a53c8e9cc615d290cb927947a6bb Mon Sep 17 00:00:00 2001 From: Thomas Charlat Date: Mon, 7 May 2018 21:34:04 +0200 Subject: [PATCH] [react-redux]: decoration target props should not extend injected props, contravariance issue (#25228) * feat(react-redux): add strict null check * feat(react-redux): remove improper inference tests Infered decorated typings would rely on the implicit and false assumption that decorated component props extend injected props and own props #24922 #24913 * fix(react-redux): injected props and decorated component props intersection * InjectedProps should not have to extend DecoratedProps * DecoratedProps should not have to extend InjectedProps * DecoratedProps should extend Intersection * Remaining Props should be required on the decoration output #24913 #24922 * feat(react-redux): add new commiters * feat(react-redux): 2.9 keyof compatibility depends on Extract (2.8) --- types/mirrorx/index.d.ts | 2 +- types/next-redux-wrapper/index.d.ts | 2 +- types/react-intl-redux/index.d.ts | 2 +- types/react-redux-toastr/index.d.ts | 2 +- types/react-redux/index.d.ts | 29 ++++++- types/react-redux/react-redux-tests.tsx | 101 ++++++++++++++++------- types/react-redux/tsconfig.json | 2 +- types/react-router-redux/index.d.ts | 2 +- types/redux-auth-wrapper/index.d.ts | 2 +- types/redux-devtools/index.d.ts | 2 +- types/redux-form/v6/index.d.ts | 2 +- types/redux-form/v6/redux-form-tests.tsx | 2 +- types/redux-little-router/index.d.ts | 2 +- 13 files changed, 109 insertions(+), 43 deletions(-) diff --git a/types/mirrorx/index.d.ts b/types/mirrorx/index.d.ts index 3e7090eab2..07b26d494c 100644 --- a/types/mirrorx/index.d.ts +++ b/types/mirrorx/index.d.ts @@ -2,7 +2,7 @@ // Project: https://github.com/mirrorjs/mirror // Definitions by: Aaronphy // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 import * as H from 'history'; diff --git a/types/next-redux-wrapper/index.d.ts b/types/next-redux-wrapper/index.d.ts index 16dc88b1a4..cdbcb7b9e5 100644 --- a/types/next-redux-wrapper/index.d.ts +++ b/types/next-redux-wrapper/index.d.ts @@ -2,7 +2,7 @@ // Project: https://github.com/kirill-konshin/next-redux-wrapper // Definitions by: Steve // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 /// /*~ Note that ES6 modules cannot directly export callable functions. diff --git a/types/react-intl-redux/index.d.ts b/types/react-intl-redux/index.d.ts index 300bd0542d..daa5dd9926 100644 --- a/types/react-intl-redux/index.d.ts +++ b/types/react-intl-redux/index.d.ts @@ -2,7 +2,7 @@ // Project: https://github.com/ratson/react-intl-redux // Definitions by: Karol Janyst // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 import { Action } from "redux" import { Provider as ReduxProvider } from "react-redux" diff --git a/types/react-redux-toastr/index.d.ts b/types/react-redux-toastr/index.d.ts index bf50211775..7c9f30b056 100644 --- a/types/react-redux-toastr/index.d.ts +++ b/types/react-redux-toastr/index.d.ts @@ -4,7 +4,7 @@ // Artyom Stukans // Mika Kuitunen // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 import { Component } from 'react'; import { Action, ActionCreator, Reducer } from 'redux'; diff --git a/types/react-redux/index.d.ts b/types/react-redux/index.d.ts index 59d8e1078b..0519a43bb5 100644 --- a/types/react-redux/index.d.ts +++ b/types/react-redux/index.d.ts @@ -8,8 +8,11 @@ // Nicholas Boll // Dibyo Majumdar // Prashant Deva +// Thomas Charlat +// Valentin Descamps +// Johann Rakotoharisoa // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 // Known Issue: // There is a known issue in TypeScript, which doesn't allow decorators to change the signature of the classes @@ -37,21 +40,39 @@ type ActionCreator = Redux.ActionCreator; // Diff / Omit taken from https://github.com/Microsoft/TypeScript/issues/12215#issuecomment-311923766 type Omit = Pick; + export interface DispatchProp { - dispatch?: Dispatch; + dispatch: Dispatch; } interface AdvancedComponentDecorator { (component: Component): ComponentClass; } +/** + * a property P will be present if : + * - it is present in both DecorationTargetProps and InjectedProps + * - DecorationTargetProps[P] extends InjectedProps[P] + * ie: decorated component can accept more types than decorator is injecting + * + * For decoration, inject props or ownProps are all optionnaly + * required by the decorated (right hand side) component. + * But any property required by the decorated component must extend the injected property + */ +type Shared< + InjectedProps, + DecorationTargetProps extends Shared + > = { + [P in Extract]?: DecorationTargetProps[P] extends InjectedProps[P] ? InjectedProps[P] : never; + }; + // 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 { -

( +

>( component: Component

- ): ComponentClass & TNeedsProps> & {WrappedComponent: Component

} + ): ComponentClass> & TNeedsProps> & {WrappedComponent: Component

} } // Injects props and removes them from the prop requirements. diff --git a/types/react-redux/react-redux-tests.tsx b/types/react-redux/react-redux-tests.tsx index 367afedad6..ebc174dd1c 100644 --- a/types/react-redux/react-redux-tests.tsx +++ b/types/react-redux/react-redux-tests.tsx @@ -349,8 +349,7 @@ connect { - render(): JSX.Element { - // ... + render() { return null; } } @@ -544,14 +543,12 @@ interface TestState { class TestComponent extends Component { } const WrappedTestComponent = connect()(TestComponent); -// return value of the connect()(TestComponent) is of the type TestComponent -let ATestComponent: React.ComponentClass = null; -ATestComponent = TestComponent; -ATestComponent = WrappedTestComponent; - -let anElement: ReactElement; +// return value of the connect()(TestComponent) is assignable to a ComponentClass +// ie: DispatchProp has been removed through decoration +const ADecoratedTestComponent: React.ComponentClass = WrappedTestComponent; ; -; + +const ATestComponent: React.ComponentClass = TestComponent; // $ExpectError class NonComponent {} // this doesn't compile @@ -748,7 +745,8 @@ namespace Issue15463 { interface ISpinnerProps{ showGlobalSpinner: boolean; } - class SpinnerClass extends React.Component { + + class SpinnerClass extends React.Component { render() { return (

); } @@ -815,7 +813,7 @@ namespace TestControlledComponentWithoutDispatchProp { } namespace TestDispatchToPropsAsObject { - const onClick: ActionCreator<{}> = null; + const onClick: ActionCreator<{}> = () => ({}); const mapStateToProps = (state: any) => { return { title: state.app.title as string, @@ -900,24 +898,71 @@ namespace TestCreateProvider { ReactDOM.render(, document.body); } -namespace TestTypeInference { - interface State { a: number }; +namespace TestWithoutTOwnPropsDecoratedInference { - const OnlyState = connect( - (state: {a: number}, props: {b: number}) => ({a: state.a, c: state.a + props.b}) - )(props => {props.a} + {props.b} = {props.c}) - interface State { a: number }; - ReactDOM.render(, document.body); + interface ForwardedProps { + forwarded: string; + } - const OnlyDispatch = connect( - undefined, - (dispatch, props: {b: number}) => ({action: () => dispatch({type: 'action', b: props.b})}) - )(props => {props.b}) - ReactDOM.render(, document.body); + interface OwnProps { + own: string; + } - const StateAndDispatch = connect( - (state: {a: number}, props: {b: number}) => ({a: state.a, c: state.a + props.b}), - (dispatch, props: {b: number}) => ({action: () => dispatch({type: 'action', b: props.b})}) - )(props => {props.a} + {props.b} = {props.c}) - ReactDOM.render(, document.body); + interface StateProps { + state: string; + } + + class WithoutOwnPropsComponentClass extends React.Component> { + render() { + return
; + } + } + + const WithoutOwnPropsComponentStateless: React.StatelessComponent> = () => (
); + + function mapStateToProps4(state: any, ownProps: OwnProps): StateProps { + return { state: 'string' }; + } + + // these decorations should compile, it is perfectly acceptable to receive props and ignore them + const ConnectedWithOwnPropsClass = connect(mapStateToProps4)(WithoutOwnPropsComponentClass); + const ConnectedWithOwnPropsStateless = connect(mapStateToProps4)(WithoutOwnPropsComponentStateless); + const ConnectedWithTypeHintClass = connect(mapStateToProps4)(WithoutOwnPropsComponentClass); + const ConnectedWithTypeHintStateless = connect(mapStateToProps4)(WithoutOwnPropsComponentStateless); + + // This should compile + React.createElement(ConnectedWithOwnPropsClass, { own: 'string', forwarded: 'string' }); + React.createElement(ConnectedWithOwnPropsClass, { own: 'string', forwarded: 'string' }); + + // This should not compile, it is missing ForwardedProps + React.createElement(ConnectedWithOwnPropsClass, { own: 'string' }); // $ExpectError + React.createElement(ConnectedWithOwnPropsStateless, { own: 'string' }); // $ExpectError + + // This should compile + React.createElement(ConnectedWithOwnPropsClass, { own: 'string', forwarded: 'string' }); + React.createElement(ConnectedWithOwnPropsStateless, { own: 'string', forwarded: 'string' }); + + // This should not compile, it is missing ForwardedProps + React.createElement(ConnectedWithTypeHintClass, { own: 'string' }); // $ExpectError + React.createElement(ConnectedWithTypeHintStateless, { own: 'string' }); // $ExpectError + + interface AllProps { + own: string + state: string + } + + class AllPropsComponent extends React.Component> { + render() { + return
; + } + } + + type PickedOwnProps = Pick + type PickedStateProps = Pick + + const mapStateToPropsForPicked: MapStateToProps = (state: any): PickedStateProps => { + return { state: "string" } + } + const ConnectedWithPickedOwnProps = connect(mapStateToPropsForPicked)(AllPropsComponent); + } diff --git a/types/react-redux/tsconfig.json b/types/react-redux/tsconfig.json index 981f3c9b0f..5fa4508cd8 100644 --- a/types/react-redux/tsconfig.json +++ b/types/react-redux/tsconfig.json @@ -11,7 +11,7 @@ ], "noImplicitAny": true, "noImplicitThis": true, - "strictNullChecks": false, + "strictNullChecks": true, "strictFunctionTypes": true, "baseUrl": "../", "jsx": "react", diff --git a/types/react-router-redux/index.d.ts b/types/react-router-redux/index.d.ts index 7c8d78ccc9..d85db1969e 100644 --- a/types/react-router-redux/index.d.ts +++ b/types/react-router-redux/index.d.ts @@ -4,7 +4,7 @@ // Shoya Tanaka // Mykolas // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 import { Store, diff --git a/types/redux-auth-wrapper/index.d.ts b/types/redux-auth-wrapper/index.d.ts index fc2543385a..728be4a54b 100644 --- a/types/redux-auth-wrapper/index.d.ts +++ b/types/redux-auth-wrapper/index.d.ts @@ -2,7 +2,7 @@ // Project: https://github.com/mjrussell/redux-auth-wrapper // Definitions by: Karol Janyst // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 import { ComponentClass, StatelessComponent, ComponentType, ReactType } from "react"; diff --git a/types/redux-devtools/index.d.ts b/types/redux-devtools/index.d.ts index 6bc8c3a4f2..234fb9a4bb 100644 --- a/types/redux-devtools/index.d.ts +++ b/types/redux-devtools/index.d.ts @@ -2,7 +2,7 @@ // Project: https://github.com/gaearon/redux-devtools // Definitions by: Petryshyn Sergii // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 import * as React from 'react'; import { GenericStoreEnhancer } from 'redux'; diff --git a/types/redux-form/v6/index.d.ts b/types/redux-form/v6/index.d.ts index ce34a772ed..a8209a7643 100644 --- a/types/redux-form/v6/index.d.ts +++ b/types/redux-form/v6/index.d.ts @@ -2,7 +2,7 @@ // Project: https://github.com/erikras/redux-form // Definitions by: Carson Full , Daniel Lytkin , Karol Janyst , Luka Zakrajsek // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 import { ComponentClass, diff --git a/types/redux-form/v6/redux-form-tests.tsx b/types/redux-form/v6/redux-form-tests.tsx index 6865051e0d..3ab303ebde 100644 --- a/types/redux-form/v6/redux-form-tests.tsx +++ b/types/redux-form/v6/redux-form-tests.tsx @@ -127,7 +127,7 @@ const ConnectedDecoratedInitializeFromStateFormFunction = connect( // React ComponentClass instead of StatelessComponent -class InitializeFromStateFormClass extends React.Component> { +class InitializeFromStateFormClass extends React.Component { render() { return InitializeFromStateFormFunction(this.props); } diff --git a/types/redux-little-router/index.d.ts b/types/redux-little-router/index.d.ts index 10aa3536f1..5ea39ce138 100644 --- a/types/redux-little-router/index.d.ts +++ b/types/redux-little-router/index.d.ts @@ -2,7 +2,7 @@ // Project: https://github.com/FormidableLabs/redux-little-router // Definitions by: priecint // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.6 +// TypeScript Version: 2.8 import * as React from "react"; import { Reducer, Middleware, StoreEnhancer } from "redux";