STRF-9434: Reimplement #258

* Bring back behavior implemented in #258
* Change LocaleParser.getPreferredLocale to getPreferredLocales and have
it return an ordered list of locales based on the Accept-Language header
* Change Transformer.transform to take a list of preferred locales that
also includes the fallback rather than a single locale which loses a lot
of information that previously needed to be reconstructed. In addition,
we no longer take the fallback locale, but assume that it is already in
the preferred locale list.
* Translator.getLocale (which is used by paper-handlebars to inject into
the theme context) now returns the primary locale (first in the list of
preferred locales). This should be equivalent to before.
* LocaleParser.getLocales has now be made an internal method to the
module
* We now automatically inject the regionless language code if not
present in the Accept-Language header. For example, if fr-FR is present,
but fr is not, and this is a supported language in the theme, we add it
immediately after fr-FR. We previously were only doing this if there was
a single language in the Accept-Language header.

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)
 

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

lib/translator/index.js

@@ -14,7 +14,7 @@ const Transformer = require('./transformer');
  * @private
  * @type {string}
  */
-const DEFAULT_LOCALE = 'en';
+const FALLBACK_LOCALE = 'en';
 
 /**
  * Translator constructor
@@ -33,9 +33,9 @@ function Translator(acceptLanguage, allTranslations, logger = console) {
 
     /**
      * @private
-     * @type {string}
+     * @type {string[]}
      */
-    this._locale = LocaleParser.getPreferredLocale(acceptLanguage, Object.keys(allTranslations), DEFAULT_LOCALE);
+    this._preferredLocales = LocaleParser.getPreferredLocales(acceptLanguage, Object.keys(allTranslations), FALLBACK_LOCALE);
 
     /**
      * @private
@@ -58,7 +58,7 @@ function Translator(acceptLanguage, allTranslations, logger = console) {
      *   }
      * }
      */
-    this._language = Transformer.transform(allTranslations, this._locale, DEFAULT_LOCALE, logger) || {};
+    this._language = Transformer.transform(allTranslations, this._preferredLocales, this._logger) || {};
 
     /**
      * @private
@@ -112,12 +112,12 @@ Translator.prototype.translate = function (key, parameters) {
 };
 
 /**
- * Get locale name
+ * Get primary locale name
  *
- * @returns {string} Translation locale
+ * @returns {string} Primary locale
  */
 Translator.prototype.getLocale = function () {
-    return this._locale;
+    return this._preferredLocales[0];
 };
 
 /**

lib/translator/locale-parser.js

@@ -2,55 +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');
 
 /**
- * Parse the Accept-Language header and return the preferred locale after matching
- * up against the list of supported locales.
+ * 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} acceptLanguage The Accept-Language header
- * @param {Array} supportedLocales A list of supported locales
- * @param {string} defaultLocale The default fallback locale
- * @returns {string}
+ * @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(acceptLanguage, supportedLocales, defaultLocale) {
-    const acceptableLocales = getLocales(acceptLanguage);
-    const preferredLocale = acceptableLocales.find(locale => supportedLocales.includes(locale)) || defaultLocale;
-
-    // Make sure that MessageFormat supports it
-    try {
-        new MessageFormat(preferredLocale);
-        return preferredLocale;
-    } 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 Accept-Language header and return a list of locales
  *
- * @param {string} acceptLanguage The Accept-Language header
- * @returns {string[]} List of locale identifiers
+ * @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,
 };

lib/translator/transformer.js

@@ -2,6 +2,9 @@
 
 /**
  * @module paper/lib/translator/transformer
+ *
+ * This should be considered an internal concern of Translator, and not a
+ * public interface.
  */
 
 /**
@@ -31,7 +34,7 @@
  *   },
  * }
  *
- * The return value looks like this, assuming preferredLocale of 'fr-CA' and defaultLocale of 'en':
+ * The return value looks like this, assuming preferredLocales of ['fr-CA', 'en']:
  * {
  *   'fr-CA': {
  *     locale: 'fr-CA',
@@ -51,19 +54,18 @@
  * }
  *
  * @param {Object.<string, Object>} allTranslations
- * @param {string} preferredLocale
- * @param {string} defaultLocale
+ * @param {string[]} preferredLocales
  * @param {Object} logger
  * @returns {Object.<string, Object>} Transformed translations
  */
-function transform(allTranslations, preferredLocale, defaultLocale, logger = console) {
-    const flattened = flatten(allTranslations, preferredLocale, defaultLocale, logger);
-    return cascade(flattened, preferredLocale, defaultLocale);
+function transform(allTranslations, preferredLocales, logger = console) {
+    const flattened = flatten(allTranslations, preferredLocales, logger);
+    return cascade(flattened, preferredLocales);
 }
 
 /**
  * Flatten translation keys to a single top-level namespace, keeping only necessary
- * languages based on preferredLocale and defaultLocale.
+ * languages based on preferredLocales.
  *
  * The `allTranslations` object looks like this:
  * {
@@ -96,7 +98,7 @@ function transform(allTranslations, preferredLocale, defaultLocale, logger = con
  *   },
  * }
  *
- * The return value looks like this, assuming preferredLocale of 'fr-CA' and defaultLocale of 'en':
+ * The return value looks like this, assuming preferredLocales of ['fr-CA', 'en']:
  * {
  *   en: {
  *     'salutations.welcome': 'Welcome',
@@ -114,16 +116,14 @@ function transform(allTranslations, preferredLocale, defaultLocale, logger = con
  *   },
  * }
  * @param {Object.<string, Object>} translations
- * @param {string} preferredLocale
- * @param {string} defaultLocale
+ * @param {string[]} preferredLocales
  * @param {Object} logger
  * @returns {Object.<string, Object>} Flattened translations
  */
-function flatten(translations, preferredLocale, defaultLocale, logger = console) {
+function flatten(translations, preferredLocales, logger = console) {
     const result = {};
-    const localeList = preferredLocaleList(translations, preferredLocale, defaultLocale);
-    for (let i = 0; i < localeList.length; i++) {
-        const locale = localeList[i];
+    for (let i = 0; i < preferredLocales.length; i++) {
+        const locale = preferredLocales[i];
         try {
             result[locale] = flattenObject(translations[locale]);
         } catch (err) {
@@ -156,7 +156,7 @@ function flatten(translations, preferredLocale, defaultLocale, logger = console)
  *   },
  * }
  *
- * The return value looks like this, assuming preferredLocale of 'fr-CA' and defaultLocale of 'en':
+ * The return value looks like this, assuming preferredLocales of ['fr-CA', 'en']:
  * {
  *   'fr-CA': {
  *     locale: 'fr-CA',
@@ -176,15 +176,14 @@ function flatten(translations, preferredLocale, defaultLocale, logger = console)
  * }
  *
  * @param {Object.<string, Object>} translations Flattened translations
- * @param {string} preferredLocale
- * @param {string} defaultLocale
+ * @param {string[]} preferredLocales Ordered list of preferred locales
  * @returns {Object.<string, Object>} Cascaded translations spec
  */
-function cascade(translations, preferredLocale, defaultLocale) {
-    const result = { locale: preferredLocale, locales: {}, translations: {} };
+function cascade(translations, preferredLocales) {
+    const result = { locale: preferredLocales[0], locales: {}, translations: {} };
 
     // Process the list of locales in reverse order of preference for proper layering
-    const localeList = preferredLocaleList(translations, preferredLocale, defaultLocale).reverse();
+    const localeList = preferredLocales.slice().reverse();
 
     // Build the layered set of translations
     for (let i = 0; i < localeList.length; i++) {
@@ -245,40 +244,6 @@ function flattenObject(object, result, parentKey) {
     return result;
 }
 
-/**
- * Internal method to return the list of possible locales to use from the incoming
- * translations object based on availability and preference.
- *
- * @private
- * @param {Object.<String, Object>} translations Flattened translations
- * @param {String} preferredLocale
- * @param {String} defaultLocale
- * @returns {Array<String>} Expanded list of locales in order of preference
- */
-function preferredLocaleList(translations, preferredLocale, defaultLocale) {
-    const result = [];
-
-    // Start with preferred locale if available
-    if (typeof translations[preferredLocale] !== 'undefined') {
-        result.push(preferredLocale);
-    }
-
-    // If preferred locale includes language and region, fallback to regionless language if available
-    if (preferredLocale.includes('-')) {
-        const regionless = preferredLocale.split('-')[0];
-        if (typeof translations[regionless] !== 'undefined') {
-            result.push(regionless);
-        }
-    }
-
-    // Fallback to default locale if available
-    if (typeof translations[defaultLocale] !== 'undefined' && preferredLocale !== defaultLocale) {
-        result.push(defaultLocale);
-    }
-
-    return result;
-}
-
 module.exports = {
     cascade,
     flatten,

spec/lib/filter.js

@@ -20,7 +20,7 @@ describe('Filter', () => {
                 throw err;
             }
 
-            const translations = Transformer.transform(JSON.parse(data), 'en', 'en');
+            const translations = Transformer.transform(JSON.parse(data), ['en']);
             filtered = Filter.filterByKey(translations, 'header');
             expected = {
                 locale: 'en',

spec/lib/transformer.js

@@ -152,12 +152,12 @@ describe('Transformer', () => {
 
     describe('.flatten', done => {
         it('should return object with flattened keys', done => {
-            expect(Transformer.flatten(translations, 'en', 'en')).to.equal({ en: flattened['en']});
+            expect(Transformer.flatten(translations, ['en'])).to.equal({ en: flattened['en']});
             done();
         });
 
         it('should filter based on the languages needed to resolve translations for given preferred locale', done => {
-            expect(Transformer.flatten(translations, 'fr-CA', 'en')).to.equal({
+            expect(Transformer.flatten(translations, ['fr-CA', 'fr', 'en'])).to.equal({
                 en: flattened['en'],
                 fr: flattened['fr'],
                 'fr-CA': flattened['fr-CA'],
@@ -168,19 +168,19 @@ describe('Transformer', () => {
 
     describe('.flatten', done => {
         it('should return object with cascading translations', done => {
-            expect(Transformer.cascade(flattened, 'en', 'en')).to.equal(cascaded['en']);
+            expect(Transformer.cascade(flattened, ['en'])).to.equal(cascaded['en']);
             done();
         });
 
         it('should return object based on preferred locale', done => {
-            expect(Transformer.cascade(flattened, 'zh', 'en')).to.equal(cascaded['zh']);
+            expect(Transformer.cascade(flattened, ['zh', 'en'])).to.equal(cascaded['zh']);
             done();
         });
     });
 
     describe('.transform', done => {
         it('transform should do both flatten and cascade', done => {
-            expect(Transformer.transform(translations, 'en', 'en')).to.equal(cascaded['en']);
+            expect(Transformer.transform(translations, ['en'])).to.equal(cascaded['en']);
             done();
         });
     });