Skip to content

fix: narrow withConnectedCount own-props cast#308

Open
narutamaaurum wants to merge 1 commit into
piotrwitek:masterfrom
narutamaaurum:fix-204-connected-count-cast
Open

fix: narrow withConnectedCount own-props cast#308
narutamaaurum wants to merge 1 commit into
piotrwitek:masterfrom
narutamaaurum:fix-204-connected-count-cast

Conversation

@narutamaaurum

@narutamaaurum narutamaaurum commented May 23, 2026

Copy link
Copy Markdown

Summary

  • cast the stripped restProps to Diff<BaseProps, InjectedProps> before passing it through the JSX spread
  • keep the injected count / onIncrement props supplied separately at the call site
  • regenerate the README output so the published snippet matches the playground source

Validation

  • npm run ci-check
  • cd playground && CI=true npm run tsc
  • cd playground && npm run lint

Closes #204

- cast the stripped rest props to Diff<BaseProps, InjectedProps>
- keep the injected count/increment props separate at the JSX boundary
- regenerate README output

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +38 to +42
const ownProps = restProps as unknown as Diff<BaseProps, InjectedProps>;

return (
<BaseComponent
{...(restProps as BaseProps)}
{...(ownProps as BaseProps)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Comment thread README.md
Comment on lines +933 to +937
const ownProps = restProps as unknown as Diff<BaseProps, InjectedProps>;

return (
<BaseComponent
{...(restProps as BaseProps)}
{...(ownProps as BaseProps)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

src/hoc/connected-with-count.tsx type cast is incorrect

1 participant