fix(@vben-core/form-ui): 支持object类型值的合并 #6709
Conversation
修复 FormApi.setValues 函数 values 多层嵌套 object 时无法合并导致依然是默认值的问题
XinRoom
requested review from
anncwb,
vince292007,
mynetfan and
jinmao88
as code owners
|
WalkthroughTightens FormApi.setValues merge behavior: only performs in-place recursive merge when the incoming value is an object; otherwise assigns the incoming value directly. Preserves Dayjs/Date and array checks; adds comments and returns false to signal the key was handled. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant FormApi as FormApi.setValues
participant Merge as fieldMergeFn
Caller->>FormApi: setValues(partialValues)
loop for each key,value in partialValues
alt obj[key] is non-array object AND value is object
rect rgb(220,240,255)
Note over FormApi,Merge: Changed: recurse only when both are objects
FormApi->>Merge: fieldMergeFn(obj[key], value)
Merge-->>FormApi: merged in-place
end
FormApi-->>FormApi: return false (handled)
else if value is Array
Note over FormApi: Array handling (unchanged)
FormApi-->>FormApi: default assign/merge
else if value is Dayjs/Date
Note over FormApi: Dayjs/Date checks (unchanged)
else
Note over FormApi: Direct assignment for non-object value
FormApi-->>FormApi: obj[key] = value
FormApi-->>FormApi: return false (handled)
end
end
FormApi-->>Caller: done
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/@core/ui-kit/form-ui/src/form-api.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/@core/ui-kit/form-ui/src/form-api.ts (2)
packages/@core/base/shared/src/utils/inference.ts (1)
isObject(160-160)packages/@core/base/shared/src/utils/date.ts (2)
isDayjsObject(24-26)isDate(20-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (2)
packages/@core/ui-kit/form-ui/src/form-api.ts (2)
339-349: Solid fix: only deep-merge object→object and skip default merge.This closes the multi-layer object merge gap while preserving Dayjs/Date and array behavior on the target side. The in-place merge is appropriate for avoiding unintended overwrites.
339-349: Double-check vee-validate meta/dirty updates with in-place nested writes.Because nested merges bypass
setValuesfor those top-level keys, ensure vee-validate still flags fields as dirty/touched as expected. If not, consider emitting the top-level key infilteredFields(with a shallow reference bump) to drive form meta updates.
| if (!Array.isArray(obj[key]) && | ||
| isObject(obj[key]) && | ||
| !isDayjsObject(obj[key]) && | ||
| !isDate(obj[key]) && | ||
| isObject(value)) { // 添加对 value 也是对象的检查 | ||
| // 递归合并对象 | ||
| fieldMergeFn(obj[key], value); | ||
| } else { | ||
| obj[key] = value; // 直接赋值 | ||
| } | ||
| return false; // 返回 false 表示已经处理，不需要默认合并 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Also guard the incoming value for arrays/Dayjs/Date to avoid accidental deep-merge or no-op.
If isObject(value) returns true for arrays or Date-like objects in your util, we might deep-merge where we should assign, or silently do nothing (e.g., merging a Date). Recommend symmetric checks on value.
Apply this minimal diff:
- if (!Array.isArray(obj[key]) &&
- isObject(obj[key]) &&
- !isDayjsObject(obj[key]) &&
- !isDate(obj[key]) &&
- isObject(value)) { // 添加对 value 也是对象的检查
+ if (!Array.isArray(obj[key]) &&
+ isObject(obj[key]) &&
+ !isDayjsObject(obj[key]) &&
+ !isDate(obj[key]) &&
+ isObject(value) &&
+ !Array.isArray(value) &&
+ !isDayjsObject(value) &&
+ !isDate(value)) { // 双方均为纯对象才递归合并
// 递归合并对象
fieldMergeFn(obj[key], value);
} else {
obj[key] = value; // 直接赋值
}📝 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.
| if (!Array.isArray(obj[key]) && | |
| isObject(obj[key]) && | |
| !isDayjsObject(obj[key]) && | |
| !isDate(obj[key]) && | |
| isObject(value)) { // 添加对 value 也是对象的检查 | |
| // 递归合并对象 | |
| fieldMergeFn(obj[key], value); | |
| } else { | |
| obj[key] = value; // 直接赋值 | |
| } | |
| return false; // 返回 false 表示已经处理，不需要默认合并 | |
| if (!Array.isArray(obj[key]) && | |
| isObject(obj[key]) && | |
| !isDayjsObject(obj[key]) && | |
| !isDate(obj[key]) && | |
| isObject(value) && | |
| !Array.isArray(value) && | |
| !isDayjsObject(value) && | |
| !isDate(value)) { // 双方均为纯对象才递归合并 | |
| // 递归合并对象 | |
| fieldMergeFn(obj[key], value); | |
| } else { | |
| obj[key] = value; // 直接赋值 | |
| } | |
| return false; // 返回 false 表示已经处理，不需要默认合并 |
🤖 Prompt for AI Agents
In packages/@core/ui-kit/form-ui/src/form-api.ts around lines 339 to 349, the
merge branch only checks obj[key] for arrays/Dayjs/Date but not the incoming
value, which can cause accidental deep-merge of arrays/Dayjs/Date or no-op on
Date-like objects; update the conditional to also guard the incoming value by
requiring !Array.isArray(value) && !isDayjsObject(value) && !isDate(value) (in
addition to isObject(value)) so only plain objects are recursively merged and
all others are assigned.
💡 Verification agent
🧩 Analysis chain
Confirm createMerge and isObject contracts to ensure no surprises.
- Verify
isObjectexcludes arrays and Date/Dayjs; otherwise the above guard is required. - Confirm
createMerge’s callback semantics (returningfalsefilters the handled key from the resulting patch, andobjis the target). This underpins the “in-place merge + filteredFields” flow.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== isObject implementation ==="
sed -n '1,220p' packages/@core/base/shared/src/utils/inference.ts | rg -nC3 'export (const|function) isObject'
echo -e "\n=== createMerge definition & docs ==="
fd -t f --full-path 'packages' -g '**/*.ts' | rg -nP 'export (const|function)\s+createMerge|function createMerge' -n -H -C3
echo -e "\n=== createMerge usage examples (to confirm return false semantics) ==="
rg -nP --glob '!**/node_modules/**' 'createMerge\(' -C2Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== find isObject across repo ====="
rg -nP "\bisObject\b" -g "!**/node_modules/**" -C2 || echo "No matches for isObject"
echo
echo "===== find createMerge across repo ====="
rg -nP "\bcreateMerge\b" -g "!**/node_modules/**" -C2 || echo "No matches for createMerge"Length of output: 9491
Strengthen object‐guards & fix merge‐callback return
isObjectreturns true for arrays and Date objects; extend thevaluecheck to also exclude arrays,Dateand Dayjs before recursing.createMerge(alias ofcreateDefu) requires the merger callback to returntruewhen it has applied a custom merge (to skip the default behavior); change the existingreturn falsetoreturn true. (npmjs.com)
🤖 Prompt for AI Agents
In packages/@core/ui-kit/form-ui/src/form-api.ts around lines 339 to 349,
strengthen the object guard and fix the merge callback return: when checking
value before recursing, ensure you exclude arrays, Date objects and Dayjs
objects (similar to the checks for obj[key]) so you only recurse for plain
objects; and change the merger callback to return true (not false) to signal to
createMerge/createDefu that the custom merge was applied and default merging
should be skipped.
|
出现了一个新bug，数组类型的合并后不是取设定值，而是设定值和默认值进行了合并 我将非isObject的return true;后，修复了这个问题，但是很奇怪，我不能理解 |
修复 vbenjs@00bb25f 下 Array 类型设定值与默认值合并后出现和叠加问题 setValues({a:[1]},{a:[1]}) => {a:[1,1]}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/@core/ui-kit/form-ui/src/form-api.ts (1)
337-355: Swap return value in object branch to skip default merge
In packages/@core/ui-kit/form-ui/src/form-api.ts (lines 337–343), change the object-merge branch toreturn trueinstead ofreturn false.createMergeis an alias fordefu’screateDefu, whose callback must returntruewhen it has applied custom merging to prevent the default deep-merge behavior (npmjs.com).
♻️ Duplicate comments (1)
packages/@core/ui-kit/form-ui/src/form-api.ts (1)
339-350: Fix merge-callback return and guard incoming value to stop array duplication.Returning
falseafter the recursive merge lets the default merge continue, which concatenates arrays (e.g.,role_ids→[1,1]). Also guardvalueagainst Array/Dayjs/Date so only plain objects recurse.- if (!Array.isArray(obj[key]) && - isObject(obj[key]) && - !isDayjsObject(obj[key]) && - !isDate(obj[key]) && - isObject(value)) { // 添加对 value 也是对象的检查 + if (!Array.isArray(obj[key]) && + isObject(obj[key]) && + !isDayjsObject(obj[key]) && + !isDate(obj[key]) && + isObject(value) && + !Array.isArray(value) && + !isDayjsObject(value) && + !isDate(value)) { // 双方均为纯对象才递归合并 // 递归合并对象 fieldMergeFn(obj[key], value); - return false; + return true; // 已处理，跳过默认合并，避免数组被拼接 } else { obj[key] = value; // 直接赋值 return true; }
🧹 Nitpick comments (1)
packages/@core/ui-kit/form-ui/src/form-api.ts (1)
321-357: Add unit tests for merge behavior (arrays replace; Date/Dayjs assign; deep objects merge).Cover cases:
- Arrays replace, not concat: default
[1]+ incoming[1]→[1].- Deep object merge with plain objects only.
- Date/Dayjs assigned (no deep merge).
- Mixed: object with nested array replaced.
I can draft vitest specs targeting
FormApi.setValueswith these scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/@core/ui-kit/form-ui/src/form-api.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/@core/ui-kit/form-ui/src/form-api.ts (2)
packages/@core/base/shared/src/utils/inference.ts (1)
isObject(160-160)packages/@core/base/shared/src/utils/date.ts (2)
isDayjsObject(24-26)isDate(20-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
修复 FormApi.setValues 函数 values 多层嵌套 object 时无法合并导致依然是默认值的问题
Description
Type of change
Please delete options that are not relevant.
Checklist
pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
Bug Fixes
Refactor