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

feat: upgrade to react v18 #1789

Merged
merged 1 commit into from
Aug 14, 2024
Merged

feat: upgrade to react v18 #1789

merged 1 commit into from
Aug 14, 2024

Conversation

ripecosta
Copy link
Contributor

@ripecosta ripecosta commented Aug 14, 2024

Description:

Upgrades to react v18.

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • commit message and code follows the Developer's Certification of Origin

@coveralls
Copy link

coveralls commented Aug 14, 2024

Coverage Status

coverage: 97.073% (-0.02%) from 97.094%
when pulling f4238b0 on ripecosta:rc/react
into 8276a35 on i18next:master.

@adrai
Copy link
Member

adrai commented Aug 14, 2024

Theoretically, if we keep the peerDependency of react as is, there is no need for a breaking release, since the v18 react is only needed because of the tests.
I know there are a lot of users still using react 16/17...
I would prefer not to drop this older react compatibility for now.
We can say older versions are not supported, but technically still compatible.
What do you think?

@ripecosta
Copy link
Contributor Author

ripecosta commented Aug 14, 2024

@adrai If in the future we use a feature that isn't compatible with v16/17 we could that break compatibility without realising. However since the project is pretty stable (and so is react) I guess that's not very likely to happen. I'm happy to not bump the react version if that's your preference.

Any thoughts on the test with the comment? Leave as is/simplify? I'll make the peerdep change now to unblock this PR from being merged but can simplify the tests in a separate PR if you think that's fine.

@ripecosta ripecosta changed the title feat!: upgrade to react v18 feat: upgrade to react v18 Aug 14, 2024
@adrai
Copy link
Member

adrai commented Aug 14, 2024

ok, 👍
Regarding the test: it is important in that test case that no render is triggered.

@ripecosta
Copy link
Contributor Author

ripecosta commented Aug 14, 2024

I see. In that case I'll leave the test as is.

I have reverted the change to the react peer deps so the PR should be ready to merge.

@adrai
Copy link
Member

adrai commented Aug 14, 2024

thank you 👍

@adrai adrai merged commit 5b492e8 into i18next:master Aug 14, 2024
8 of 9 checks passed
@ripecosta ripecosta deleted the rc/react branch August 14, 2024 12:43
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.

3 participants