Nullish mapDispatchToProps causes Typescript Issues

[marconi1992]

What version of React, ReactDOM/React Native, Redux, and React Redux are you using?

• React: 18.2.0
• ReactDOM: 18.2.0
• Redux: 4.2.0
• React Redux: 8.0.2

What is the current behavior?

The connect HoC returns incorrect wrapped component prop types when passing null or undefined as mapDispatchToProps argument.

  class TestComponent extends React.Component<OwnProps & StateProps> {}

  const TestDispatchPropsNull = connect(mapStateToProps, null)(TestComponent)

  const verifyNull = <TestDispatchPropsNull foo="bar" />

  const TestDispatchPropsUndefined = connect(
    mapStateToProps,
    undefined
  )(TestComponent)

  const verifyNonUn = <TestDispatchPropsUndefined foo="bar" />

Codesandbox: https://codesandbox.io/s/nullish-mapdispatchtoprops-q0eolm

What is the expected behavior?

If the connect HoC accepts null or undefined then it should infer the wrapped component props correctly

Which browser and OS are affected by this issue?

No response

Did this work in previous versions of React Redux?

• Yes

[marconi1992]

I think passing a nullish value in mapDispatchToProps is not recommended

connect(mapStateToProps, null) or connect(mapDispatchToProps, null)

We should only use mapStateToProps alone connect(mapStateToProps). However, it's a common pattern I've seen in production code and there is no warning or TS definition preventing us to do that.

I created a PR adding a new type into the Connect type but let me know if we want to improve the types for preventing it rather than allowing it.

[markerikson]

Hmm. Passing a null value for mapDispatch is entirely legal :) For example, say you want to pass in either mergeProps or options and the component doesn't need to dispatch:

connect(mapState, null, null, {whateverOptionHere})

I'm actually curious about how the @types/react-redux types for v7 handle this. What does the TS behavior look like there?

[marconi1992]

Hmm. Passing a null value for mapDispatch is entirely legal :) For example, say you want to pass in either mergeProps or options and the component doesn't need to dispatch:

connect(mapState, null, null, {whateverOptionHere})

I'm actually curious about how the @types/react-redux types for v7 handle this. What does the TS behavior look like there?

We have the same result using @types/react-redux types. I think v7 is using types for v6

Codesandbox: https://codesandbox.io/s/nullish-mapdispatchtoprops-v7-34psnf?file=/src/App.tsx

The type I included in the PR is missing in v7 and v6

FWIW I found that issue upgrading from v5 to v8

[markerikson]

Not sure what you mean by "v7 is using the types for v6".

In general our public API for connect has stayed almost identical across multiple major versions, with the only differences being a couple tweaks to what options are accepted:

https://blog.isquaredsoftware.com/2018/11/react-redux-history-implementation/

But yeah, seems very possible that this combination of arguments just wasn't being handled right in the community types, and that carried over to v8 when we migrated the lib to TS.

[marconi1992]

Not sure what you mean by "v7 is using the types for v6".

Nevermind, I thought we didn't have v7 types because the folder is missing but the latest version of @types/react-redux is pointing to the index.d.ts 😅 rather than a version folder, there we have the v7 types.

[marconi1992]

@markerikson just to confirm. Is the change from my PR suitable for this? or should we proceed differently?

I included a custom eslint rule in my codebase to prevent developers to use that pattern while we stay in v5 to avoid introducing more TS issues before jumping to v8.

[phryneas]

Just to be sure: when working with v8, you have deinstalled @types/react-redux, right? That package is not required any more and would cause all kinds of conflicts.

As for the underlying problem: your example is honestly not complete enough to even find out if there is a problem in the first place. OwnProps & StateProps as well as mapStateToProps could be pretty much anything. Can you please provide a full example that shows the types are misbehaving?

[phryneas]

By the way, I'm saying this because when I just hand-write some minimal interfaces, everything works just fine:

TypeScript playground

Edit: I seem to have replicated it - do you mean this?

[marconi1992]

Just to be sure: when working with v8, you have deinstalled @types/react-redux, right? That package is not required any more and would cause all kinds of conflicts.

As for the underlying problem: your example is honestly not complete enough to even find out if there is a problem in the first place. OwnProps & StateProps as well as mapStateToProps could be pretty much anything. Can you please provide a full example that shows the types are misbehaving?

@phryneas Yeah, I removed the @types/react-redux. You can notice from the codesanbox example are not using them.

I'm not sure what you mean by "full example". I was able to reproduce the issue on the example I provided with a simple setup.

Also, it's clear in the example the types are not correct. Notice it's using the mapState and mapDispatch (as an object) type here
https://github.com/reduxjs/react-redux/blob/master/src/components/connect.tsx#L306

This is the type mess up things

   /** mapState and mapDispatch (as an object) */
  <TStateProps = {}, TDispatchProps = {}, TOwnProps = {}, State = DefaultState>(
    mapStateToProps: MapStateToPropsParam<TStateProps, TOwnProps, State>,
    mapDispatchToProps: MapDispatchToPropsParam<TDispatchProps, TOwnProps>
  ): InferableComponentEnhancerWithProps<
    TStateProps & ResolveThunks<TDispatchProps>,
    TOwnProps
  >

In this case, the mapDispatchToProps could be null or undefined, then ResolveThunks<TDispatchProps> returns never when passing a nullish value.

You can see the returned type for that wrapped component includes never instead of the right StateProps and DispatchProps

[marconi1992]

@phryneas I updated the type tests in the project to reproduce the issue.
#1929

You will see the errors when running yarnpkg type-tests. Feel free to update the test case or point out if I'm doing something weird.