Fix JSON Editor to display existing data in visual mode on mount#4977
Open
pzyyll wants to merge 2 commits into
Open
Fix JSON Editor to display existing data in visual mode on mount#4977pzyyll wants to merge 2 commits into
pzyyll wants to merge 2 commits into
Conversation
…ount Previously, the JsonEditor rows state was initialized as an empty array, while the value and jsonValue states were initialized to the prop value. The useEffect guard `value !== jsonValue` would always be false on mount, preventing parseJsonToRows from ever running. This caused visual mode to display empty state even when the value prop contained valid JSON. Now, rows is initialized synchronously via a lazy useState initializer using a shared parseJsonRows helper, which also replaces the inline parse logic. Invalid JSON values still gracefully fall through to empty rows (null return preserved on parse failure).
Contributor
WalkthroughA ChangesJSON Editor Parsing Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/default/src/components/json-editor.tsx`:
- Around line 46-61: parseJsonRows currently calls Object.entries(parsed)
without ensuring parsed is a plain object; update parseJsonRows to validate
after JSON.parse that parsed is an object suitable for key/value rows (i.e.,
typeof parsed === 'object', parsed !== null, and !Array.isArray(parsed)) and
return null for any non-object JSON (arrays, primitives, strings). Keep the
existing mapping to EditorRow and id generation when the check passes; ensure
the validation occurs before calling Object.entries so non-object inputs yield
null.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13eace38-c115-48c8-bce3-d22ed893ea32
📒 Files selected for processing (1)
web/default/src/components/json-editor.tsx
Comment on lines
+46
to
+61
| function parseJsonRows(json: string): EditorRow[] | null { | ||
| try { | ||
| if (!json.trim()) { | ||
| return [] | ||
| } | ||
|
|
||
| const parsed = JSON.parse(json) | ||
| return Object.entries(parsed).map(([key, val], index) => ({ | ||
| id: `${Date.now()}-${index}`, | ||
| key, | ||
| value: typeof val === 'object' ? JSON.stringify(val) : String(val), | ||
| })) | ||
| } catch (_error) { | ||
| return null | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add type validation to ensure parsed JSON is a plain object.
The function calls Object.entries(parsed) without checking whether parsed is actually an object. Non-object JSON values will produce incorrect results:
- Arrays like
[1, 2, 3]become rows with numeric string keys ("0","1","2") - Primitives like
123ortruebecome empty rows (data loss) - Strings like
"hello"become character-indexed rows
Since the component is designed for key-value pairs (objects only), add a type guard after parsing to return null for non-object types.
🛡️ Proposed fix to validate object type
function parseJsonRows(json: string): EditorRow[] | null {
try {
if (!json.trim()) {
return []
}
const parsed = JSON.parse(json)
+ if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
+ return null
+ }
+
return Object.entries(parsed).map(([key, val], index) => ({
id: `${Date.now()}-${index}`,
key,
value: typeof val === 'object' ? JSON.stringify(val) : String(val),
}))
} catch (_error) {
return null
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function parseJsonRows(json: string): EditorRow[] | null { | |
| try { | |
| if (!json.trim()) { | |
| return [] | |
| } | |
| const parsed = JSON.parse(json) | |
| return Object.entries(parsed).map(([key, val], index) => ({ | |
| id: `${Date.now()}-${index}`, | |
| key, | |
| value: typeof val === 'object' ? JSON.stringify(val) : String(val), | |
| })) | |
| } catch (_error) { | |
| return null | |
| } | |
| } | |
| function parseJsonRows(json: string): EditorRow[] | null { | |
| try { | |
| if (!json.trim()) { | |
| return [] | |
| } | |
| const parsed = JSON.parse(json) | |
| if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { | |
| return null | |
| } | |
| return Object.entries(parsed).map(([key, val], index) => ({ | |
| id: `${Date.now()}-${index}`, | |
| key, | |
| value: typeof val === 'object' ? JSON.stringify(val) : String(val), | |
| })) | |
| } catch (_error) { | |
| return null | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/default/src/components/json-editor.tsx` around lines 46 - 61,
parseJsonRows currently calls Object.entries(parsed) without ensuring parsed is
a plain object; update parseJsonRows to validate after JSON.parse that parsed is
an object suitable for key/value rows (i.e., typeof parsed === 'object', parsed
!== null, and !Array.isArray(parsed)) and return null for any non-object JSON
(arrays, primitives, strings). Keep the existing mapping to EditorRow and id
generation when the check passes; ensure the validation occurs before calling
Object.entries so non-object inputs yield null.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
修复渠道
参数覆盖新增参数后，返回设置页面后未正常显示新增加的参数BUG。📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
之前：JsonEditor 的 rows 状态初始化为空数组，而 value 和 jsonValue 状态则初始化为 prop value。
value !== jsonValue这个 useEffect 保护条件在挂载时总是为 false，导致parseJsonToRows不会执行。修复：rows 通过一个懒初始化的 useState initializer 同步初始化，使用共享的
parseJsonRows。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
Release Notes