From ad9b18e00f3a6f7d810905be7739b6daf65a0889 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 6 May 2021 10:38:56 +0100 Subject: [PATCH] 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 --- .../services/theme-engine/i18n/i18n.js | 32 +++++++++++++------ .../services/theme-engine/i18n/index.js | 8 +++-- core/shared/i18n/i18n.js | 18 ++++++----- core/shared/i18n/index.js | 4 ++- test/unit/helpers/t_spec.js | 11 ++++--- 5 files changed, 47 insertions(+), 26 deletions(-) diff --git a/core/frontend/services/theme-engine/i18n/i18n.js b/core/frontend/services/theme-engine/i18n/i18n.js index 4fcc43601a..9b19402ba3 100644 --- a/core/frontend/services/theme-engine/i18n/i18n.js +++ b/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() { diff --git a/core/frontend/services/theme-engine/i18n/index.js b/core/frontend/services/theme-engine/i18n/index.js index a5e1924f65..751487b762 100644 --- a/core/frontend/services/theme-engine/i18n/index.js +++ b/core/frontend/services/theme-engine/i18n/index.js @@ -1,5 +1,7 @@ -const ThemeI18n = require('./i18n'); -const themeI18n = new ThemeI18n(); +const config = require('../../../../shared/config'); +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; diff --git a/core/shared/i18n/i18n.js b/core/shared/i18n/i18n.js index 74db96cc65..73339d2324 100644 --- a/core/shared/i18n/i18n.js +++ b/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}` })); } diff --git a/core/shared/i18n/index.js b/core/shared/i18n/index.js index 81e31e4537..677805555c 100644 --- a/core/shared/i18n/index.js +++ b/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; diff --git a/test/unit/helpers/t_spec.js b/test/unit/helpers/t_spec.js index e377ff249f..79a7f576f1 100644 --- a/test/unit/helpers/t_spec.js +++ b/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 () { - configUtils.set('paths:contentPath', path.join(__dirname, '../../utils/fixtures/')); + let ogBasePath = themeI18n.basePath; + + before(function () { + themeI18n.basePath = path.join(__dirname, '../../utils/fixtures/themes/'); }); - afterEach(function () { - configUtils.restore(); + after(function () { + themeI18n.basePath = ogBasePath; }); it('theme translation is DE', function () {