BREAKING: Removing i18next-icu #799
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vasharma05
changed the title
|
Size Change: -353 kB (-11%) 👏 Total Size: 2.83 MB
ℹ️ View Unchanged
|
|
Hey Vineet, could you clear up the PR description and the title some more? |
ibacher
changed the title
ibacher
changed the title
|
@vasharma05 I've re-written the description. Please feel free to change it, though. |
ibacher
reviewed
Oct 27, 2023
| <Trans i18nKey="emptyStateText" values={{ displayText: displayText }}> | ||
| There are no {displayText} to display | ||
| </Trans> | ||
| {t("emptyStateText", "There are no {{displayText}} to display", { |
There was a problem hiding this comment.
As a general rule, I'd probably actually favour the <Trans /> component where its used like this.
There was a problem hiding this comment.
Actually, the {{displayText}} was getting changed to {{ displayText }} by prettier, hence I made the change
|
Sorry @ibacher, I think I overrided your description with mine, you submitted before me and I didn't see that as I was also writing at the same time and my submission became the final submission (GH description race condition 🤣 ) @denniskigen please feel free to ask any doubts. |
|
I am creating PRs in another repos as well, we'll merge them altogether. |
This was referenced Oct 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
For changes to apps
If applicable
Summary
This PR remove the
i18next-icuto be used in the i18next initialization because it is supported till i18next'sv19. Hence the pluralization translations weren't working in the application.Basically
i18next-icuwas helping us to use{in the variables instead of{{. The{{interpolation is what i18next's default configuration.(@jayasanka-sack I finally got the answer to why we were using
{instead of{{)Detailed Explaination
i18next-icuuses the Intl Message Format. If we look into the i18next-icu's description here, it says:For pluralization, i18next uses the Intl.PluralRules, which is a browser API. As said above, the i18next-icu overrides the i18next plural API with the Intl message formatter's rules and you can have a look at the Intl message formatter's plural support here, which is very different than the
Intl.PluralRules.So what was happening is that the translations for keys
key_oneorkey_otherwasn't working fort('key', {count: 2}), because the it wasn't finding the exact key, and as mentioned above, the ICU message formatter is different than the Intl.PluralRules, hence the no proper working for i18next pluralization.Screenshots
None
Related Issue
None
Other