Support comment in json key value (#205)

* support comment in json key value

* update testcase

* strip comment only when normal parse failed

Author: zhiyuanliang-ms

package-lock.json

@@ -57,6 +57,7 @@
   "dependencies": {
     "@azure/app-configuration": "^1.6.1",
     "@azure/identity": "^4.2.1",
-    "@azure/keyvault-secrets": "^4.7.0"
+    "@azure/keyvault-secrets": "^4.7.0",
+    "jsonc-parser": "^3.3.1"
   }
 }

package.json

@@ -11,6 +11,7 @@ export default [
       "@azure/identity",
       "crypto",
       "dns/promises",
+      "jsonc-parser",
       "@microsoft/feature-management"
     ],
     input: "src/index.ts",

rollup.config.mjs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license.
 
 import { ConfigurationSetting, featureFlagContentType, secretReferenceContentType } from "@azure/app-configuration";
+import { stripComments } from "jsonc-parser";
 import { parseContentType, isJsonContentType } from "./common/contentType.js";
 import { IKeyValueAdapter } from "./IKeyValueAdapter.js";
 
@@ -25,18 +26,35 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter {
     async processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]> {
         let parsedValue: unknown;
         if (setting.value !== undefined) {
-            try {
-                parsedValue = JSON.parse(setting.value);
-            } catch (error) {
-                parsedValue = setting.value;
+            const parseResult = this.#tryParseJson(setting.value);
+            if (parseResult.success) {
+                parsedValue = parseResult.result;
+            } else {
+                // Try parsing with comments stripped
+                const parseWithoutCommentsResult = this.#tryParseJson(stripComments(setting.value));
+                if (parseWithoutCommentsResult.success) {
+                    parsedValue = parseWithoutCommentsResult.result;
+                } else {
+                    // If still not valid JSON, return the original value
+                    parsedValue = setting.value;
+                }
             }
-        } else {
-            parsedValue = setting.value;
         }
         return [setting.key, parsedValue];
     }
 
     async onChangeDetected(): Promise<void> {
         return;
     }
+
+        #tryParseJson(value: string): { success: true; result: unknown } | { success: false } {
+        try {
+            return { success: true, result: JSON.parse(value) };
+        } catch (error) {
+            if (error instanceof SyntaxError) {
+                return { success: false };
+            }
+            throw error;
+        }
+    }
 }

src/JsonKeyValueAdapter.ts

@@ -60,7 +60,7 @@ describe("json", function () {
             createMockedJsonKeyValue("json.settings.object", "{}"),
             createMockedJsonKeyValue("json.settings.array", "[]"),
             createMockedJsonKeyValue("json.settings.number", "8"),
-            createMockedJsonKeyValue("json.settings.string", "string"),
+            createMockedJsonKeyValue("json.settings.string", "\"string\""),
             createMockedJsonKeyValue("json.settings.false", "false"),
             createMockedJsonKeyValue("json.settings.true", "true"),
             createMockedJsonKeyValue("json.settings.null", "null"),
@@ -88,4 +88,87 @@ describe("json", function () {
         expect(settings.get("json.settings.emptyString")).eq("", "is empty string");
         expect(settings.get("json.settings.illegalString")).eq("[unclosed", "is illegal string");
     });
+
+    it("should load json values with comments", async () => {
+        // Test various comment styles and positions
+        const mixedCommentStylesValue = `{
+            // Single line comment at start
+            "ApiSettings": {
+                "BaseUrl": "https://api.example.com", // Inline single line
+                /* Multi-line comment
+                   spanning multiple lines */
+                "ApiKey": "secret-key",
+                "Endpoints": [
+                    // Comment before array element
+                    "/users",
+                    /* Comment between elements */
+                    "/orders",
+                    "/products" // Comment after element
+                ]
+            },
+            // Test edge cases
+            "StringWithSlashes": "This is not a // comment",
+            "StringWithStars": "This is not a /* comment */",
+            "UrlValue": "https://example.com/path", // This is a real comment
+            "EmptyComment": "value", //
+            /**/
+            "AfterEmptyComment": "value2"
+            /* Final multi-line comment */
+        }`;
+
+        // Test invalid JSON with comments
+        const invalidJsonWithCommentsValue = `// This is a comment
+                { invalid json structure
+                // Another comment
+                missing quotes and braces`;
+
+        // Test only comments (should be invalid JSON)
+        const onlyCommentsValue = `
+                // Just comments
+                /* No actual content */
+            `;
+
+        const keyValues = [
+            createMockedJsonKeyValue("MixedCommentStyles", mixedCommentStylesValue),
+            createMockedJsonKeyValue("InvalidJsonWithComments", invalidJsonWithCommentsValue),
+            createMockedJsonKeyValue("OnlyComments", onlyCommentsValue)
+        ];
+
+        mockAppConfigurationClientListConfigurationSettings([keyValues]);
+
+        const connectionString = createMockedConnectionString();
+        const settings = await load(connectionString);
+        expect(settings).not.undefined;
+
+        // Verify mixed comment styles are properly parsed
+        const mixedConfig = settings.get<any>("MixedCommentStyles");
+        expect(mixedConfig).not.undefined;
+        expect(mixedConfig.ApiSettings).not.undefined;
+        expect(mixedConfig.ApiSettings.BaseUrl).eq("https://api.example.com");
+        expect(mixedConfig.ApiSettings.ApiKey).eq("secret-key");
+        expect(mixedConfig.ApiSettings.Endpoints).not.undefined;
+        expect(Array.isArray(mixedConfig.ApiSettings.Endpoints)).eq(true);
+        expect(mixedConfig.ApiSettings.Endpoints[0]).eq("/users");
+        expect(mixedConfig.ApiSettings.Endpoints[1]).eq("/orders");
+        expect(mixedConfig.ApiSettings.Endpoints[2]).eq("/products");
+
+        // Verify edge cases where comment-like text appears in strings
+        expect(mixedConfig.StringWithSlashes).eq("This is not a // comment");
+        expect(mixedConfig.StringWithStars).eq("This is not a /* comment */");
+        expect(mixedConfig.UrlValue).eq("https://example.com/path");
+        expect(mixedConfig.EmptyComment).eq("value");
+        expect(mixedConfig.AfterEmptyComment).eq("value2");
+
+        // Invalid JSON should fall back to string value
+        const invalidConfig = settings.get("InvalidJsonWithComments");
+        expect(invalidConfig).not.undefined;
+        expect(typeof invalidConfig).eq("string");
+        expect(invalidConfig).eq(invalidJsonWithCommentsValue);
+
+        // Only comments should be treated as string value (invalid JSON)
+        const onlyCommentsConfig = settings.get("OnlyComments");
+        expect(onlyCommentsConfig).not.undefined;
+        expect(typeof onlyCommentsConfig).eq("string");
+        expect(onlyCommentsConfig).eq(onlyCommentsValue);
+    });
 });