aikoven / typescript-fsa

Type-safe action creator utilities
MIT License
607 stars 28 forks source link

Typing issues with react-redux connect and class components #55

Closed supermihi closed 6 years ago

supermihi commented 6 years ago

Not sure if this is related to #51 or #32, but I have issues with typescript-fsa in connection to react-redux' connect and class components.

In the following example, the class component version does not compile, but the functional component version does:

import * as React from 'react';
import actionCreatorFactory, { ActionCreator } from 'typescript-fsa';
import { connect } from 'react-redux';

interface Props {
  doSomething: (thing: string) => void;
}
class MyComponent extends React.Component<Props> {
  render() {
    return <button onClick={() => this.props.doSomething('test')}/>;
  }
}
const MyFunctionalComponent = (props: Props) => <button onClick={() => props.doSomething('test')}/>;

const createActionCreator = actionCreatorFactory();
const doSomething = createActionCreator<string>('DO_SOMETHING');

const mapDispatchToProps = {
  doSomething,
};
connect(null, mapDispatchToProps)(MyComponent); // error: Argument not assignable
connect(null, mapDispatchToProps)(MyFunctionalComponent); // works

Both work if I declare Props as

interface Props {
  doSomething: ActionCreator<string>;
}

But I obviously do not want to couple the component to redux-specific stuff. Using typescript 2.8 and the most up-to-date version of all packages. Any ideas?

aikoven commented 6 years ago

Could you please post a full error message?

supermihi commented 6 years ago
[tsl] ERROR in ...\test.tsx(21,35)
      TS2345: Argument of type 'typeof MyComponent' is not assignable to parameter of type 'ComponentType<{ doSomething: ActionCreator<string>; }>'.
  Type 'typeof MyComponent' is not assignable to type 'StatelessComponent<{ doSomething: ActionCreator<string>; }>'.
    Type 'typeof MyComponent' provides no match for the signature '(props: { doSomething: ActionCreator<string>; } & { children?: ReactNode; }, context?: any): ReactElement<any>'.
aikoven commented 6 years ago

My guess is that this is because doSomething doesn’t really return void and it would work if you change return type to any or {}. Of course it is far from ideal. As far as I know in a regular situation () => Something should be assignable to () => void. But react-redux typings are mostly dark magic so I wouldn't be surprised if somehow it makes it behave this way.

suutari-ai commented 6 years ago

I think mapDispatchToProps should be a function that takes in dispatch argument and returns an object of type Partial<Props>. If you define it as object like that, it won't work. What if you change the mapDispatchToProps to something like:

const mapDispatchToProps = (dispatch) => {
  doSomething: (x: string) => dispatch(doSomething(x)),
};
supermihi commented 6 years ago

@aikoven changing to any does not resolve the error

@suutari-ai the react-redux API seems to support both versions of specifying mapDispatchToProps. The typescript issue arises with both.

bschlenk commented 6 years ago

I am also getting an error in this case, but with a different error message:

Type '((id: string) => any) | undefined' is not assignable to type 'ActionCreator<string> | undefined'.
                Type '(id: string) => any' is not assignable to type 'ActionCreator<string> | undefined'.
                  Type '(id: string) => any' is not assignable to type 'ActionCreator<string>'.
                    Property 'type' is missing in type '(id: string) => any'.
aikoven commented 6 years ago

@supermihi I'm sorry for not answering for so long.

I took a closer look and it still seems that this has to do with react-redux typings. They do a lot of work to give you better type inference, but sometimes it confuses the compiler.

I managed to fix the error you get by specifying type arguments explicitly:

connect<{}, Props>(null, mapDispatchToProps)(MyComponent); // no error

@bschlenk Could you please check if the above trick fixes your case?

supermihi commented 6 years ago

@aikoven Thanks for providing the workaround!

lucas1677 commented 6 years ago

@supermihi it works fine, the only thing you should do, is just define the type of component props clearly. you can refer to this:

import * as React from 'react';
import {connect} from 'react-redux';

import {TodoAppState, TodoItemState} from '@src/types/todoApp';

const TodoItem = ({id, isComplete, name}) => (
  <li>
    <input type="checkbox" defaultChecked={isComplete}/>
    {name}
  </li>
);

type PropsType = {
  todos: TodoItemState[];
};

class TodoList extends React.Component<PropsType> {
  render() {
    return (
      <div className="Todo-list">
        <ul>
          {this.props.todos.map(todo => <TodoItem key={todo.id} {...todo}/>)}
        </ul>
      </div>
    );
  }
}

export default connect(
  (state: TodoAppState) => ({
    todos: state.todos,
  })
)(TodoList);