From e4fcd55566ec86eb162018c1291920ba7da63089 Mon Sep 17 00:00:00 2001 From: Fabien Schurter Date: Sat, 4 May 2024 15:16:38 +0200 Subject: [PATCH] refactor: Do some cleaning and normalizing --- src/adapter/icu.js | 9 +++------ src/domain/data.js | 10 ++++------ src/domain/i18n.js | 5 +++-- src/main.js | 3 +-- src/utils/fs.js | 15 +++++++------- src/utils/json.js | 6 +++--- src/utils/object.js | 40 ++++++++++++++++++-------------------- tests/adapter/icu.test.js | 9 ++++++++- tests/domain/data.test.js | 8 +++++--- tests/domain/i18n.test.js | 11 +++-------- tests/utils/fs.test.js | 8 ++++++-- tests/utils/object.test.js | 31 ++++++++++++++++++----------- 12 files changed, 82 insertions(+), 73 deletions(-) diff --git a/src/adapter/icu.js b/src/adapter/icu.js index 6f44f0c..8a6b781 100644 --- a/src/adapter/icu.js +++ b/src/adapter/icu.js @@ -1,12 +1,9 @@ -import assert from 'node:assert' - export const translateString = ( (IntlMessageFormat, dictionary, defaultLocale = 'en') => ( (msgID, params = {}, locale = defaultLocale) => { - assert( - msgID in dictionary, - `The messsage with ID \`${msgID}\` does not exist in the dictionary.`, - ) + if (!(msgID in dictionary)) { + throw new RangeError(`The messsage with ID \`${msgID}\` does not exist in the dictionary.`) + } return new IntlMessageFormat(dictionary[msgID], locale).format(params) } diff --git a/src/domain/data.js b/src/domain/data.js index caada15..5b2456d 100644 --- a/src/domain/data.js +++ b/src/domain/data.js @@ -1,14 +1,12 @@ const DATA_FILE_PATH = 'data.json' const ENV_VAR_PLACEHOLDER_PATTERN = /^%(.+)%$/ -const captureEnvVarName = (value) => { - const match = value.match?.(ENV_VAR_PLACEHOLDER_PATTERN) - - return match ? match[1] : null -} +const captureEnvVarName = (value) => value.match?.(ENV_VAR_PLACEHOLDER_PATTERN)?.[1] ?? null export const parseProjectData = (parseJSONFile, withSrcDir) => ( - withSrcDir((prefixWithSrcDir) => parseJSONFile(prefixWithSrcDir(DATA_FILE_PATH))) + withSrcDir( + (prefixWithSrcDir) => parseJSONFile(prefixWithSrcDir(DATA_FILE_PATH)) + ) ) export const mergeDataWithEnvVars = ( diff --git a/src/domain/i18n.js b/src/domain/i18n.js index 55cc2e3..3f4ebed 100644 --- a/src/domain/i18n.js +++ b/src/domain/i18n.js @@ -1,4 +1,3 @@ -import assert from 'node:assert' import { join } from 'node:path' const LANG_PATTERN = /^[a-z]{2}$/ @@ -8,7 +7,9 @@ const TRANSLATION_FILE_EXT = '.json' export const parseProjectTranslations = ( (parseJSONFile, withSrcDir, dotFlattenObject) => ( (lang) => { - assert.match(lang, LANG_PATTERN, `\`${lang}\` is not a valid language code.`) + if (!LANG_PATTERN.test(lang)) { + throw new Error(`\`${lang}\` is not a valid language code.`) + } return withSrcDir((prefixWithSrcDir) => { const translationFilePath = join( diff --git a/src/main.js b/src/main.js index e0e9cea..66e68ce 100644 --- a/src/main.js +++ b/src/main.js @@ -30,14 +30,13 @@ export default async function main( lang = null, envVars = {}, ) { - const _parseJSONFile = parseJSONFile(ifPathExists, readFile) - await ifPathExists(buildDirPath, rmDir) const [withSrcDir, withBuildDir] = await Promise.all([ withDir(srcDirPath), withScratchDir(buildDirPath), ]) + const _parseJSONFile = parseJSONFile(ifPathExists, readFile) return ( Promise.all([ diff --git a/src/utils/fs.js b/src/utils/fs.js index 14dfbf3..b2e2230 100644 --- a/src/utils/fs.js +++ b/src/utils/fs.js @@ -1,15 +1,14 @@ -import assert from 'node:assert' import * as fs from 'node:fs/promises' import { join } from 'node:path' +const prefixWithDir = (dirPath) => (relativePath = '') => join(dirPath, relativePath) + export const withDir = ( (dirPath) => ( async (cb) => { await fs.access(dirPath) - const prefixWithDir = (relativePath = '') => join(dirPath, relativePath) - - return cb(prefixWithDir) + return cb(prefixWithDir(dirPath)) } ) ) @@ -29,9 +28,7 @@ export const withScratchDir = ( await fs.access(dirPath, fs.constants.W_OK) - const prefixWithDir = (relativePath = '') => join(dirPath, relativePath) - - return cb(prefixWithDir) + return cb(prefixWithDir(dirPath)) } ) ) @@ -59,7 +56,9 @@ export const copyDir = (srcPath, destPath) => ( ) export const rmDir = (path) => { - assert.notStrictEqual(path, '/', 'Removing the filesystem’s root is not allowed.') + if (path === '/') { + throw new SystemError('Removing the filesystem’s root is not allowed.') + } return fs.rm(path, { recursive: true }) } diff --git a/src/utils/json.js b/src/utils/json.js index e4cc93b..fe569f8 100644 --- a/src/utils/json.js +++ b/src/utils/json.js @@ -1,9 +1,9 @@ export const parseJSONFile = ( (ifPathExists, readFile) => ( - (filePath) => ( + (jsonFilePath) => ( ifPathExists( - filePath, - () => readFile(filePath).then(JSON.parse), + jsonFilePath, + (filePath) => readFile(filePath).then(JSON.parse), {}, ) ) diff --git a/src/utils/object.js b/src/utils/object.js index 8aae1e6..64b080d 100644 --- a/src/utils/object.js +++ b/src/utils/object.js @@ -1,21 +1,14 @@ -import assert from 'node:assert' +const PROP_PATH_PATTERN = /^(?[a-z_]+)(?(?:\.[a-z_]+)*)$/i -const valueIsObject = (value) => ( - typeof value !== 'undefined' && - value.constructor.name === 'Object' -) +const valueIsObject = (value) => value.constructor?.name === 'Object' -const valueIsArray = (value) => ( - typeof value !== 'undefined' && - value.constructor.name === 'Array' -) +const valueIsArray = (value) => value.constructor?.name === 'Array' const valueIsComposite = (value) => valueIsObject(value) || valueIsArray(value) -const testPropPathValidity = (propPath) => ( - /^(?[a-z_]+)(?(?:\.[a-z_]+)*)$/i - .exec(propPath) -) +const objectIsEmpty = (obj) => Object.keys(obj).length > 0 + +const matchPropPath = (propPath) => propPath.match?.(PROP_PATH_PATTERN) ?? null export const deepCloneObject = (obj) => JSON.parse(JSON.stringify(obj)) @@ -35,37 +28,42 @@ export const transformObjectValues = (obj, cb) => { return obj } -export const cleanUpObjectList = (objectList) => objectList.filter((obj) => Object.keys(obj).length) +export const cleanUpObjectList = (objectList) => objectList.filter(objectIsEmpty) export const mergeObjectList = (objectList) => Object.assign({}, ...objectList) export const accessObjectProp = (obj, propPath) => { - const match = testPropPathValidity(propPath) + const match = matchPropPath(propPath) - assert.notStrictEqual(match, null, `The property path \`${propPath}\` is invalid.` ) + if (match === null) { + throw new Error(`The property path \`${propPath}\` is invalid.` ) + } const currentKey = match.groups.currentKey - if (typeof obj[currentKey] === 'undefined') { + if (!(currentKey in obj)) { throw new RangeError(`The key \`${currentKey}\` does not exist in the current object tree.`) } const rest = match.groups.rest const hasRest = Boolean(rest.length) + const currentValue = obj[currentKey] - if (hasRest && !valueIsObject(obj[currentKey])) { + if (hasRest && !valueIsObject(currentValue)) { throw new TypeError(`The value at key \`${currentKey}\` is not an object and can’t be traversed.`) } return ( hasRest - ? accessObjectProp(obj[currentKey], rest.substring(1)) - : obj[currentKey] + ? accessObjectProp(currentValue, rest.substring(1)) + : currentValue ) } export const dotFlattenObject = (obj) => { - assert(valueIsComposite(obj), 'Only composite values can be flattened.') + if (!valueIsComposite(obj)) { + throw new TypeError('Only composite values can be flattened.') + } const output = {} diff --git a/tests/adapter/icu.test.js b/tests/adapter/icu.test.js index 602ff8e..ecc26d9 100644 --- a/tests/adapter/icu.test.js +++ b/tests/adapter/icu.test.js @@ -11,7 +11,7 @@ describe('#src/adapter/icu', () => { occupation: 'I’m a {job}, the best in {town}.', number_of_kids: 'I have {kidNum, plural, =0 {no kids} =1 {a single child} other {# kids}}.', bastille_day: 'France’s Bastille Day was on {bastilleDay, date, ::yyyyMMMMdd}.', - account_balance: 'I have {balance, number} moneyz on my bank account.' + account_balance: 'I have {balance, number} moneyz on my bank account.', } const trans = translateString(IntlMessageFormat, dictionary) @@ -67,6 +67,13 @@ describe('#src/adapter/icu', () => { 'I have 12 345,67 moneyz on my bank account.', ) }) + + it('throws if the message ID does not exist in the dictionary', () => { + assert.throws( + () => trans('askdjww00--'), + RangeError, + ) + }) }) }) }) diff --git a/tests/domain/data.test.js b/tests/domain/data.test.js index e932883..1c2075a 100644 --- a/tests/domain/data.test.js +++ b/tests/domain/data.test.js @@ -10,7 +10,7 @@ import { parseProjectData, mergeDataWithEnvVars } from '#src/domain/data' describe('#src/domain/data', () => { describe('parseProjectData()', () => { - it('parses data from a `data.json` file at project root', async () => { + it('parses data from a `data.json` file at the project root', async () => { await withTempDir(async (prefixWithTempDir) => { const srcDirPath = prefixWithTempDir('src') const dataFilePath = join(srcDirPath, 'data.json') @@ -46,7 +46,7 @@ describe('#src/domain/data', () => { 42, '%SECRET_PHONE_NUMBER%', false, - 'LANG' + 'LANG', ], }, cruft: { @@ -55,6 +55,7 @@ describe('#src/domain/data', () => { lang: '%LANG%', phone: 'SECRET_PHONE_NUMBER', }, + nil: '%NOPE%', }, } @@ -74,7 +75,7 @@ describe('#src/domain/data', () => { 42, '+33700000000', false, - 'LANG' + 'LANG', ], }, cruft: { @@ -83,6 +84,7 @@ describe('#src/domain/data', () => { lang: 'fr', phone: 'SECRET_PHONE_NUMBER', }, + nil: null, }, }, ) diff --git a/tests/domain/i18n.test.js b/tests/domain/i18n.test.js index 66d9d0c..98fd23b 100644 --- a/tests/domain/i18n.test.js +++ b/tests/domain/i18n.test.js @@ -47,12 +47,10 @@ describe('#src/domain/i18n', () => { } `) - const withSrcDir = withDir(srcDirPath) - const _parseJSONFile = parseJSONFile(ifPathExists, readFile) const _parseProjectTranslations = ( parseProjectTranslations( - _parseJSONFile, - withSrcDir, + parseJSONFile(ifPathExists, readFile), + withDir(srcDirPath), dotFlattenObject, ) ) @@ -83,10 +81,7 @@ describe('#src/domain/i18n', () => { }) it('throws if an invalid `lang` parameter is passed', () => { - assert.throws( - () => parseProjectTranslations(noop, noop, noop)('fAiL'), - assert.AssertionError, - ) + assert.throws(() => parseProjectTranslations(noop, noop, noop)('fAiL')) }) }) }) diff --git a/tests/utils/fs.test.js b/tests/utils/fs.test.js index 8b8682f..52109ee 100644 --- a/tests/utils/fs.test.js +++ b/tests/utils/fs.test.js @@ -78,8 +78,8 @@ describe('#src/utils/fs', () => { const nonExistentPath = generateNonExistentPath() let marker = 0 - await ifPathExists('/', () => marker = 1) - await ifPathExists(nonExistentPath, () => marker = 2) + await ifPathExists('/', (_) => marker = 1) + await ifPathExists(nonExistentPath, (_) => marker = 2) assert.strictEqual(marker, 1) }) @@ -185,5 +185,9 @@ describe('#src/utils/fs', () => { await assert.rejects(fs.access(dirPath)) }) }) + + it('prevents the deletion of the filesystem’s root', { + skip: 'This is too scary to test 😱 (disk wipe hazard).', + }) }) }) diff --git a/tests/utils/object.test.js b/tests/utils/object.test.js index 8fcfc3c..9365d99 100644 --- a/tests/utils/object.test.js +++ b/tests/utils/object.test.js @@ -60,10 +60,10 @@ describe('#src/utils/object', () => { describe('transformObjectValues()', () => { it('recursively passes each of an object’s nested values trough a callback', () => { const obj = { - foo: 'bar', + foo: 42, stuff: [ 'foo', - 'bar', + Symbol.for('clutter'), { some_prop: 'a', other_prop: 'b', @@ -90,10 +90,10 @@ describe('#src/utils/object', () => { assert.deepStrictEqual( transformObjectValues(obj, transformer), { - foo: 'BAR', + foo: 42, stuff: [ 'FOO', - 'BAR', + Symbol.for('clutter'), { some_prop: 'A', other_prop: 'B', @@ -112,6 +112,17 @@ describe('#src/utils/object', () => { }, ) }) + + it('throws if not passed a composite value', () => { + assert.throws( + () => dotFlattenObject(3.14), + TypeError, + ) + assert.throws( + () => dotFlattenObject(Symbol.for('foobar')), + TypeError, + ) + }) }) describe('cleanUpObjectList()', () => { @@ -127,6 +138,7 @@ describe('#src/utils/object', () => { { stuff: 'clutter', }, + {}, ] assert.deepStrictEqual( @@ -170,7 +182,7 @@ describe('#src/utils/object', () => { identity: { name: 'John', surname: 'Doe', - } + }, } ) }) @@ -192,10 +204,7 @@ describe('#src/utils/object', () => { }) it('throws if the property path is invalid', () => { - assert.throws( - () => accessObjectProp({}, '379&239--'), - assert.AssertionError, - ) + assert.throws(() => accessObjectProp({}, '379&239--')) }) it('throws if one of the nested properties does not exist or is not an object', () => { @@ -248,11 +257,11 @@ describe('#src/utils/object', () => { it('throws if not passed a composite value', () => { assert.throws( () => dotFlattenObject(42), - assert.AssertionError, + TypeError, ) assert.throws( () => dotFlattenObject('Hello!'), - assert.AssertionError, + TypeError, ) }) })