Skip to content

Commit aad3262

Browse files
cliffhallcliffhall
authored
Merge pull request modelcontextprotocol#264 from samuel871211/addUnitTest
refactor(json): Consolidate JSON utilities and type definitions
2 parents da4e2fa + 068d432 commit aad3262

File tree

8 files changed

+221
-69
lines changed

8 files changed

+221
-69
lines changed

‎client/src/components/DynamicJsonForm.tsx

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,9 @@ import { Button } from "@/components/ui/button";
33
import { Input } from "@/components/ui/input";
44
import { Label } from "@/components/ui/label";
55
import JsonEditor from "./JsonEditor";
6-
import { updateValueAtPath, JsonObject } from "@/utils/jsonPathUtils";
6+
import { updateValueAtPath } from "@/utils/jsonUtils";
77
import { generateDefaultValue, formatFieldLabel } from "@/utils/schemaUtils";
8-
9-
export type JsonValue =
10-
| string
11-
| number
12-
| boolean
13-
| null
14-
| undefined
15-
| JsonValue[]
16-
| { [key: string]: JsonValue };
17-
18-
export type JsonSchemaType = {
19-
type:
20-
| "string"
21-
| "number"
22-
| "integer"
23-
| "boolean"
24-
| "array"
25-
| "object"
26-
| "null";
27-
description?: string;
28-
required?: boolean;
29-
default?: JsonValue;
30-
properties?: Record<string, JsonSchemaType>;
31-
items?: JsonSchemaType;
32-
};
8+
import type { JsonValue, JsonSchemaType, JsonObject } from "@/utils/jsonUtils";
339

3410
interface DynamicJsonFormProps {
3511
schema: JsonSchemaType;

‎client/src/components/JsonView.tsx

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { useState, memo, useMemo, useCallback, useEffect } from "react";
2-
import { JsonValue } from "./DynamicJsonForm";
2+
import type { JsonValue } from "@/utils/jsonUtils";
33
import clsx from "clsx";
44
import { Copy, CheckCheck } from "lucide-react";
55
import { Button } from "@/components/ui/button";
66
import { useToast } from "@/hooks/use-toast";
7+
import { getDataType, tryParseJson } from "@/utils/jsonUtils";
78

89
interface JsonViewProps {
910
data: unknown;
@@ -13,21 +14,6 @@ interface JsonViewProps {
1314
withCopyButton?: boolean;
1415
}
1516

16-
function tryParseJson(str: string): { success: boolean; data: JsonValue } {
17-
const trimmed = str.trim();
18-
if (
19-
!(trimmed.startsWith("{") && trimmed.endsWith("}")) &&
20-
!(trimmed.startsWith("[") && trimmed.endsWith("]"))
21-
) {
22-
return { success: false, data: str };
23-
}
24-
try {
25-
return { success: true, data: JSON.parse(str) };
26-
} catch {
27-
return { success: false, data: str };
28-
}
29-
}
30-
3117
const JsonView = memo(
3218
({
3319
data,
@@ -119,23 +105,15 @@ interface JsonNodeProps {
119105
const JsonNode = memo(
120106
({ data, name, depth = 0, initialExpandDepth }: JsonNodeProps) => {
121107
const [isExpanded, setIsExpanded] = useState(depth < initialExpandDepth);
122-
123-
const getDataType = (value: JsonValue): string => {
124-
if (Array.isArray(value)) return "array";
125-
if (value === null) return "null";
126-
return typeof value;
127-
};
128-
129-
const dataType = getDataType(data);
130-
131-
const typeStyleMap: Record<string, string> = {
108+
const [typeStyleMap] = useState<Record<string, string>>({
132109
number: "text-blue-600",
133110
boolean: "text-amber-600",
134111
null: "text-purple-600",
135112
undefined: "text-gray-600",
136113
string: "text-green-600 break-all whitespace-pre-wrap",
137114
default: "text-gray-700",
138-
};
115+
});
116+
const dataType = getDataType(data);
139117

140118
const renderCollapsible = (isArray: boolean) => {
141119
const items = isArray

‎client/src/components/ToolsTab.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import { Input } from "@/components/ui/input";
55
import { Label } from "@/components/ui/label";
66
import { TabsContent } from "@/components/ui/tabs";
77
import { Textarea } from "@/components/ui/textarea";
8-
import DynamicJsonForm, { JsonSchemaType, JsonValue } from "./DynamicJsonForm";
8+
import DynamicJsonForm from "./DynamicJsonForm";
9+
import type { JsonValue, JsonSchemaType } from "@/utils/jsonUtils";
910
import { generateDefaultValue } from "@/utils/schemaUtils";
1011
import {
1112
CallToolResultSchema,

‎client/src/components/__tests__/DynamicJsonForm.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { render, screen, fireEvent } from "@testing-library/react";
22
import { describe, it, expect, jest } from "@jest/globals";
33
import DynamicJsonForm from "../DynamicJsonForm";
4-
import type { JsonSchemaType } from "../DynamicJsonForm";
4+
import type { JsonSchemaType } from "@/utils/jsonUtils";
55

66
describe("DynamicJsonForm String Fields", () => {
77
const renderForm = (props = {}) => {

‎client/src/utils/__tests__/jsonPathUtils.test.ts renamed to ‎client/src/utils/__tests__/jsonUtils.test.ts

Lines changed: 149 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,146 @@
1-
import { updateValueAtPath, getValueAtPath } from "../jsonPathUtils";
2-
import { JsonValue } from "../../components/DynamicJsonForm";
1+
import {
2+
getDataType,
3+
tryParseJson,
4+
updateValueAtPath,
5+
getValueAtPath,
6+
} from "../jsonUtils";
7+
import type { JsonValue } from "../jsonUtils";
8+
9+
describe("getDataType", () => {
10+
test("should return 'string' for string values", () => {
11+
expect(getDataType("hello")).toBe("string");
12+
expect(getDataType("")).toBe("string");
13+
});
14+
15+
test("should return 'number' for number values", () => {
16+
expect(getDataType(123)).toBe("number");
17+
expect(getDataType(0)).toBe("number");
18+
expect(getDataType(-10)).toBe("number");
19+
expect(getDataType(1.5)).toBe("number");
20+
expect(getDataType(NaN)).toBe("number");
21+
expect(getDataType(Infinity)).toBe("number");
22+
});
23+
24+
test("should return 'boolean' for boolean values", () => {
25+
expect(getDataType(true)).toBe("boolean");
26+
expect(getDataType(false)).toBe("boolean");
27+
});
28+
29+
test("should return 'undefined' for undefined value", () => {
30+
expect(getDataType(undefined)).toBe("undefined");
31+
});
32+
33+
test("should return 'object' for object values", () => {
34+
expect(getDataType({})).toBe("object");
35+
expect(getDataType({ key: "value" })).toBe("object");
36+
});
37+
38+
test("should return 'array' for array values", () => {
39+
expect(getDataType([])).toBe("array");
40+
expect(getDataType([1, 2, 3])).toBe("array");
41+
expect(getDataType(["a", "b", "c"])).toBe("array");
42+
expect(getDataType([{}, { nested: true }])).toBe("array");
43+
});
44+
45+
test("should return 'null' for null value", () => {
46+
expect(getDataType(null)).toBe("null");
47+
});
48+
});
49+
50+
describe("tryParseJson", () => {
51+
test("should correctly parse valid JSON object", () => {
52+
const jsonString = '{"name":"test","value":123}';
53+
const result = tryParseJson(jsonString);
54+
55+
expect(result.success).toBe(true);
56+
expect(result.data).toEqual({ name: "test", value: 123 });
57+
});
58+
59+
test("should correctly parse valid JSON array", () => {
60+
const jsonString = '[1,2,3,"test"]';
61+
const result = tryParseJson(jsonString);
62+
63+
expect(result.success).toBe(true);
64+
expect(result.data).toEqual([1, 2, 3, "test"]);
65+
});
66+
67+
test("should correctly parse JSON with whitespace", () => {
68+
const jsonString = ' { "name" : "test" } ';
69+
const result = tryParseJson(jsonString);
70+
71+
expect(result.success).toBe(true);
72+
expect(result.data).toEqual({ name: "test" });
73+
});
74+
75+
test("should correctly parse nested JSON structures", () => {
76+
const jsonString =
77+
'{"user":{"name":"test","details":{"age":30}},"items":[1,2,3]}';
78+
const result = tryParseJson(jsonString);
79+
80+
expect(result.success).toBe(true);
81+
expect(result.data).toEqual({
82+
user: {
83+
name: "test",
84+
details: {
85+
age: 30,
86+
},
87+
},
88+
items: [1, 2, 3],
89+
});
90+
});
91+
92+
test("should correctly parse empty objects and arrays", () => {
93+
expect(tryParseJson("{}").success).toBe(true);
94+
expect(tryParseJson("{}").data).toEqual({});
95+
96+
expect(tryParseJson("[]").success).toBe(true);
97+
expect(tryParseJson("[]").data).toEqual([]);
98+
});
99+
100+
test("should return failure for non-JSON strings", () => {
101+
const nonJsonString = "this is not json";
102+
const result = tryParseJson(nonJsonString);
103+
104+
expect(result.success).toBe(false);
105+
expect(result.data).toBe(nonJsonString);
106+
});
107+
108+
test("should return failure for malformed JSON", () => {
109+
const malformedJson = '{"name":"test",}';
110+
const result = tryParseJson(malformedJson);
111+
112+
expect(result.success).toBe(false);
113+
expect(result.data).toBe(malformedJson);
114+
});
115+
116+
test("should return failure for strings with correct delimiters but invalid JSON", () => {
117+
const invalidJson = "{name:test}";
118+
const result = tryParseJson(invalidJson);
119+
120+
expect(result.success).toBe(false);
121+
expect(result.data).toBe(invalidJson);
122+
});
123+
124+
test("should handle edge cases", () => {
125+
expect(tryParseJson("").success).toBe(false);
126+
expect(tryParseJson("").data).toBe("");
127+
128+
expect(tryParseJson(" ").success).toBe(false);
129+
expect(tryParseJson(" ").data).toBe(" ");
130+
131+
expect(tryParseJson("null").success).toBe(false);
132+
expect(tryParseJson("null").data).toBe("null");
133+
134+
expect(tryParseJson('"string"').success).toBe(false);
135+
expect(tryParseJson('"string"').data).toBe('"string"');
136+
137+
expect(tryParseJson("123").success).toBe(false);
138+
expect(tryParseJson("123").data).toBe("123");
139+
140+
expect(tryParseJson("true").success).toBe(false);
141+
expect(tryParseJson("true").data).toBe("true");
142+
});
143+
});
3144

4145
describe("updateValueAtPath", () => {
5146
// Basic functionality tests
@@ -8,17 +149,17 @@ describe("updateValueAtPath", () => {
8149
});
9150

10151
test("initializes an empty object when input is null/undefined and path starts with a string", () => {
11-
expect(updateValueAtPath(null as any, ["foo"], "bar")).toEqual({
152+
expect(updateValueAtPath(null, ["foo"], "bar")).toEqual({
12153
foo: "bar",
13154
});
14-
expect(updateValueAtPath(undefined as any, ["foo"], "bar")).toEqual({
155+
expect(updateValueAtPath(undefined, ["foo"], "bar")).toEqual({
15156
foo: "bar",
16157
});
17158
});
18159

19160
test("initializes an empty array when input is null/undefined and path starts with a number", () => {
20-
expect(updateValueAtPath(null as any, ["0"], "bar")).toEqual(["bar"]);
21-
expect(updateValueAtPath(undefined as any, ["0"], "bar")).toEqual(["bar"]);
161+
expect(updateValueAtPath(null, ["0"], "bar")).toEqual(["bar"]);
162+
expect(updateValueAtPath(undefined, ["0"], "bar")).toEqual(["bar"]);
22163
});
23164

24165
// Object update tests
@@ -152,10 +293,8 @@ describe("getValueAtPath", () => {
152293
});
153294

154295
test("returns default value when input is null/undefined", () => {
155-
expect(getValueAtPath(null as any, ["foo"], "default")).toBe("default");
156-
expect(getValueAtPath(undefined as any, ["foo"], "default")).toBe(
157-
"default",
158-
);
296+
expect(getValueAtPath(null, ["foo"], "default")).toBe("default");
297+
expect(getValueAtPath(undefined, ["foo"], "default")).toBe("default");
159298
});
160299

161300
test("handles array indices correctly", () => {

‎client/src/utils/__tests__/schemaUtils.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { generateDefaultValue, formatFieldLabel } from "../schemaUtils";
2-
import { JsonSchemaType } from "../../components/DynamicJsonForm";
2+
import type { JsonSchemaType } from "../jsonUtils";
33

44
describe("generateDefaultValue", () => {
55
test("generates default string", () => {

‎client/src/utils/jsonPathUtils.ts renamed to ‎client/src/utils/jsonUtils.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,66 @@
1-
import { JsonValue } from "../components/DynamicJsonForm";
1+
export type JsonValue =
2+
| string
3+
| number
4+
| boolean
5+
| null
6+
| undefined
7+
| JsonValue[]
8+
| { [key: string]: JsonValue };
9+
10+
export type JsonSchemaType = {
11+
type:
12+
| "string"
13+
| "number"
14+
| "integer"
15+
| "boolean"
16+
| "array"
17+
| "object"
18+
| "null";
19+
description?: string;
20+
required?: boolean;
21+
default?: JsonValue;
22+
properties?: Record<string, JsonSchemaType>;
23+
items?: JsonSchemaType;
24+
};
225

326
export type JsonObject = { [key: string]: JsonValue };
427

28+
export type DataType =
29+
| "string"
30+
| "number"
31+
| "bigint"
32+
| "boolean"
33+
| "symbol"
34+
| "undefined"
35+
| "object"
36+
| "function"
37+
| "array"
38+
| "null";
39+
40+
export function getDataType(value: JsonValue): DataType {
41+
if (Array.isArray(value)) return "array";
42+
if (value === null) return "null";
43+
return typeof value;
44+
}
45+
46+
export function tryParseJson(str: string): {
47+
success: boolean;
48+
data: JsonValue;
49+
} {
50+
const trimmed = str.trim();
51+
if (
52+
!(trimmed.startsWith("{") && trimmed.endsWith("}")) &&
53+
!(trimmed.startsWith("[") && trimmed.endsWith("]"))
54+
) {
55+
return { success: false, data: str };
56+
}
57+
try {
58+
return { success: true, data: JSON.parse(str) };
59+
} catch {
60+
return { success: false, data: str };
61+
}
62+
}
63+
564
/**
665
* Updates a value at a specific path in a nested JSON structure
766
* @param obj The original JSON value

‎client/src/utils/schemaUtils.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { JsonValue, JsonSchemaType } from "../components/DynamicJsonForm";
2-
import { JsonObject } from "./jsonPathUtils";
1+
import type { JsonValue, JsonSchemaType, JsonObject } from "./jsonUtils";
32

43
/**
54
* Generates a default value based on a JSON schema type

0 commit comments

Comments
 (0)