Merge pull request #262 from mattolson/STRF-9433

Performance improvements for translations
bigcommerce · Jan 29, 2022 · bad4620 · bad4620
2 parents 7231f6a + c267e05
commit bad4620
Show file tree

Hide file tree

Showing 8 changed files with 524 additions and 200 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+## Pending
+* Revert some unused code that should have been removed as part of previous reversions [#260](https://github.com/bigcommerce/paper/pull/260)
+* Cleanup filterByKey [#261](https://github.com/bigcommerce/paper/pull/261)
+* Improve performance of Translator constructor through internal refactor of Transformer [#262](https://github.com/bigcommerce/paper/pull/262)
+
 ## 3.0.0-rc.53 (2021-12-20)
 - STRF-9553 Fallback languages in the chain [#258](https://github.com/bigcommerce/paper/pull/258)
 

diff --git a/lib/translator/filter.js b/lib/translator/filter.js
@@ -2,7 +2,10 @@
 
 /**
  * @module paper/lib/translator/filter
- */
+ *
+ * This should be considered an internal concern of Translator, and not a
+ * public interface.
+*/
 
 /*
 * Internal method to filter an object containing string keys using the given keyPrefix

diff --git a/lib/translator/index.js b/lib/translator/index.js
@@ -14,32 +14,51 @@ const Transformer = require('./transformer');
  * @private
  * @type {string}
  */
-const DEFAULT_LOCALE = 'en';
+const FALLBACK_LOCALE = 'en';
 
 /**
  * Translator constructor
+ *
  * @constructor
- * @param {string} acceptLanguage
- * @param {Object} allTranslations
+ * @param {string} acceptLanguage The Accept-Language header to be parsed to determine language to use
+ * @param {Object} allTranslations Object containing all translations coming from theme
  * @param {Object} logger
  */
 function Translator(acceptLanguage, allTranslations, logger = console) {
-    this.logger = logger;
-
-    const locales = LocaleParser.getLocales(acceptLanguage);
-    const languages = Transformer.transform(allTranslations, locales, DEFAULT_LOCALE, this.logger);
+    /**
+     * @private
+     * @type {Object}
+     */
+    this._logger = logger;
 
     /**
      * @private
-     * @type {string}
+     * @type {string[]}
      */
-    this._locale = LocaleParser.getPreferredLocale(locales, languages, DEFAULT_LOCALE);
+    this._preferredLocales = LocaleParser.getPreferredLocales(acceptLanguage, Object.keys(allTranslations), FALLBACK_LOCALE);
 
     /**
      * @private
-     * @type {Object.<string, string>}
+     * @type {Object.<string, string|Object>}
+     *
+     * Looks like this:
+     * {
+     *   locale: 'en',
+     *   locales: {
+     *     'salutations.welcome': 'en',
+     *     'salutations.hello': 'en',
+     *     'salutations.bye': 'en',
+     *     items: 'en',
+     *   }
+     *   translations: {
+     *     'salutations.welcome': 'Welcome',
+     *     'salutations.hello': 'Hello {name}',
+     *     'salutations.bye': 'Bye bye',
+     *     items: '{count, plural, one{1 Item} other{# Items}}',
+     *   }
+     * }
      */
-    this._language = languages[this._locale] || {};
+    this._language = Transformer.transform(allTranslations, this._preferredLocales, this._logger) || {};
 
     /**
      * @private
@@ -56,6 +75,7 @@ function Translator(acceptLanguage, allTranslations, logger = console) {
 
 /**
  * Translator factory method
+ *
  * @static
  * @param {string} acceptLanguage
  * @param {Object} allTranslations
@@ -68,6 +88,7 @@ Translator.create = function (acceptLanguage, allTranslations, logger = console)
 
 /**
  * Get translated string
+ *
  * @param {string} key
  * @param {Object} parameters
  * @returns {string}
@@ -85,22 +106,23 @@ Translator.prototype.translate = function (key, parameters) {
     try {
         return this._formatFunctions[key](parameters);
     } catch (err) {
-        this.logger.error(err);
-
+        this._logger.error(err);
         return '';
     }
 };
 
 /**
- * Get locale name
- * @returns {string} Translation locale
+ * Get primary locale name
+ *
+ * @returns {string} Primary locale
  */
 Translator.prototype.getLocale = function () {
-    return this._locale;
+    return this._preferredLocales[0];
 };
 
 /**
  * Get language object
+ *
  * @param {string} [keyFilter]
  * @returns {Object} Language object
  */
@@ -114,6 +136,7 @@ Translator.prototype.getLanguage = function (keyFilter) {
 
 /**
  * Get formatter
+ *
  * @private
  * @param {string} locale
  * @returns {MessageFormat} Return cached or new MessageFormat
@@ -128,6 +151,8 @@ Translator.prototype._getFormatter = function (locale) {
 
 /**
  * Compile a translation template and return a formatter function
+ *
+ * @private
  * @param {string} key
  * @return {Function}
  */
@@ -140,8 +165,7 @@ Translator.prototype._compileTemplate = function (key) {
         return formatter.compile(language.translations[key]);
     } catch (err) {
         if (err.name === 'SyntaxError') {
-            this.logger.error(`Language File Syntax Error: ${err.message} for key "${key}"`, err.expected);
-
+            this._logger.error(`Language File Syntax Error: ${err.message} for key "${key}"`, err.expected);
             return () => '';
         }
 

diff --git a/lib/translator/locale-parser.js b/lib/translator/locale-parser.js
@@ -2,50 +2,72 @@
 
 /**
  * @module paper/lib/translator/locale-parser
+ *
+ * This should be considered an internal concern of Translator, and not a
+ * public interface.
  */
 const AcceptLanguageParser = require('accept-language-parser');
 const MessageFormat = require('messageformat');
 
 /**
- * Get preferred locale
- * @param {string[]} locales
- * @param {Object} languages
- * @param {string} defaultLocale
- * @returns {string}
+ * Parse the Accept-Language header and return the list of preferred locales
+ * filtered based on the list of supported locales and making sure that MessageFormat
+ * supports it as well.
+ *
+ * @param {string} acceptLanguageHeader The Accept-Language header
+ * @param {Array} availableLocales A list of available locales from translations file
+ * @param {string} fallbackLocale The default fallback locale
+ * @returns {string[]} List of preferred+supported locales
  */
-function getPreferredLocale(locales, languages, defaultLocale) {
-    const locale = locales.find(locale => languages[locale]) || defaultLocale;
-    try {
-        new MessageFormat(locale);
-
-        return locale;
-    } catch (err) {
-        return defaultLocale;
-    }
+function getPreferredLocales(acceptLanguageHeader, availableLocales, fallbackLocale) {
+    // Parse header
+    const acceptableLocales = parseLocales(acceptLanguageHeader, fallbackLocale);
+
+    // Filter based on list of available locales
+    const preferredLocales = acceptableLocales.filter(locale => availableLocales.includes(locale));
+
+    // Filter list based on what MessageFormat actually supports
+    return preferredLocales.filter(locale => {
+        try {
+            new MessageFormat(locale);
+            return true;
+        } catch (err) {
+            return false;
+        }
+    });
 }
 
 /**
- * Parse locale header
- * @param {string} acceptLanguage
- * @returns {string[]} Locales
+ * Parse Accept-Language header and return a list of locales
+ *
+ * @param {string} acceptLanguageHeader The Accept-Language header
+ * @param {string} fallbackLocale The default fallback locale
+ * @returns {string[]} Ordered list of locale identifiers
  */
-function getLocales(acceptLanguage) {
-    const localeObjects = AcceptLanguageParser.parse(acceptLanguage);
+function parseLocales(acceptLanguageHeader, fallbackLocale) {
+    // Parse the header, adding fallback to the very end of the list (via low quality)
+    const parsed = AcceptLanguageParser.parse(`${acceptLanguageHeader},${fallbackLocale};q=0`);
 
-    const locales = localeObjects.map(localeObject => {
-        return localeObject.region ? `${localeObject.code}-${localeObject.region}` : localeObject.code;
-    });
+    // Iterate through the parsed locales, pushing into a Set to deduplicate as we go along
+    const locales = new Set();
+    for (let i = 0; i < parsed.length; i++) {
+        const locale = parsed[i];
+        if (locale.region && locale.code) {
+            locales.add(`${locale.code}-${locale.region}`);
+        }
 
-    //  Safari sends only one language code, this is to have a default fallback in case we don't have that language
-    //  As an example we may not have fr-FR so add fr to the header
-    if (locales.length === 1 && locales[0].split('-').length === 2) {
-        locales.push(locales[0].split('-')[0]);
+        // Insert regionless fallbacks into the chain. As an example, if fr-FR is in the chain,
+        // but fr is not, add it. This enables appropriate fallback logic for the translations file.
+        // If we have already seen it previously, it will not be added to the Set.
+        if (locale.code) {
+            locales.add(locale.code);
+        }
     }
 
-    return locales;
+    // Return an array based on insertion order of the Set
+    return [...locales];
  }
 
 module.exports = {
-    getPreferredLocale: getPreferredLocale,
-    getLocales: getLocales,
+    getPreferredLocales,
 };