Prevent multiple warnings for the same locale message (#7091)

* Prevent multiple warnings for the same locale message

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').

* Change locale and localeMessages as single action

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.

test/unit/ui/app/actions.spec.js

@@ -1155,9 +1155,8 @@ describe('Actions', () => {
 
       const expectedActions = [
         { type: 'SHOW_LOADING_INDICATION', value: undefined },
+        { type: 'SET_CURRENT_LOCALE', value: { locale: 'en', messages: enLocale }},
         { type: 'HIDE_LOADING_INDICATION' },
-        { type: 'SET_CURRENT_LOCALE', value: 'en' },
-        { type: 'SET_LOCALE_MESSAGES', value: enLocale },
       ]
 
       return store.dispatch(actions.updateCurrentLocale('en'))

test/unit/ui/app/reducers/metamask.spec.js

@@ -456,7 +456,7 @@ describe('MetaMask Reducers', () => {
   it('sets current locale', () => {
     const state = reduceMetamask({}, {
       type: actions.SET_CURRENT_LOCALE,
-      value: 'ge',
+      value: { locale: 'ge' },
     })
 
     assert.equal(state.currentLocale, 'ge')

ui/app/ducks/locale/locale.js

@@ -7,9 +7,9 @@ function reduceMetamask (state, action) {
   const localeMessagesState = extend({}, state.localeMessages)
 
   switch (action.type) {
-    case actions.SET_LOCALE_MESSAGES:
+    case actions.SET_CURRENT_LOCALE:
       return extend(localeMessagesState, {
-        current: action.value,
+        current: action.value.messages,
       })
     default:
       return localeMessagesState

ui/app/ducks/metamask/metamask.js

@@ -377,7 +377,7 @@ function reduceMetamask (state, action) {
 
     case actions.SET_CURRENT_LOCALE:
       return extend(metamaskState, {
-        currentLocale: action.value,
+        currentLocale: action.value.locale,
       })
 
     case actions.SET_PENDING_TOKENS:

ui/app/helpers/higher-order-components/i18n-provider.js

@@ -7,12 +7,12 @@ const t = require('../utils/i18n-helper').getMessage
 
 class I18nProvider extends Component {
   tOrDefault = (key, defaultValue, ...args) => {
-    const { localeMessages: { current, en } = {} } = this.props
+    const { localeMessages: { current, en } = {}, currentLocale } = this.props
-    return t(current, key, ...args) || t(en, key, ...args) || defaultValue
+    return t(currentLocale, current, key, ...args) || t(currentLocale, en, key, ...args) || defaultValue
   }
 
   getChildContext () {
-    const { localeMessages } = this.props
+    const { localeMessages, currentLocale } = this.props
     const { current, en } = localeMessages
     return {
       /**
@@ -26,7 +26,7 @@ class I18nProvider extends Component {
           return key
         }
 
-        return t(current, key, ...args) || t(en, key, ...args) || `[${key}]`
+        return t(currentLocale, current, key, ...args) || t(currentLocale, en, key, ...args) || `[${key}]`
       },
       tOrDefault: this.tOrDefault,
       tOrKey: (key, ...args) => {
@@ -42,6 +42,7 @@ class I18nProvider extends Component {
 
 I18nProvider.propTypes = {
   localeMessages: PropTypes.object,
+  currentLocale: PropTypes.string,
   children: PropTypes.object,
 }
 
@@ -52,8 +53,9 @@ I18nProvider.childContextTypes = {
 }
 
 const mapStateToProps = state => {
-  const { localeMessages } = state
+  const { localeMessages, metamask: { currentLocale } } = state
   return {
+    currentLocale,
     localeMessages,
   }
 }

ui/app/helpers/utils/i18n-helper.js

@@ -1,22 +1,30 @@
 // cross-browser connection to extension i18n API
 const log = require('loglevel')
 
+const warned = {}
 /**
  * Returns a localized message for the given key
- * @param {object} locale The locale
+ * @param {string} localeCode The code for the current locale
+ * @param {object} localeMessages The map of messages for the current locale
  * @param {string} key The message key
  * @param {string[]} substitutions A list of message substitution replacements
  * @return {null|string} The localized message
  */
-const getMessage = (locale, key, substitutions) => {
+const getMessage = (localeCode, localeMessages, key, substitutions) => {
-  if (!locale) {
+  if (!localeMessages) {
     return null
   }
-  if (!locale[key]) {
+  if (!localeMessages[key]) {
-    log.warn(`Translator - Unable to find value for key "${key}"`)
+    if (!warned[localeCode] || !warned[localeCode][key]) {
+      if (!warned[localeCode]) {
+        warned[localeCode] = {}
+      }
+      warned[localeCode][key] = true
+      log.warn(`Translator - Unable to find value of key "${key}" for locale "${localeCode}"`)
+    }
     return null
   }
-  const entry = locale[key]
+  const entry = localeMessages[key]
   let phrase = entry.message
   // perform substitutions
   if (substitutions && substitutions.length) {
@@ -28,12 +36,12 @@ const getMessage = (locale, key, substitutions) => {
   return phrase
 }
 
-async function fetchLocale (localeName) {
+async function fetchLocale (localeCode) {
   try {
-    const response = await fetch(`./_locales/${localeName}/messages.json`)
+    const response = await fetch(`./_locales/${localeCode}/messages.json`)
     return await response.json()
   } catch (error) {
-    log.error(`failed to fetch ${localeName} locale because of ${error}`)
+    log.error(`failed to fetch ${localeCode} locale because of ${error}`)
     return {}
   }
 }

ui/app/store/actions.js

@@ -308,10 +308,8 @@ var actions = {
 
   // locale
   SET_CURRENT_LOCALE: 'SET_CURRENT_LOCALE',
-  SET_LOCALE_MESSAGES: 'SET_LOCALE_MESSAGES',
   setCurrentLocale,
   updateCurrentLocale,
-  setLocaleMessages,
   //
   // Feature Flags
   setFeatureFlag,
@@ -2604,25 +2602,20 @@ function updateCurrentLocale (key) {
             return dispatch(actions.displayWarning(err.message))
           }
           switchDirection(textDirection)
+          dispatch(actions.setCurrentLocale(key, localeMessages))
           dispatch(actions.hideLoadingIndication())
-          dispatch(actions.setCurrentLocale(key))
-          dispatch(actions.setLocaleMessages(localeMessages))
         })
       })
   }
 }
 
-function setCurrentLocale (key) {
+function setCurrentLocale (locale, messages) {
   return {
     type: actions.SET_CURRENT_LOCALE,
-    value: key,
+    value: {
-  }
+      locale,
-}
+      messages,
-
+    },
-function setLocaleMessages (localeMessages) {
-  return {
-    type: actions.SET_LOCALE_MESSAGES,
-    value: localeMessages,
   }
 }