refactor: remove json5 dependency

Use a more standard config file format and reduce the amount of external
code running in the security-sensitive context of the main process.

Signed-off-by: Kristóf Marussy <kristof@marussy.com>

5 files changed, 31 insertions, 39 deletions

diff --git a/docs/architecture.md b/docs/architecture.md
index b76b2ff..8b61288 100644
--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -69,7 +69,7 @@ Within the `MainStore`, changes to the [`SharedStore`](https://gitlab.com/say-hi
 Thus it can hold application state relevant to displaying the UI.
 
 The [`Config`](https://gitlab.com/say-hi-to-sophie/sophie/-/blob/main/packages/shared/src/stores/Config.ts) in the `SharedStore` holds the application configuration, including the list of services to be displayed.
-It is synchronized with the `config.json5` file in user data directory, which should be human-readable and -editable to facilitate debugging and other advanced use cases.
+It is synchronized with the `settings.json` file in user data directory, which should be human-readable and -editable to facilitate debugging and other advanced use cases.
 
 In the UI renderer process, the [`RendererStore`](https://gitlab.com/say-hi-to-sophie/sophie/-/blob/main/packages/renderer/src/stores/RendererStore.ts) hold the UI sate.
 It contains a read-only copy of the `SharedStore`.
diff --git a/packages/main/package.json b/packages/main/package.json
index 112c358..fceb392 100644
--- a/packages/main/package.json
+++ b/packages/main/package.json
@@ -15,7 +15,6 @@
     "electron": "^19.0.0-alpha.1",
     "fs-extra": "^10.1.0",
     "i18next": "^21.6.16",
-    "json5": "^2.2.1",
     "lodash-es": "^4.17.21",
     "loglevel": "^1.8.0",
     "loglevel-plugin-prefix": "^0.8.4",
diff --git a/packages/main/src/infrastructure/config/impl/ConfigFile.ts b/packages/main/src/infrastructure/config/impl/ConfigFile.ts
index a817717..c4e6d22 100644
--- a/packages/main/src/infrastructure/config/impl/ConfigFile.ts
+++ b/packages/main/src/infrastructure/config/impl/ConfigFile.ts
@@ -22,7 +22,6 @@ import { watch } from 'node:fs';
 import { readFile, stat, writeFile } from 'node:fs/promises';
 import path from 'node:path';
 
-import JSON5 from 'json5';
 import { throttle } from 'lodash-es';
 
 import type Config from '../../../stores/config/Config';
@@ -35,70 +34,65 @@ import type { ReadConfigResult } from '../ConfigRepository';
 const log = getLogger('ConfigFile');
 
 export default class ConfigFile implements ConfigRepository {
-  readonly #userDataDir: string;
+  private readonly configFilePath: string;
 
-  readonly #configFileName: string;
+  private writingConfig = false;
 
-  readonly #configFilePath: string;
+  private timeLastWritten: Date | undefined;
 
-  #writingConfig = false;
-
-  #timeLastWritten: Date | undefined;
-
-  constructor(userDataDir: string, configFileName = 'config.json5') {
-    this.#userDataDir = userDataDir;
-    this.#configFileName = configFileName;
-    this.#configFilePath = path.join(userDataDir, configFileName);
+  constructor(
+    private readonly userDataDir: string,
+    private readonly configFileName = 'settings.json',
+  ) {
+    this.configFilePath = path.join(userDataDir, configFileName);
   }
 
   async readConfig(): Promise<ReadConfigResult> {
     let configStr: string;
     try {
-      configStr = await readFile(this.#configFilePath, 'utf8');
+      configStr = await readFile(this.configFilePath, 'utf8');
     } catch (error) {
       if (isErrno(error, 'ENOENT')) {
-        log.debug('Config file', this.#configFilePath, 'was not found');
+        log.debug('Config file', this.configFilePath, 'was not found');
         return { found: false };
       }
       throw error;
     }
-    log.info('Read config file', this.#configFilePath);
+    log.info('Read config file', this.configFilePath);
     return {
       found: true,
-      data: JSON5.parse(configStr),
+      data: JSON.parse(configStr),
     };
   }
 
   async writeConfig(configSnapshot: Config): Promise<void> {
-    const configJson = JSON5.stringify(configSnapshot, {
-      space: 2,
-    });
-    this.#writingConfig = true;
+    const configJson = JSON.stringify(configSnapshot, undefined, 2);
+    this.writingConfig = true;
     try {
-      await writeFile(this.#configFilePath, configJson, 'utf8');
-      const { mtime } = await stat(this.#configFilePath);
-      log.trace('Config file', this.#configFilePath, 'last written at', mtime);
-      this.#timeLastWritten = mtime;
+      await writeFile(this.configFilePath, configJson, 'utf8');
+      const { mtime } = await stat(this.configFilePath);
+      log.trace('Config file', this.configFilePath, 'last written at', mtime);
+      this.timeLastWritten = mtime;
     } finally {
-      this.#writingConfig = false;
+      this.writingConfig = false;
     }
-    log.debug('Wrote config file', this.#configFilePath);
+    log.debug('Wrote config file', this.configFilePath);
   }
 
   watchConfig(callback: () => Promise<void>, throttleMs: number): Disposer {
-    log.debug('Installing watcher for', this.#userDataDir);
+    log.debug('Installing watcher for', this.userDataDir);
 
     const configChanged = throttle(async () => {
       let mtime: Date;
       try {
-        const stats = await stat(this.#configFilePath);
+        const stats = await stat(this.configFilePath);
         mtime = stats.mtime;
         log.trace('Config file last modified at', mtime);
       } catch (error) {
         if (isErrno(error, 'ENOENT')) {
           log.debug(
             'Config file',
-            this.#configFilePath,
+            this.configFilePath,
             'was deleted after being changed',
           );
           return;
@@ -106,27 +100,27 @@ export default class ConfigFile implements ConfigRepository {
         throw error;
       }
       if (
-        !this.#writingConfig &&
-        (this.#timeLastWritten === undefined || mtime > this.#timeLastWritten)
+        !this.writingConfig &&
+        (this.timeLastWritten === undefined || mtime > this.timeLastWritten)
       ) {
         log.debug(
           'Found a config file modified at',
           mtime,
           'which is newer than last written',
-          this.#timeLastWritten,
+          this.timeLastWritten,
         );
         await callback();
       }
     }, throttleMs);
 
-    const watcher = watch(this.#userDataDir, {
+    const watcher = watch(this.userDataDir, {
       persistent: false,
     });
 
     watcher.on('change', (eventType, filename) => {
       if (
         eventType === 'change' &&
-        (filename === this.#configFileName || filename === null)
+        (filename === this.configFileName || filename === null)
       ) {
         configChanged()?.catch((err) => {
           log.error('Unhandled error while listening for config changes', err);
@@ -135,7 +129,7 @@ export default class ConfigFile implements ConfigRepository {
     });
 
     return () => {
-      log.trace('Removing watcher for', this.#configFilePath);
+      log.trace('Removing watcher for', this.configFilePath);
       watcher.close();
     };
   }
diff --git a/packages/main/src/reactions/synchronizeConfig.ts b/packages/main/src/reactions/synchronizeConfig.ts
index adbe712..7e366e2 100644
--- a/packages/main/src/reactions/synchronizeConfig.ts
+++ b/packages/main/src/reactions/synchronizeConfig.ts
@@ -59,7 +59,7 @@ export default async function synchronizeConfig(
       lastConfigOnDisk = sharedStore.config;
       // We can't use `comparer.structural` from `mobx`, because
       // it handles missing values and `undefined` values differently,
-      // but JSON5 is unable to distinguish them.
+      // but JSON is unable to distinguish them.
       if (!deepEqual(result.data, lastConfigOnDisk, { strict: true })) {
         await writeConfig();
       }
diff --git a/yarn.lock b/yarn.lock
index b934e68..0c26d14 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1280,7 +1280,6 @@ __metadata:
     i18next: ^21.6.16
     jest: ^27.5.1
     jest-mock: ^27.5.1
-    json5: ^2.2.1
     lodash-es: ^4.17.21
     loglevel: ^1.8.0
     loglevel-plugin-prefix: ^0.8.4