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

Fix Trans component to support react 18 types #1492

Merged
merged 2 commits into from
May 5, 2022

Conversation

pedrodurek
Copy link
Member

@pedrodurek pedrodurek commented Apr 28, 2022

Closes #1490 and #1483

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

@pedrodurek pedrodurek marked this pull request as ready for review May 5, 2022 04:18
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.951% when pulling 0db824f on pedrodurek:fix-trans-types-react-18 into 926cbac on i18next:master.

@adrai adrai merged commit 8ed568a into i18next:master May 5, 2022
@adrai
Copy link
Member

adrai commented May 5, 2022

included in v11.16.8

Comment on lines +105 to +109
declare module 'react' {
interface HTMLAttributes {
children?: ReactNode | ObjectOrNever;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this override is breaking all components that use render props when upgrading to React 18. We can patch this away, but I would strongly encourage NOT overriding generic types for all components. Can't it be limited to just the Trans component? A sledgehammer can't be the only solution when a scalpel will do?

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to provide a PR with a better solution that works.

// @marcalexiei do you know about this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@cellog extending is different from overriding. Share a minimal reproducible example so we can see how we can help you.

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.

React 18 - Trans component interpolation issue
4 participants