Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stricter typescript type for Trans components prop #1516

Merged
merged 4 commits into from Jun 10, 2022

Conversation

danreeves
Copy link
Contributor

Since the values in the components prop are being passed to cloneElement only the ReactElement type is really supported. All other types accepted by ReactNode will cause a runtime error. You can test this by checking out the first two commits in this branch.

  • d8904f7: Types pass but as the unit test shows react throws
  • 563966b: Corrected types: tslint now fails
  • 516fbdc & 9509d54: I removed the example typescript error and tests for invalid behaviour

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included

Checklist (for documentation change)

  • only relevant documentation part is changed (make a diff before you submit the PR)
  • motivation/reason is provided

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.951% when pulling 9509d54 on danreeves:fix-trans-components-types into b2dcdfa on i18next:master.

@danreeves danreeves changed the title More correct typescript type for Trans components prop Stricter typescript type for Trans components prop Jun 7, 2022
Copy link
Member

@pedrodurek pedrodurek left a comment

Choose a reason for hiding this comment

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

Looks great to me, thanks @danreeves

@adrai adrai merged commit 5c80355 into i18next:master Jun 10, 2022
@adrai
Copy link
Member

adrai commented Jun 10, 2022

thank you, released in v11.17.1

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

Successfully merging this pull request may close these issues.

None yet

4 participants