feat: replace json5 with comment-json to preserve comments

Author: Swiftwork

packages/cli/commands/mcp.ts

@@ -1,7 +1,7 @@
-import { confirm, select } from "@inquirer/prompts";
+import { select } from "@inquirer/prompts";
 import chalk from "chalk";
 import { Command } from "commander";
-import JSON5 from "json5";
+import { parse as parseJSON, stringify as stringifyJSON } from "comment-json";
 import { mkdir, readFile, writeFile } from "node:fs/promises";
 import { dirname, join, relative } from "node:path";
 import ora, { type Ora } from "ora";
@@ -117,50 +117,31 @@ export const mcpAction = async (options: {
     `Reading configuration file: ${chalk.cyan(displayConfigPath)}...`,
   ).start();
   let editorConfig: any = {};
-  try {
-    if (await fileExists(configPath)) {
-      const content = await readFile(configPath, "utf-8");
-      // Attempt to parse JSON, handle potential comments if needed
-      try {
-        editorConfig = JSON5.parse(content);
-      } catch (parseError) {
-        spinner.fail(
-          `Failed to parse configuration file ${chalk.cyan(displayConfigPath)}.`,
-        );
-        throw parseError; // Re-throw original error if cleaning fails
-      }
-      spinner.succeed("Configuration file read.");
-    } else {
-      spinner.info("Configuration file not found, will create a new one.");
-      editorConfig = {}; // Initialize empty config if file doesn't exist
-    }
-  } catch (error: any) {
-    // Handle file access errors separately from parsing errors
-    if (error.code !== "ENOENT") {
-      // ENOENT means file not found, which is handled above
-      spinner.fail("Failed to read configuration file.");
-      console.error(
-        chalk.red(`Error reading ${displayConfigPath}: ${error.message}`),
+  if (await fileExists(configPath)) {
+    const content = await readFile(configPath, "utf-8");
+    // Attempt to parse JSON, handle potential comments if needed
+    try {
+      editorConfig = parseJSON(content);
+    } catch {
+      spinner.fail(
+        `Failed to parse configuration file ${chalk.cyan(displayConfigPath)}.`,
       );
-      const proceed = await confirm({
-        message: "Reading failed. Attempt to overwrite the file?",
-        default: false,
-      });
-      if (!proceed) return;
-      editorConfig = {}; // Start fresh if reading failed and user agreed
-    } else {
-      // This case should ideally be caught by the fileExists check, but handle defensively
-      spinner.info("Configuration file not found, creating a new one.");
-      editorConfig = {};
     }
+    spinner.succeed(
+      `Read configuration file ${chalk.cyan(displayConfigPath)}.`,
+    );
+  } else {
+    spinner.info("Configuration file not found, will create a new one.");
+    editorConfig = {}; // Initialize empty config if file doesn't exist
   }
 
-  // Ensure mcpServers object exists and get existing Bucket entries
+  // Ensure MCP servers object exists
   const serversConfig = getServersConfig(
     editorConfig,
     selectedEditor,
     configPathType,
   );
+  // Check for existing Bucket servers
   const existingBucketEntries = Object.keys(serversConfig).filter((key) =>
     /bucket/i.test(key),
   );
@@ -203,10 +184,10 @@ export const mcpAction = async (options: {
 
   const newEntryValue = {
     type: "stdio",
-    command: "npx", // Assuming npx is standard
+    command: "npx",
     args: [
-      "mcp-remote@next", // Use the specified package
-      mcpUrlWithAppId, // Always include appId explicitly
+      "mcp-remote@next", // todo: remove next once stable
+      mcpUrlWithAppId, // Always include appId explicitly as that is more stable
     ],
   };
 
@@ -220,7 +201,7 @@ export const mcpAction = async (options: {
   try {
     // Ensure the directory exists before writing
     await mkdir(dirname(configPath), { recursive: true });
-    const configString = JSON.stringify(editorConfig, null, 2); // Pretty print JSON
+    const configString = stringifyJSON(editorConfig, null, 2);
     await writeFile(configPath, configString);
     spinner.succeed(
       `Configuration updated successfully in ${chalk.cyan(displayConfigPath)}.`,
@@ -231,7 +212,9 @@ export const mcpAction = async (options: {
       ),
     );
   } catch (error) {
-    spinner.fail("Failed to write configuration file.");
+    spinner.fail(
+      `Failed to write configuration file ${chalk.cyan(displayConfigPath)}.`,
+    );
     void handleError(error, "MCP Configuration");
   }
 };

packages/cli/package.json

@@ -46,10 +46,10 @@
     "chalk": "^5.3.0",
     "change-case": "^5.4.4",
     "commander": "^12.1.0",
+    "comment-json": "^4.2.5",
     "express": "^4.21.2",
     "fast-deep-equal": "^3.1.3",
     "find-up": "^7.0.0",
-    "json5": "^2.2.3",
     "open": "^10.1.0",
     "ora": "^8.1.0",
     "slug": "^10.0.0",

packages/cli/stores/config.ts

@@ -1,7 +1,11 @@
 import { Ajv, ValidateFunction } from "ajv";
+import {
+  assign as assignJSON,
+  parse as parseJSON,
+  stringify as stringifyJSON,
+} from "comment-json";
 import equal from "fast-deep-equal";
 import { findUp } from "find-up";
-import JSON5 from "json5";
 import { readFile, writeFile } from "node:fs/promises";
 import { dirname, join } from "node:path";
 import { fileURLToPath } from "node:url";
@@ -71,7 +75,7 @@ class ConfigStore {
         "schema.json",
       );
       const content = await readFile(schemaPath, "utf-8");
-      const parsed = JSON5.parse<Config>(content);
+      const parsed = parseJSON(content) as unknown as Config;
       const ajv = new Ajv();
       this.validateConfig = ajv.compile(parsed);
     } catch {
@@ -95,7 +99,7 @@ class ConfigStore {
       if (!this.configPath) return;
 
       const content = await readFile(this.configPath, "utf-8");
-      const parsed = JSON5.parse<Partial<Config>>(content);
+      const parsed = parseJSON(content) as unknown as Partial<Config>;
 
       // Normalize values
       if (parsed.baseUrl)
@@ -112,7 +116,7 @@ class ConfigStore {
         );
       }
 
-      this.config = { ...this.config, ...parsed };
+      this.config = assignJSON(this.config, parsed);
     } catch {
       // No config file found
     }
@@ -123,9 +127,7 @@ class ConfigStore {
    * @param overwrite If true, overwrites existing config file. Defaults to false
    */
   async saveConfigFile(overwrite = false) {
-    const configWithoutDefaults: Partial<Config> = {
-      ...this.config,
-    };
+    const configWithoutDefaults: Partial<Config> = assignJSON({}, this.config);
 
     // Only include non-default values and $schema
     for (const untypedKey in configWithoutDefaults) {
@@ -138,7 +140,7 @@ class ConfigStore {
       }
     }
 
-    const configJSON = JSON.stringify(configWithoutDefaults, null, 2);
+    const configJSON = stringifyJSON(configWithoutDefaults, null, 2);
 
     if (this.configPath && !overwrite) {
       throw new Error("Config file already exists");