add TypeScript typings

[bbenezech]

Follow up to #538
Rebased against next

[aikoven]

Continuing on previous thread:

I agree that having a combination of each allowed type for each overload makes too much bloat.

I think we can omit the case when mapStateToProps: null, because if we have connect(null), the TStateProps will be inferred to {} which is not that bad, because after merging TStateProps & TOwnProps becomes the same as just TOwnProps.

But for the case when second argument is object, I think we must include another signature, because in this case TDispatchProps is constrained to object of action creators, not just any object.

There are a few other things we could improve, but I don't have enough time right now for a thorough review. Will check back in a couple days.

[bbenezech]

@aikoven

But for the case when second argument is object, I think we must include another signature, because in this case TDispatchProps is constrained to object of action creators, not just any object.

We ask mapDispatchToProps to be a MapDispatchToPropsObject & TDispatchProps

Not sure I understand.

And I tried

function connect<State, TOwnProps, TStateProps, TDispatchProps extends MapDispatchToPropsObject>(
  mapStateToProps: FuncOrSelf<MapStateToProps<State, TOwnProps, TStateProps>>|null,
  mapDispatchToProps: FuncOrSelf<TDispatchProps>,
): ComponentDecorator<TStateProps & TDispatchProps & TOwnProps, TOwnProps>;

can' t get past

[ts] Type 'DispatchProps' does not satisfy the constraint 'MapDispatchToPropsObject'.
       Index signature is missing in type 'DispatchProps'.

TS won't associate

interface DispatchProps {
  someAction: any
}

with

interface MapDispatchToPropsObject {
  [name: string]: ActionCreator<any>;
}

[bbenezech]

@aikoven I narrowed allowed Actions. I am quite satisfied with the result. It fixes your test failure (i.e. it now fails). There is a very tiny little hack for Thunks (we provide the wrong return type if TDispatchProps is not provided, covariantly compatible of course), but it is going to make everyone life's so much easier I think it is a must. I added tests around it to be sure that behavior is perfect in any cases.

Have a look and tell me what you think.

[aikoven]

Good work!

I'm not sure that react-redux should be explicitly concerned with redux-thunk. What about other middlewares that could add support for dispatching other stuff? I think the best we can do now is to allow any function as action creator. By the way, if an object is passed as 2nd argument to connect, it just uses bindActionCreators from Redux, so why don't we align it with Redux typings?

Also, since we're working on the next branch, we have to add typings for newly added connectAdvanced.

[bbenezech]

@timdorr Why is it closed? Should we stop our effort to release the typings inside react-redux repo?

[timdorr]

I think this was auto closed by my adjusting of the branches on the repo. The next branch was deleted, so that's probably why. I think we just need to change the branch and then I can open it. I'm on mobile at the moment, but will do that soon.

[bbenezech]

@timdorr Awesome thanks!

[rokoroku]

@bbenezech Why don't we use Partial type (introduced in Typescript 2.1) for annoying TOwnProps, TStateProps, TDispatchProps things? we can just pass two type arguments to connect: TState and TProps.

[aikoven]

@rokoroku What's your intended use of Partial here?

[rokoroku]

@aikoven We can simplify things. Writing types for every TOwnProps, TStateProps, TDispatchProps is really cumbersome, and Partial<T> is very useful for reducing them.

For example,

with given redux state

interface ReduxState {
  todos: TodoItem[];
};

suggested (with partial)

interface AppProps {
  todos: TodoItem[];
  addTodo: typeof TodoActions.addTodo;
};

interface AppState {
  /* something */
};

@connect<ReduxState, AppProps>(
  (state) => ({ todos: state.todos }),
  (dispatch) => ({ addTodo: bindActionCreators(TodoActions.addTodo, dispatch) })
)
export default class App extends React.Component<AppProps, AppState>{
  ...
}

// Perhaps it will be similar to this.
@connect<ReduxState, Partial<AppProps>, Partial<AppProps>, Partial<AppProps>>( ... )

current (without partial)

interface AppOwnProps {
  todos: TodoItem[];
}

interface AppStateProps {
  todos: TodoItem[];
}

interface AppDispatchProps {
  addTodo: typeof TodoActions.addTodo;
};

interface AppState {
  /* something */
};

@connect<ReduxState, AppOwnProps, AppStateProps, AppDispatchProps>(
  (state) => ({ todos: state.todos }),
  (dispatch) => ({ addTodo: bindActionCreators(TodoActions.addTodo, dispatch) })
)
export default class App extends React.Component<AppOwnProps & AppStateProps & AppDispatchProps, AppState>{
  ...
}

(Note: above code is just an example.)

[aikoven]

@rokoroku It's not really accurate.

interface AppProps {
  todos: TodoItem[];
  addTodo: typeof TodoActions.addTodo;
};

@connect<ReduxState, AppProps>(
  (state) => ({ todos: state.todos }),
  (dispatch) => ({ addTodo: bindActionCreators(TodoActions.addTodo, dispatch) })
)
export default class App extends React.Component<AppProps, AppState>{
  ...
}

In this case, props of connected component (the one returned from connect(...)(Component)) should have type {}, because both todos and addTodo are provided by connect. By using Partial we'd get {todos?: ..., addTodo?: ...} which is not correct.

Btw, TypeScript doesn't yet work well with decorators that change the type of decorated stuff, which connect does indeed.

To avoid having to set all generic arguments, you can use this:

connect(
  (state: MyState, props: MyProps) => {...},  
)

i.e. let TypeScript infer everything for you by just specifying types for mapStateToProps/mapDispatchProps arguments.

[rokoroku]

@aikoven However, we already set strict type here: React.Component<MyProps, MyState>
So the component will follow MyProps, not Partial<MyProps>.

Can't we add some abbreviated definitions while preserving existing definitions?
(And of course, I'm using the way you gave me :) )

[rokoroku]

Ohh, I figured out your intentions.

I focused on the inside of the component (where our logic will be written), and you are likely paying attention to the connect-ed component. (Am I right?)

Considering the result of connect, you're right. But is it really necessary to export the decorated type (like dispatch in StateProps by default) and export it to the outside? I think the strength of Typescript is to help developers write code with more joy and less pain, not to force them to match the exact types.

I might have been wrong, but I just wanted to share my thoughts :)

[maclockard]

Is there any reason why connectAdvanced is not included in the typings?

[theduke]

The current typings on DefinitelyTyped do not expose the connected components props on the connected component, breaking type checking when you have to pass some additional props.

Does anyone have a workaround for that?

Are the typings in this branch usable?

[bbenezech]

@theduke They are totally usable and include a lot of improvements over the DT ones.
If you add them to your app, simply nest them like this:

declare module "react-redux" {
  <typings here>
}

and remove the @types/react-redux package.

It may change a bit before release, but unless you have thousands of connected components, this should not be of concern.

@maclockard lack of time, sadly. I do not use this part of the API, I have no time to investigate atm.
Any help is welcome.

[theduke]

@bbenezech thanks for the update, glad to hear.

[aksharpatel47]

Hi @bbenezech: We are using react-redux with Typescript in one of our projects and would like to help in any way we can to get this merged. Let me know if you need any help.

[timdorr]

Closing in favor of #815