Improved theme locale handling

- when activating a theme, we need to load the current locale
- this request used to be buried deep in the themeI18n init call
- now we surface it in the bridge and pass it down, which is closer to what we want to do with eventually initialising the frontend
with everything it needs up front (or not initialising it, if it isn't needed)

- in the related helpers we depend on the site.locale value instead of proxy -> themeI18n -> settingsCache drastically simplifying the code and removing deep requires
- site.locale is updated via middleware and can be relied upon

core/bridge.js

@@ -14,15 +14,16 @@ const logging = require('./shared/logging');
 const events = require('./server/lib/common/events');
 const i18n = require('./shared/i18n');
 const themeEngine = require('./frontend/services/theme-engine');
+const settingsCache = require('./server/services/settings/cache');
 
 class Bridge {
     constructor() {
         /**
          * When locale changes, we reload theme translations
-         * @deprecated: the term "lang" was deprecated in favour of "locale" publicly 4.0
+         * @deprecated: the term "lang" was deprecated in favour of "locale" publicly in 4.0
          */
-        events.on('settings.lang.edited', () => {
+        events.on('settings.lang.edited', (model) => {
-            this.getActiveTheme().initI18n();
+            this.getActiveTheme().initI18n({locale: model.get('value')});
         });
     }
 
@@ -31,6 +32,9 @@ class Bridge {
     }
 
     activateTheme(loadedTheme, checkedTheme, error) {
+        let settings = {
+            locale: settingsCache.get('lang')
+        };
         // no need to check the score, activation should be used in combination with validate.check
         // Use the two theme objects to set the current active theme
         try {
@@ -40,7 +44,7 @@ class Bridge {
                 previousGhostAPI = this.getActiveTheme().engine('ghost-api');
             }
 
-            themeEngine.setActive(loadedTheme, checkedTheme, error);
+            themeEngine.setActive(settings, loadedTheme, checkedTheme, error);
             const currentGhostAPI = this.getActiveTheme().engine('ghost-api');
 
             if (previousGhostAPI !== undefined && (previousGhostAPI !== currentGhostAPI)) {

core/frontend/helpers/date.js

@@ -3,7 +3,7 @@
 //
 // Formats a date using moment-timezone.js. Formats published_at by default but will also take a date as a parameter
 
-const {SafeString, themeI18n} = require('../services/proxy');
+const {SafeString} = require('../services/proxy');
 const moment = require('moment-timezone');
 const _ = require('lodash');
 
@@ -26,6 +26,8 @@ module.exports = function (...attrs) {
     date = date === null ? undefined : date;
 
     const timezone = options.data.site.timezone;
+    const locale = options.data.site.locale;
+
     const {
         format = 'll',
         timeago
@@ -44,7 +46,7 @@ module.exports = function (...attrs) {
     // i18n: Making dates, including month names, translatable to any language.
     // Documentation: http://momentjs.com/docs/#/i18n/
     // Locales: https://github.com/moment/moment/tree/develop/locale
-    dateMoment.locale(themeI18n.locale());
+    dateMoment.locale(locale);
 
     if (timeago) {
         date = dateMoment.tz(timezone).from(timeNow);

core/frontend/helpers/lang.js

@@ -12,8 +12,9 @@
 // Language tags in HTML and XML
 // https://www.w3.org/International/articles/language-tags/
 
-const {SafeString, themeI18n} = require('../services/proxy');
+const {SafeString} = require('../services/proxy');
 
-module.exports = function lang() {
+module.exports = function lang(options) {
-    return new SafeString(themeI18n.locale());
+    const locale = options.data.site.locale;
+    return new SafeString(locale);
 };

core/frontend/services/theme-engine/active.js

@@ -30,13 +30,16 @@ class ActiveTheme {
      * @param {object} checkedTheme - the result of gscan.format for the theme we're activating
      * @param {object} error - bootstrap validates the active theme, we would like to remember this error
      */
-    constructor(loadedTheme, checkedTheme, error) {
+    constructor(settings, loadedTheme, checkedTheme, error) {
         // Assign some data, mark it all as pseudo-private
         this._name = loadedTheme.name;
         this._path = loadedTheme.path;
         this._mounted = false;
         this._error = error;
 
+        // We get passed in a locale
+        this._locale = settings.locale || 'en';
+
         // @TODO: get gscan to return validated, useful package.json fields for us!
         this._packageInfo = loadedTheme['package.json'];
         this._partials = checkedTheme.partials;
@@ -96,8 +99,17 @@ class ActiveTheme {
         return this._engines[key];
     }
 
-    initI18n() {
+    /**
-        themeI18n.init(this._name);
+     *
+     * @param {object} options
+     * @param {string} [options.activeTheme]
+     * @param {string} [options.locale]
+     */
+    initI18n(options = {}) {
+        options.activeTheme = options.activeTheme || this._name;
+        options.locale = options.locale || this._locale;
+
+        themeI18n.init(options);
     }
 
     mount(siteApp) {
@@ -129,8 +141,8 @@ module.exports = {
      * @param {object} checkedTheme - the result of gscan.format for the theme we're activating
      * @return {ActiveTheme}
      */
-    set(loadedTheme, checkedTheme, error) {
+    set(settings, loadedTheme, checkedTheme, error) {
-        currentActiveTheme = new ActiveTheme(loadedTheme, checkedTheme, error);
+        currentActiveTheme = new ActiveTheme(settings, loadedTheme, checkedTheme, error);
         return currentActiveTheme;
     }
 };

core/frontend/services/theme-engine/i18n/i18n.js

@@ -1,7 +1,6 @@
 const errors = require('@tryghost/errors');
 const i18n = require('../../../../shared/i18n');
 const logging = require('../../../../shared/logging');
-const settingsCache = require('../../../../server/services/settings/cache');
 const config = require('../../../../shared/config');
 
 const isNil = require('lodash/isNil');
@@ -19,18 +18,18 @@ class ThemeI18n extends i18n.I18n {
      *
      * @param {String} activeTheme - name of the currently loaded theme
      */
-    init(activeTheme) {
+    init({activeTheme, locale}) {
         // This function is called during theme initialization, and when switching language or theme.
-        const currentLocale = this._loadLocale();
+        this._locale = locale || this._locale;
 
         // Reading file for current locale and active theme and keeping its content in memory
         if (activeTheme) {
             // Reading translation file for theme .hbs templates.
             // Compatibility with both old themes and i18n-capable themes.
             // Preventing missing files.
-            this._strings = this._tryGetLocale(activeTheme, currentLocale);
+            this._strings = this._tryGetLocale(activeTheme, this._locale);
 
-            if (!this._strings && currentLocale !== this.defaultLocale()) {
+            if (!this._strings && this._locale !== this.defaultLocale()) {
                 logging.warn(`Falling back to locales/${this.defaultLocale()}.json.`);
                 this._strings = this._tryGetLocale(activeTheme, this.defaultLocale());
             }
@@ -68,14 +67,6 @@ class ThemeI18n extends i18n.I18n {
             }
         }
     }
-
-    /**
-     * Load the current locale out of the settings cache
-     */
-    _loadLocale() {
-        this._locale = settingsCache.get('lang');
-        return this._locale;
-    }
 }
 
 module.exports = ThemeI18n;

test/unit/helpers/date_spec.js

@@ -4,17 +4,9 @@ const should = require('should');
 // Stuff we are testing
 const helpers = require('../../../core/frontend/helpers');
 
-const proxy = require('../../../core/frontend/services/proxy');
-const settingsCache = require('../../../core/server/services/settings/cache');
-
 const moment = require('moment-timezone');
 
 describe('{{date}} helper', function () {
-    afterEach(function () {
-        settingsCache.reset();
-        proxy.themeI18n._loadLocale();
-    });
-
     it('creates properly formatted date strings', function () {
         const testDates = [
             '2013-12-31T11:28:58.593+02:00',
@@ -74,36 +66,35 @@ describe('{{date}} helper', function () {
         const timezone = 'Europe/Dublin';
         const format = 'll';
 
+        locales.forEach(function (locale) {
+            let rendered;
+
             const context = {
                 hash: {},
                 data: {
                     site: {
-                    timezone
+                        timezone,
+                        locale
                     }
                 }
             };
 
-        locales.forEach(function (l) {
-            settingsCache.set('lang', {value: l});
-            proxy.themeI18n._loadLocale();
-            let rendered;
-
             testDates.forEach(function (d) {
                 rendered = helpers.date.call({published_at: d}, context);
 
                 should.exist(rendered);
-                String(rendered).should.equal(moment(d).tz(timezone).locale(l).format(format));
+                String(rendered).should.equal(moment(d).tz(timezone).locale(locale).format(format));
 
                 rendered = helpers.date.call({}, d, context);
 
                 should.exist(rendered);
-                String(rendered).should.equal(moment(d).tz(timezone).locale(l).format(format));
+                String(rendered).should.equal(moment(d).tz(timezone).locale(locale).format(format));
             });
 
             // No date falls back to now
             rendered = helpers.date.call({}, context);
             should.exist(rendered);
-            String(rendered).should.equal(moment().tz(timezone).locale(l).format(format));
+            String(rendered).should.equal(moment().tz(timezone).locale(locale).format(format));
         });
     });
 

test/unit/helpers/lang_spec.js

@@ -1,24 +1,28 @@
 const should = require('should');
-const settingsCache = require('../../../core/server/services/settings/cache');
 const helpers = require('../../../core/frontend/helpers');
-const proxy = require('../../../core/frontend/services/proxy');
 
 describe('{{lang}} helper', function () {
-    beforeEach(function () {
-        settingsCache.set('lang', {value: 'en'});
-        proxy.themeI18n._loadLocale();
-    });
-
-    afterEach(function () {
-        settingsCache.shutdown();
-        proxy.themeI18n._loadLocale();
-    });
-
     it('returns correct language tag', function () {
-        let expected = proxy.themeI18n.locale();
+        const locales = [
-        let rendered = helpers.lang.call();
+            'en',
+            'en-gb',
+            'de'
+        ];
+
+        locales.forEach((locale) => {
+            const context = {
+                hash: {},
+                data: {
+                    site: {
+                        locale
+                    }
+                }
+            };
+
+            let rendered = helpers.lang.call({}, context);
 
             should.exist(rendered);
-        rendered.string.should.equal(expected);
+            rendered.string.should.equal(locale);
+        });
     });
 });

test/unit/helpers/t_spec.js

@@ -12,12 +12,10 @@ describe('{{t}} helper', function () {
 
     afterEach(function () {
         configUtils.restore();
-        settingsCache.shutdown();
     });
 
     it('theme translation is DE', function () {
-        settingsCache.set('lang', {value: 'de'});
+        themeI18n.init({activeTheme: 'casper', locale: 'de'});
-        themeI18n.init('casper');
 
         let rendered = helpers.t.call({}, 'Top left Button', {
             hash: {}
@@ -27,8 +25,7 @@ describe('{{t}} helper', function () {
     });
 
     it('theme translation is EN', function () {
-        settingsCache.set('lang', {value: 'en'});
+        themeI18n.init({activeTheme: 'casper', locale: 'en'});
-        themeI18n.init('casper');
 
         let rendered = helpers.t.call({}, 'Top left Button', {
             hash: {}
@@ -38,8 +35,7 @@ describe('{{t}} helper', function () {
     });
 
     it('[fallback] no theme translation file found for FR', function () {
-        settingsCache.set('lang', {value: 'fr'});
+        themeI18n.init({activeTheme: 'casper', locale: 'fr'});
-        themeI18n.init('casper');
 
         let rendered = helpers.t.call({}, 'Top left Button', {
             hash: {}
@@ -49,8 +45,7 @@ describe('{{t}} helper', function () {
     });
 
     it('[fallback] no theme files at all, use key as translation', function () {
-        settingsCache.set('lang', {value: 'de'});
+        themeI18n.init({activeTheme: 'casper-1.4', locale: 'de'});
-        themeI18n.init('casper-1.4');
 
         let rendered = helpers.t.call({}, 'Top left Button', {
             hash: {}

test/unit/services/theme-engine/active_spec.js

@@ -15,6 +15,7 @@ describe('Themes', function () {
         describe('Mount', function () {
             let engineStub;
             let configStub;
+            let fakeSettings;
             let fakeBlogApp;
             let fakeLoadedTheme;
             let fakeCheckedTheme;
@@ -23,6 +24,10 @@ describe('Themes', function () {
                 engineStub = sinon.stub(engine, 'configure');
                 configStub = sinon.stub(config, 'set');
 
+                fakeSettings = {
+                    locale: 'en'
+                };
+
                 fakeBlogApp = {
                     cache: ['stuff'],
                     set: sinon.stub(),
@@ -45,7 +50,7 @@ describe('Themes', function () {
                 // setup partials
                 fakeCheckedTheme.partials = ['loop', 'navigation'];
 
-                const theme = activeTheme.set(fakeLoadedTheme, fakeCheckedTheme);
+                const theme = activeTheme.set(fakeSettings, fakeLoadedTheme, fakeCheckedTheme);
 
                 // Check the theme is not yet mounted
                 activeTheme.get().mounted.should.be.false();
@@ -76,7 +81,7 @@ describe('Themes', function () {
                 // setup partials
                 fakeCheckedTheme.partials = [];
 
-                const theme = activeTheme.set(fakeLoadedTheme, fakeCheckedTheme);
+                const theme = activeTheme.set(fakeSettings, fakeLoadedTheme, fakeCheckedTheme);
 
                 // Check the theme is not yet mounted
                 activeTheme.get().mounted.should.be.false();