Prevent multiple warnings for the same locale message #7091
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
Gudahtt
force-pushed
the
prevent-multiple-warnings-per-locale-key
branch
4 times, most recently
from
17198ea
to
a8fb303
Compare
whymarrh
reviewed
Sep 4, 2019
Our i18n helper issues warnings whenever a requested message is
missing. These warnings are helpful in assisting translation efforts,
but they can be distracting otherwise. They're especially problematic
for locales that are missing many messages. My browser ended up
crashing on more than one occasion due to the sheer volume of warnings.
The warning has been updated to only be issued once per missing key.
This required updating the method to pass in the current locale. The
current locale was added to the warning itself as well, which could be
helpful for cases where a message is missing in both the current locale
and the fallback ('en').
Updating the current locale and the locale messages resulted in two renders, and during the first the state was inconsistent (it would say the locale had changed to the new one, but still be using the old set of locale messages). Instead the locale is now updated with one atomic action. This was required after adding the locale to the missing locale message warning, as otherwise it would say the message was missing from the wrong locale.
Gudahtt
force-pushed
the
prevent-multiple-warnings-per-locale-key
branch
from
a8fb303
to
51874e7
Compare
|
Code changes here all make sense to me. |
whymarrh
approved these changes
Sep 6, 2019
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.
Our i18n helper issues warnings whenever a requested message is missing. These warnings are helpful in assisting translation efforts, but they can be distracting otherwise. They're especially problematic for locales that are missing many messages. My browser ended up crashing on more than one occasion due to the sheer volume of warnings.
The warning has been updated to only be issued once per missing key. This required updating the method to pass in the current locale. The current locale was added to the warning itself as well, which could be helpful for cases where a message is missing in both the current locale and the fallback ('en').
Additionally, the two actions used to change locale have been merged into one. Updating the current locale and the locale messages resulted in two renders, and during the first the state was inconsistent (it would say the locale had changed to the new one, but still be using the old set of locale messages). Instead the locale is now updated with one atomic action.
This was required after adding the locale to the missing locale message warning, as otherwise it would say the message was missing from the wrong locale.