fix: narrow withConnectedCount own-props cast#308
Conversation
- cast the stripped rest props to Diff<BaseProps, InjectedProps> - keep the injected count/increment props separate at the JSX boundary - regenerate README output
There was a problem hiding this comment.
Code Review
This pull request modifies the withConnectedCount Higher-Order Component and its documentation to use double type assertions and manual casting when spreading props. The review feedback correctly identifies that using as unknown as Diff<...> followed by a cast to BaseProps is logically flawed and bypasses TypeScript's type safety, as the ownProps variable specifically excludes the properties required by BaseProps. The reviewer suggests refactoring the HOC's generic type definitions to allow for natural type inference, which would eliminate the need for these unsafe assertions and better align with the project's stated goals.
| const ownProps = restProps as unknown as Diff<BaseProps, InjectedProps>; | ||
|
|
||
| return ( | ||
| <BaseComponent | ||
| {...(restProps as BaseProps)} | ||
| {...(ownProps as BaseProps)} |
There was a problem hiding this comment.
The double type assertion as unknown as Diff<...> followed by as BaseProps is redundant and bypasses TypeScript's safety checks. Additionally, casting ownProps to BaseProps is logically incorrect because ownProps (the result of the Diff utility) specifically excludes the InjectedProps that BaseProps requires. This implementation also contradicts the project's stated goal in the README (line 34) of achieving type safety without "hacking with any type" or using excessive type assertions. A better approach would be to correctly type the Hoc class props to include Diff<BaseProps, InjectedProps>, which would allow restProps to be correctly inferred without assertions.
| const ownProps = restProps as unknown as Diff<BaseProps, InjectedProps>; | ||
|
|
||
| return ( | ||
| <BaseComponent | ||
| {...(restProps as BaseProps)} | ||
| {...(ownProps as BaseProps)} |
There was a problem hiding this comment.
This example uses a double type assertion (as unknown as Diff<...>) and an incorrect cast to BaseProps. Since ownProps is defined as the difference between BaseProps and InjectedProps, it does not satisfy the BaseProps interface on its own. In a guide focused on best practices for TypeScript, it is recommended to avoid as unknown and incorrect assertions. Consider updating the Hoc class definition to properly include the own props in its generic type, which would eliminate the need for these assertions and align with the goals mentioned in the introduction of this guide.
Summary
restPropstoDiff<BaseProps, InjectedProps>before passing it through the JSX spreadcount/onIncrementprops supplied separately at the call siteValidation
npm run ci-checkcd playground && CI=true npm run tsccd playground && npm run lintCloses #204