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 react-intl lib #709

Merged
merged 5 commits into from
Jun 11, 2024
Merged

Conversation

soygitana
Copy link
Contributor

@soygitana soygitana commented Nov 14, 2023

IMPORTANT:

  • Minimal set of changes to solve the problem.

Changes:

  1. react-intl lib has been upgrade to v6.4.2
  2. BUIE has been updated to v^20.0.0

Testing:

Make sure application and strings are not broken when language is changed.


For more information, please check:

@soygitana soygitana marked this pull request as ready for review November 14, 2023 14:11
@soygitana soygitana requested a review from a team as a code owner November 14, 2023 14:11
@soygitana
Copy link
Contributor Author

e549c20:

I have to add resolution for react-intl to package.json file ( "/react-intl//@types/react": "^17.0.2"). react-intl has set @types/react to version 16 || 17 || 18 (https://github.com/formatjs/formatjs/blob/main/package.json#L61) so yarn install the latest one, however @types/react 17 i 18 are incompatible. It caused a mismatch of types in some components.

@soygitana
Copy link
Contributor Author

4ceb40a:

I removed redundant intlShape from mock.

@soygitana
Copy link
Contributor Author

Regarding @box/frontend package:

@box/frontend is currently set to "^6.4.0," while the latest version is 9. However, similar to the Preview repository, this repo doesn't utilize language bundles. Therefore, there's no need to upgrade it in response to the react-intl update. I'll maintain @box/frontend at version 6, and you can update it later if necessary.

Copy link

@ehoogerbeets ehoogerbeets left a comment

Choose a reason for hiding this comment

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

Approved from an i18n point of view. Still needs product team's approval as well.

@soygitana soygitana changed the title chore: upgrade reac-intl lib chore: upgrade react-intl lib Nov 27, 2023
jstoffan
jstoffan previously approved these changes Jan 2, 2024
@@ -94,7 +94,7 @@
"raw-loader": "^4.0.1",
"react": "^17.0.1",
"react-dom": "^17.0.1",
"react-intl": "^3.12.0",
"react-intl": "6.4.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this exact version or can we prefix with a caret?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not 100% sure. Normally, we should use a caret. However, I discovered that react-intl version 6.4 specifies TypeScript version 4 in peerDependencies, while react-intl 6.5 requires TypeScript version 5. I am concerned that this might cause some issues, that is why I used exact version. I think they should update the react-intl package version to 7 when making this change, but they haven't done so.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it, thank you for the explanation. Should we bump to 6.4.7, in that case?

@jstoffan jstoffan changed the title chore: upgrade react-intl lib feat: upgrade react-intl lib Jan 2, 2024
@jstoffan
Copy link
Collaborator

jstoffan commented Jan 9, 2024

@soygitana, this is good to merge as-is if we don't want to mess with the patch version bump right now.

@soygitana
Copy link
Contributor Author

❗ On hold: BUIE should go first, then Preview and Annotations (confirmed by @greg-in-a-box)

@jstoffan jstoffan merged commit cc9e181 into box:master Jun 11, 2024
3 checks passed
@soygitana soygitana deleted the react-intl-upgrade branch June 11, 2024 16:54
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.

3 participants