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

react-redux's `connect` cannot be used as a decorator: type "is not assignable to type 'void'" #9951

Open
seansfkelley opened this Issue Jul 4, 2016 · 23 comments

Comments

Projects
None yet
@seansfkelley
Copy link
Contributor

seansfkelley commented Jul 4, 2016

Trying to use react-redux's connect as a class decorator can cause error messages of the form Type 'foo' is not assignable to type 'void'. The issue with these typings seems to be that TypeScript doesn't allow ClassDecorators to change the signature of the thing that they decorate, but this is the current implementation of the react-redux typings and is done purposefully, since react-redux returns a different instance with a different signature for the props.

Using connect as a function works as expected; it's only the usage as a decorator that causes type problems.

Suggestions are welcome, but any proposed solutions should be a strict improvement on what the typings can currently express, which is to say, sacrificing the current correctness of the usage-as-a-function typings is not acceptable (partly because that would be a severe regression and partly because decorators are deemed "experimental" and therefore, I assert, secondary to standard function usage).

I don't know that a complete solution to the decorator-typing problem is possible while Microsoft/TypeScript#4881 is outstanding. That said, there are incremental improvements in this case, such as by (first pass) outputting any kind of React.ComponentClass so the code at least compiles, then (second pass) outputting a component that accepts the intersection of all the props (own as well as connected), even though that would be too lenient, so the code type checks at least some of the props.

The only workaround I have right now is to not use connect as a decorator. Some may find it ugly stylistically, but it type-checks correctly, isn't any lengthier and is usable in more places than a decorator.

Instead of:

@connect(mapStateToProps, mapDispatchToProps)
class MyComponent extends React.Component<...> { ... }

use (something along the lines of):

class MyComponentImpl extends React.Component<...> { ... }
const MyComponent = connect(mapStateToProps, mapDispatchToProps)(MyComponentImpl);

Note that #8787 comes into play here and is sometimes conflated with this issue. You may also want a type hint or a cast to make sure the output component has the proper type.

Edit: Microsoft/TypeScript#12114 may help with this case: the decorator could potentially be written to create an all-optional version of the props for the generated component, which isn't totally ideal but continues to let your implementation be more assertive about prop nullity.

@ProTip

This comment has been minimized.

Copy link
Contributor

ProTip commented Feb 23, 2017

I was having similar issues the other day and dropped it for the connect func. The types looked off and using old react types. I may try to fix this and submit a PR?

@seansfkelley

This comment has been minimized.

Copy link
Contributor

seansfkelley commented Feb 23, 2017

Of course! A lot has changed since these types were originally written, though I am still skeptical that without mutating-decorator support something useful is possible. But I've worked around other inadequacies in the type system before, so give it a shot.

Perhaps mapped types could get you part of the way there, so at least you have some useful type information in the output?

@jwulf

This comment has been minimized.

Copy link

jwulf commented Mar 5, 2017

YOLO

export function myConnect(mapStateToProps, mapDispatchToProps) {
    return function (target) {
        return <any>connect(mapStateToProps, mapDispatchToProps)(target);
    }
}
@seansfkelley

This comment has been minimized.

Copy link
Contributor

seansfkelley commented Apr 16, 2017

I don't think there much point to yolo-typing this, cause the workarounds listed here and in #8787, specifically the one about using the hints to the function call, aren't that bad (especially with IDE templates for component classes) and having an any-typed component is sad. :( Any-typing connect just to use it as a decorator instead of a function call is really putting the cart before the horse.

@asterikx

This comment has been minimized.

Copy link

asterikx commented Apr 27, 2017

To use @connectdirectly (so without introducing custom decorators) I use the following workaround:

@(connect(mapStateToProps, mapDispatchToProps) as any)
class MyComponent extends React.Component<...> {...}

But I totally agree with @seansfkelley that any-typing is really not what we want to have...

@npirotte

This comment has been minimized.

Copy link
Contributor

npirotte commented May 4, 2017

Despite decorator syntax is cool, I still think using HOC like connect in a standard way is a better practice.

  1. It modifies the type of a class a goes against decorator spec.
  2. Connect is not a decorator at all, it's a HOC.
  3. It breaks the portability of the code by having a different syntax for stateless and stateful components.

So I would like to say please, don't provide a definition that allows dev to use it the wrong way :)

@raveclassic

This comment has been minimized.

Copy link

raveclassic commented Jun 28, 2017

@npirotte Agree. You're absolutely right, using HOCs as decorators violates decorators spec. It's not the base class anymore but completely different one.

I'm having the same exact issues with custom decorators and looking at the state of their support in TS (they're still behind the flag and AFAICS there're no plans to enable them by default) I'm about to drop support for them. Moreover decorators are still in w3c draft.

The best way to wrap a component to a new one is to use a higher order function for it instead of trying to decorate existing class.

@TriStarGod

This comment has been minimized.

Copy link

TriStarGod commented Jul 8, 2017

I have the same error if I import it. However, the following works for me:

const { connect } = require('react-redux');

@connect(mapStateToProps, mapDispatchToProps)
class MyComponent extends React.Component<...> { ... }
@seansfkelley

This comment has been minimized.

Copy link
Contributor

seansfkelley commented Jul 8, 2017

@TriStarGod that depends on how you've typed require. It seems likely you're ending up with connect typed as any in that case.

@offbeatful

This comment has been minimized.

Copy link

offbeatful commented Jul 23, 2017

Another solution/workaround.

Since I already have my own App State I need some very short function in the form of:

export interface PageProps {
    routing: RouterState;
}

export interface PageDispatch {
    navigate: () => void
}

@connect<PageProps, PageDispatch>(
    state => ({
        routing: state.routing
    }),
    dispatch => ({
        navigate: () => dispatch(push("/"))
    })
)
export class Page extends React.Component<PageProps & PageDispatch> {
...
}

And here is that wrapped connect method:

import { connect } from "react-redux";
import { ApplicationState } from './rootReducer';

interface MapPropsParam<TProps>{
    (state: ApplicationState, ownProps?: TProps): TProps
}

interface MapDispatchParam<TProps, TDispatchProps>{
   (dispatch: Redux.Dispatch<ApplicationState>, ownProps?: TProps): TDispatchProps;
}

export interface ConnectedComponent<TProps> {
    <TComponent extends React.ComponentType<TProps>>(component: TComponent): TComponent;
}

function connectToAppState<TProps, TDispatchProps = {}>(mapProps: MapPropsParam<TProps>, mapDispatch?: MapDispatchParam<TProps, TDispatchProps>) : ConnectedComponent<TProps> {
    return connect<TProps, TDispatchProps, TProps>(mapProps, mapDispatch) as ConnectedComponent<TProps & TDispatchProps>;    
}

export {
    connectToAppState as connect
}
@zheeeng

This comment has been minimized.

Copy link

zheeeng commented Aug 22, 2017

Thx for your work. Subscribing and waiting for progress on this issue.

@comerc

This comment has been minimized.

Copy link

comerc commented Sep 1, 2017

@offbeatful please give type declaration of other variation of mapDispatchToProps

If an object is passed, each function inside it is assumed to be a Redux action creator. An object with the same function names, but with every action creator wrapped into a dispatch call so they may be invoked directly, will be merged into the component’s props.

@pravdomil

This comment has been minimized.

Copy link
Contributor

pravdomil commented Oct 15, 2017

another workaround based on @offbeatful comment

myConnect.ts

import {
    connect as originalConnect, MapDispatchToPropsParam, MapStateToPropsParam, MergeProps, Options
} from "react-redux";
import * as React from "react";

export interface InferableComponentEnhancerWithProps<TInjectedProps, TNeedsProps> {
    <TComponent extends React.ComponentType<TInjectedProps & TNeedsProps>>(component: TComponent): TComponent;
}

interface MyConnect {
    <TStateProps = {}, TDispatchProps = {}, TOwnProps = {}>(
        mapStateToProps?: MapStateToPropsParam<TStateProps, TOwnProps>,
        mapDispatchToProps?: MapDispatchToPropsParam<TDispatchProps, TOwnProps>
    ): InferableComponentEnhancerWithProps<TStateProps & TDispatchProps, TOwnProps>;
    
    <TStateProps = {}, TDispatchProps = {}, TOwnProps = {}, TMergedProps = {}>(
        mapStateToProps?: MapStateToPropsParam<TStateProps, TOwnProps>,
        mapDispatchToProps?: MapDispatchToPropsParam<TDispatchProps, TOwnProps>,
        mergeProps?: MergeProps<TStateProps, TDispatchProps, TOwnProps, TMergedProps>,
        options?: Options<TStateProps, TOwnProps, TMergedProps>
    ): InferableComponentEnhancerWithProps<TMergedProps, TOwnProps>;
    
}

export const connect = originalConnect as MyConnect;
@offbeatful

This comment has been minimized.

Copy link

offbeatful commented Nov 26, 2017

Updated based on @pravdomil snippet and latest types (5.0.13)

import { ApplicationState } from "./rootReducer";

import * as React from "react";
import {
    connect as originalConnect, MapDispatchToPropsParam, MapStateToPropsParam, MergeProps, Options
} from "react-redux";

export type InferableComponentEnhancerWithProps<TInjectedProps, TNeedsProps> = <TComponent extends React.ComponentType<TInjectedProps & TNeedsProps>>(component: TComponent) => TComponent;

interface MyConnect {
    <TStateProps = {}, TDispatchProps = {}, TOwnProps = {}>(
        mapStateToProps?: MapStateToPropsParam<TStateProps, TOwnProps, ApplicationState>,
        mapDispatchToProps?: MapDispatchToPropsParam<TDispatchProps, TOwnProps>
    ): InferableComponentEnhancerWithProps<TStateProps & TDispatchProps, TOwnProps>;

    <TStateProps = {}, TDispatchProps = {}, TOwnProps = {}, TMergedProps = {}>(
        mapStateToProps?: MapStateToPropsParam<TStateProps, TOwnProps, ApplicationState>,
        mapDispatchToProps?: MapDispatchToPropsParam<TDispatchProps, TOwnProps>,
        mergeProps?: MergeProps<TStateProps, TDispatchProps, TOwnProps, TMergedProps>,
        options?: Options<TStateProps, TOwnProps, TMergedProps>
    ): InferableComponentEnhancerWithProps<TMergedProps, TOwnProps>;

}

export const connect = originalConnect as MyConnect;
@TomasHubelbauer

This comment has been minimized.

Copy link
Contributor

TomasHubelbauer commented Nov 26, 2017

Hey @offbeatful (cc: @pravdomil I guess?), I have recently asked a Stack Overflow question related to this issue where it was concluded this is currently not supported. As a part of that question I prepared a repository to showcase what I tried. It was concluded this is currently not supported so I was excited to see your code snippet today and went ahead to try and update my repository to use this.

Here you can see the commit with your code snippet integrated

This no longer yells with the errors I was getting before, but I have found that in my use-case where my component is connected, but also has its own props, this new signature will now make it so that even state props are required as own (TSX) props. cd my-app && yarn start in my repository will showcase what I mean.

Do you think this is something can could be addressed as well or is that not possible?

@MichaReiser MichaReiser referenced this issue Nov 30, 2017

Merged

React-i18next Generic translate #21871

3 of 3 tasks complete
@thiagoh

This comment has been minimized.

Copy link

thiagoh commented Dec 24, 2017

you can possibly be using this connect version

connect(
        mapStateToProps: MapStateToPropsParam<TStateProps, TOwnProps, State>,
        mapDispatchToProps: MapDispatchToPropsParam<TDispatchProps, TOwnProps>,
        mergeProps: null | undefined,
        options: Options<State, TStateProps, TOwnProps>
): InferableComponentEnhancerWithProps<TStateProps & TDispatchProps, TOwnProps>

if so, you just need to fix the options parameter to be Options<any, any, any> this way you won't have the TStateProps & TDispatchProps issue cuz it will be TStateProps & any

in a nutshell:

const options = { withRef:true } as Options<any, any, any>;
export const Component = connect(mapStateToProps, mapDispatchToProps, null, options)
@huhuanming

This comment has been minimized.

Copy link
Contributor

huhuanming commented Dec 28, 2017

cool like you

@JohnHandley

This comment has been minimized.

Copy link

JohnHandley commented Feb 16, 2018

@TomasHubelbauer Just tried your method, if you go back to the default redux connect method
it will work oddly enough.

Its best to make sure you use shared types between the three:

export type CounterStateProps = {
    count: number;
};

export type CounterDispatchProps = {
    onIncrement: () => void;
};

export type CounterOwnProps = {
    initialCount: number;
};

export type CounterProps = CounterStateProps & CounterDispatchProps & CounterOwnProps;

Then implement the stateful component

export class StatefulCounter extends React.Component<CounterProps, CounterStateProps> {
    timer: number;

    componentDidMount() {
        this.timer = setInterval(this.props.onIncrement, 5000);
    }

    componentWillUnmount() {
        clearInterval(this.timer);
    }

    render() {
      return (
        <StatelessCounter count={this.props.count}/>
      );
    }
}

And finally make the connector class like so using redux's build in connect NOT the custom connect code.

const mapStateToProps =
    (state: RootState, ownProps: CounterOwnProps): CounterStateProps => ({
        count: countersSelectors.getReduxCounter(state) + ownProps.initialCount,
    });

const mapDispatchToProps =
    (dispatch: Dispatch<CounterActionType>): CounterDispatchProps => bindActionCreators({
        onIncrement: CounterActions.increment,
    }, dispatch);

export const ConnectedCounter =
    connect(mapStateToProps, mapDispatchToProps)(StatefulCounter);
@TomasHubelbauer

This comment has been minimized.

Copy link
Contributor

TomasHubelbauer commented Feb 17, 2018

@JohnHandley Yeah, I know this works, in my example I showed that before I tried some of the suggestions to make it work as a decorator. I use the non-decorator variant successfully, but I'd really like to make the decorator one work, too.

Also, I think you mixed up your types, you use CounterStateProps (which is the return type of mapDispatchToProps as a component of CounterProps (which is okay, it has Redux state props, Redux dispatch props and JSX own props), but also as a type for the components state. Instead state should have its own type which is not involved in the outside component's type signature in any way.

It is possible I didn't understand your solution completely, so if you can make this work in my repository (where I use both own props in TSX and Redux state props) without getting an error saying you need to specify Redux state props in TSX, too, that'd be great!

@andrew-buckley

This comment has been minimized.

Copy link

andrew-buckley commented Apr 8, 2018

+1 on this issue

@jake-daniels

This comment has been minimized.

Copy link

jake-daniels commented May 15, 2018

I updated snippet from @offbeatful and I'm successfully using it now.

connect.ts (IAppState is the interface of redux state)

import React from 'react'
import {
    connect as originalConnect,
    MapDispatchToPropsParam,
    MapStateToPropsParam,
    MergeProps,
    Options,
} from 'react-redux'


export type InferableComponentEnhancerWithProps<IInjectedProps, INeedsProps> =
    <IComponent extends React.ComponentType<IInjectedProps & INeedsProps>>(component: IComponent) => IComponent

export interface IConnect {
    <IStateProps = {}, IDispatchProps = {}, IOwnProps = {}>(
		mapStateToProps?: MapStateToPropsParam<IStateProps, IOwnProps, IAppState>,
        mapDispatchToProps?: MapDispatchToPropsParam<IDispatchProps, IOwnProps>,
    ): InferableComponentEnhancerWithProps<IStateProps & IDispatchProps, IOwnProps>

    <IStateProps = {}, IDispatchProps = {}, IOwnProps = {}, IMergedProps = {}>(
        mapStateToProps?: MapStateToPropsParam<IStateProps, IOwnProps, IAppState>,
        mapDispatchToProps?: MapDispatchToPropsParam<IDispatchProps, IOwnProps>,
        mergeProps?: MergeProps<IStateProps, IDispatchProps, IOwnProps, IMergedProps>,
        options?: Options<IStateProps, IOwnProps, IMergedProps>,
    ): InferableComponentEnhancerWithProps<IMergedProps, IOwnProps>

}

export const connect = originalConnect as IConnect

Then my connected component looks like this:

import {connect} from 'path-to-my-connect/connect'

interface IOwnProps {
	... props exposed for the real parent component
}

interface IStateProps {
	... props from mapStateToProps
}

interface IDispatchProps {
	... props from mapDispatchToProps
}

interface IProps extends IStateProps, IDispatchProps, IOwnProps {}

@connect<IStateProps, IDispatchProps, IOwnProps>(
	(state: IAppState) => {
            return {
                foo: getFoo(state), // getFoo is a selector using 'reselect'
            }
        },
	{setFoo, increment, decrement, ... your action creators},
)
class MyComponent extends React.PureComponent<IProps> {
        // your component implementation
}

export default (MyComponent as any) as React.ComponentType<IOwnProps>

Direct casting MyComponent as React.ComponentType<IOwnProps> will fail, so I typed it as any first. I know it's a hack but works well for the parent and for the component itself too.

This is the most viable, yet still restrictive solution I have been able to come up with.

"react-redux": "^5.0.6",
"typescript": "^2.8.1",
"@types/react-redux": "^6.0.0",
@ctretyak

This comment has been minimized.

Copy link

ctretyak commented Aug 31, 2018

any news?

@marcus-sa

This comment has been minimized.

Copy link

marcus-sa commented Sep 10, 2018

@ctretyak nope, you'll eventually have to create a declaration file yourself and modify the decorator. Seems like the React Eco and TS aren't the best friends, hence why I've moved on from it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment