Strictly typed component decorator support

[JProgrammer]

I have updated the types with an additional generic type so that standard connect calls are not aware of the void call.

I have also updated the tests to be more strict as the exisiting test did not attempt to render the component under test.

Addresses #9951

[dt-bot]

react-redux/react-redux.d.ts

to authors (@tkqubo @seansfkelley). Could you review this PR?
👍 or 👎?

Checklist

• pass the Travis CI test?

[seansfkelley]

@sheetalkamat hey, I was just about to review this. Why did you merge without approval?

[seansfkelley]

This change is not correct -- dispatch is parameterized by the state it's relevant to, not by any action.

[sheetalkamat]

@JProgrammer please create a new PR addressing the feedback.

[seansfkelley]

@sheetalkamat Can we just revert this and start again since it was so recent? Also, why did you merge it without approval?

Edit: without approval and open for not even 24 hours, more accurately. I didn't even realize a PR was opened!

[sheetalkamat]

@mhegazy asked me to review the prs and merge them. This one looked ok to me hence merged.

[seansfkelley]

@mhegazy I applaud the desire to merge these faster, but we should still try to get community reviews from typings owners (within reason)!

@sheetalkamat thank you for reverting, I appreciate it!

[seansfkelley]

@JProgrammer sorry about the roller-coaster, want to try this again?

[fraserharris]

@JProgrammer yes, please try again! Waiting on the edge of my chair to be able to use connect decorators in React.

[zakdances]

I like where this is going! Looking forward to the end of Unable to resolve signature of class decorator when called as an expression. errors.

[kherock]

@seansfkelley I've been spending the past few days trying to tackle this issue, but I'm running into a couple of major roadblocks (correct me if I'm wrong on something):

From what I've gathered, @connect doesn't really function as a true class decorator (by TypeScript's standards) and isn't usable in its current form. The wrapped result of the connect call doesn't actually extend the targeted class, it uses its own type definition. TypeScript views decorated classes under the assumption that the resulting class will pass a typeof check on the target. This just simply isn't true with what react-redux is doing.

The Connect class returned is an entirely new class that wraps the targeted class, with its own unique set of props. This is the main block here is because the props that the Connect class says are allowed are different than those that the class it's wrapping are. This is what causes the Types of property 'props' are incompatible. message when using it as a decorator. TypeScript can't verify that the Connect class is indeed a typeof the wrapped class because this offending property.

To work around this, we can make the type declarations for props less strict. I am currently aware of two. The first is to say

interface StateProps {
  myState: MyState
}
interface DispatchProps { ... }
interface OwnProps { ... }

@connect<StateProps, DispatchProps, OwnProps>(..., ...)
export class MyComponent extends Component<any, ComponentState> { ... }

saying that any props are assignable on the component. This has the disadvantage that TS can't vaildate anything pertaining to props. The other (and I think better) way is to make every property in StateProps and DispatchProps optional:

interface StateProps {
  myState?: MyState
}
interface DispatchProps { ... }
interface OwnProps { ... }

@connect<StateProps, DispatchProps, OwnProps>(..., ...)
export class MyComponent extends Component<OwnProps & StateProps & DispatchProps, ComponentState> { ... }

This way everything checks out, but TS can still complain if props in OwnProps are omitted in JSX.

[seansfkelley]

Have you taken a look at #9951? It outlines some of the issues as you have here (such as microsoft/TypeScript#4881).

As for partial types, that is an improvement, though it requires you to compromise on those types available to the class implementation. microsoft/TypeScript#12114 ought to help, if this is the path the typings take.

[kherock]

Thanks for pointing me to those, glad I'm not going crazy. Is there any point in messing with this further until we have mapped types then?

[seansfkelley]

I took a hard look at this in 1.8 and 2.0 hasn't introduced anything that would help (I think), so probably not.