Improved i18n to use DI for logging + config

- final preparation for moving i18n out of Ghost core
- logging is passed in via DI
- theme i18n needs a config value, but no need to pass all of config for one parameter, a better pattern is to pass the one value needed

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

@@ -1,17 +1,31 @@
 const errors = require('@tryghost/errors');
 const i18n = require('../../../../shared/i18n');
-const logging = require('../../../../shared/logging');
-const config = require('../../../../shared/config');
 
 class ThemeI18n extends i18n.I18n {
     /**
      * @param {objec} [options]
+     * @param {string} basePath - the base path for the translation directory (e.g. where themes live)
      * @param {string} [locale] - a locale string
      */
     constructor(options = {}) {
         super(options);
         // We don't care what gets passed in, themes use fulltext mode
         this._stringMode = 'fulltext';
+        this._basePath = options.basePath;
+    }
+
+    /**
+     * BasePath getter & setter used for testing
+     */
+    set basePath(basePath) {
+        this._basePath = basePath;
+    }
+
+    /**
+     * Need to call init after this
+     */
+    get basePath() {
+        return this._basePath;
     }
 
     /**
@@ -23,36 +37,36 @@ class ThemeI18n extends i18n.I18n {
      * @param {String} options.locale - name of the currently loaded locale
      *
      */
-    init({activeTheme, locale}) {
+    init({activeTheme, locale} = {}) {
         // This function is called during theme initialization, and when switching language or theme.
         this._locale = locale || this._locale;
-        this._activetheme = activeTheme || 'casper';
+        this._activetheme = activeTheme || this._activetheme;
 
         super.init();
     }
 
     _translationFileDirs() {
-        return [config.getContentPath('themes'), this._activetheme, 'locales'];
+        return [this.basePath, this._activetheme, 'locales'];
     }
 
     _handleFallbackToDefault() {
-        logging.warn(`Theme translations falling back to locales/${this.defaultLocale()}.json.`);
+        this._logging.warn(`Theme translations falling back to locales/${this.defaultLocale()}.json.`);
     }
 
     _handleMissingFileError(locale) {
         if (locale !== this.defaultLocale()) {
-            logging.warn(`Theme translations file locales/${locale}.json not found.`);
+            this._logging.warn(`Theme translations file locales/${locale}.json not found.`);
         }
     }
     _handleInvalidFileError(locale, err) {
-        logging.error(new errors.IncorrectUsageError({
+        this._logging.error(new errors.IncorrectUsageError({
             err,
             message: `Theme translations unable to parse locales/${locale}.json. Please check that it is valid JSON.`
         }));
     }
 
     _handleEmptyKeyError() {
-        logging.warn('Theme translations {{t}} helper called without a translation key.');
+        this._logging.warn('Theme translations {{t}} helper called without a translation key.');
     }
 
     _handleMissingKeyError() {

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

@@ -1,5 +1,7 @@
-const ThemeI18n = require('./i18n');
+const config = require('../../../../shared/config');
-const themeI18n = new ThemeI18n();
+const logging = require('../../../../shared/logging');
 
-module.exports = themeI18n;
+const ThemeI18n = require('./i18n');
+
+module.exports = new ThemeI18n({logging, basePath: config.getContentPath('themes')});
 module.exports.ThemeI18n = ThemeI18n;

core/shared/i18n/i18n.js

@@ -1,3 +1,4 @@
+const errors = require('@tryghost/errors');
 const fs = require('fs-extra');
 const path = require('path');
 const MessageFormat = require('intl-messageformat');
@@ -8,18 +9,19 @@ const isEqual = require('lodash/isEqual');
 const isNil = require('lodash/isNil');
 const merge = require('lodash/merge');
 const get = require('lodash/get');
-const errors = require('@tryghost/errors');
-const logging = require('../logging');
 
 class I18n {
     /**
      * @param {objec} [options]
      * @param {string} [locale] - a locale string
      * @param {{dot|fulltext}} [stringMode] - which mode our translation keys use
+     * @param {{object}} [logging] - logging method
      */
     constructor(options = {}) {
         this._locale = options.locale || this.defaultLocale();
         this._stringMode = options.stringMode || 'dot';
+        this._logging = options.logging || console;
+
         this._strings = null;
     }
 
@@ -257,29 +259,29 @@ class I18n {
     }
 
     _handleFormatError(err) {
-        logging.error(err.message);
+        this._logging.error(err.message);
     }
 
     _handleFallbackToDefault() {
-        logging.warn(`i18n is falling back to ${this.defaultLocale()}.json.`);
+        this._logging.warn(`i18n is falling back to ${this.defaultLocale()}.json.`);
     }
 
     _handleMissingFileError(locale) {
-        logging.warn(`i18n was unable to find ${locale}.json.`);
+        this._logging.warn(`i18n was unable to find ${locale}.json.`);
     }
     _handleInvalidFileError(locale, err) {
-        logging.error(new errors.IncorrectUsageError({
+        this._logging.error(new errors.IncorrectUsageError({
             err,
             message: `i18n was unable to parse ${locale}.json. Please check that it is valid JSON.`
         }));
     }
 
     _handleEmptyKeyError() {
-        logging.warn('i18n.t() was called without a key');
+        this._logging.warn('i18n.t() was called without a key');
     }
 
     _handleMissingKeyError(key) {
-        logging.error(new errors.IncorrectUsageError({
+        this._logging.error(new errors.IncorrectUsageError({
             message: `i18n.t() was called with a key that could not be found: ${key}`
         }));
     }

core/shared/i18n/index.js

@@ -1,4 +1,6 @@
+const logging = require('../logging');
+
 const I18n = require('./i18n');
 
-module.exports = new I18n();
+module.exports = new I18n({logging});
 module.exports.I18n = I18n;

test/unit/helpers/t_spec.js

@@ -2,15 +2,16 @@ const should = require('should');
 const path = require('path');
 const helpers = require('../../../core/frontend/helpers');
 const themeI18n = require('../../../core/frontend/services/theme-engine/i18n');
-const configUtils = require('../../utils/configUtils');
 
 describe('{{t}} helper', function () {
-    beforeEach(function () {
+    let ogBasePath = themeI18n.basePath;
-        configUtils.set('paths:contentPath', path.join(__dirname, '../../utils/fixtures/'));
+
+    before(function () {
+        themeI18n.basePath = path.join(__dirname, '../../utils/fixtures/themes/');
     });
 
-    afterEach(function () {
+    after(function () {
-        configUtils.restore();
+        themeI18n.basePath = ogBasePath;
     });
 
     it('theme translation is DE', function () {