feat: setValues合并算法支持嵌套对象赋值，解决嵌套对象无法正常初始化问题 #6352
Conversation
1455668754
requested review from
anncwb,
vince292007,
mynetfan and
jinmao88
as code owners
|
WalkthroughReplaced the external merge utility with an inlined deep-merge function and added a pickFields helper. setValues now deep-merges current values with incoming fields, filters to schema-defined fields, converts string/array fields, and then sets values with explicit boolean parameters. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant FormApi
participant fieldMergeFn
participant pickFields
participant handler as handleStringToArrayFields
participant Form as form.setValues
Caller->>FormApi: setValues(fields, filterFields=true, shouldValidate=false)
FormApi->>fieldMergeFn: fieldMergeFn(form.values, fields)
fieldMergeFn-->>FormApi: mergedValues
alt filterFields == true
FormApi->>pickFields: pickFields(mergedValues, schemaFieldNames)
pickFields-->>FormApi: filteredValues
else
Note right of FormApi: filteredValues = mergedValues
end
FormApi->>handler: handleStringToArrayFields(filteredValues)
handler-->>FormApi: normalizedValues
FormApi->>Form: form.setValues(normalizedValues, shouldValidate)
Form-->>Caller: completion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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: 0
🧹 Nitpick comments (1)
packages/@core/ui-kit/form-ui/src/form-api.ts (1)
368-397: Consider adding support for array indices in field paths.The current implementation doesn't support array indices in paths (e.g., 'items.0.name'). If your form schemas use array fields with indexed access, consider extending the function to handle numeric keys.
Would you like me to provide an implementation that supports array indices in field paths?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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)
isDate(20-22)isDayjsObject(24-26)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Test (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (3)
packages/@core/ui-kit/form-ui/src/form-api.ts (3)
295-351: LGTM! The deep merge implementation handles nested objects correctly.The
fieldMergeFnfunction properly implements deep merging with appropriate handling for arrays, Date/Dayjs objects, and null/undefined values. This aligns well with the PR objective of supporting nested object assignment.
399-403: Well-structured implementation for filtered nested object merging.The approach of first merging with
fieldMergeFnand then filtering withpickFieldsensures that nested objects are properly merged while maintaining schema integrity. This effectively solves the nested object initialization problem mentioned in the PR objectives.
379-379: Fix the falsy value handling in field extraction.The condition
value && typeof value === 'object'will incorrectly skip falsy values like0,false, or empty strings. This could lead to data loss when extracting fields with falsy values.Apply this diff to fix the falsy value handling:
- if (value && typeof value === 'object' && key in value) { + if (value !== null && value !== undefined && typeof value === 'object' && key in value) {Likely an incorrect or invalid review comment.
|
能否提供一个测试demo |
|
示例： 此结构在原有代码中，只有字段1能正常被初始化值，字段2及内部的对象无法正常进行初始化 |
There was a problem hiding this comment.
Actionable comments posted: 2
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)
326-329: Apply string→array normalization also when filterFields is falseCurrently the no-filter branch bypasses handleStringToArrayFields, which can lead to inconsistent shapes. Normalize before setValues in both branches.
- if (!filterFields) { - form.setValues(fields, shouldValidate); - return; - } + if (!filterFields) { + const nextFields = { ...fields }; + this.handleStringToArrayFields(nextFields); + form.setValues(nextFields, shouldValidate); + return; + }
🧹 Nitpick comments (2)
packages/@core/ui-kit/form-ui/src/form-api.ts (2)
358-361: Reconsider null/undefined "ignore" semantics (may block explicit clears)Ignoring null/undefined in source prevents callers from clearing existing values (e.g., setting a field to null). If this is intentional, fine; otherwise, consider a flag to allow explicit clearing, or treat null as a legitimate "clear" while still ignoring undefined.
Would you like me to propose a small optional parameter (e.g., { ignoreNull: boolean }) to control this behavior?
435-439: Prefer merging with toRaw(form.values) to avoid merging Vue proxiesMinor, but reduces surprises if form.values is reactive.
- const filteredFields = pickFields( - fieldMergeFn(form.values, fields), + const filteredFields = pickFields( + fieldMergeFn(toRaw(form.values), fields), fieldNames, );
📜 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)
isDate(20-22)isDayjsObject(24-26)
⏰ 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). (7)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
🔇 Additional comments (2)
packages/@core/ui-kit/form-ui/src/form-api.ts (2)
331-347: Good direction: deep-merge + schema picking enables nested initializationThe overall approach (deep merge first, then pick schema fields) correctly addresses the nested object initialization gap described in the PR. The documentation here is clear and helpful.
320-324: No internalsetValuescall‐sites foundI ran a global search for 1-, 2-, and 3-argument calls to
setValues(excluding its own definition) and found zero matches in the codebase. Changing the default offilterFieldstotruewon’t affect any existing internal usage.• If this is part of a public API, please verify external consumers or update the docs to call out the new default.
| const fieldMergeFn = ( | ||
| target: Record<string, any>, | ||
| source: Record<string, any>, | ||
| ) => { | ||
| const result = { ...target }; | ||
|
|
||
| for (const key in source) { | ||
| const targetValue = result[key]; | ||
| const sourceValue = source[key]; | ||
|
|
||
| // 如果 sourceValue 是 null 或 undefined，保留旧值 | ||
| if (sourceValue === null || sourceValue === undefined) { | ||
| continue; | ||
| } | ||
|
|
||
| // 如果 sourceValue 是数组，直接覆盖 | ||
| if (Array.isArray(sourceValue)) { | ||
| result[key] = sourceValue; | ||
| } | ||
| // 如果 sourceValue 是对象（非 Date、非 Dayjs），进行深度合并 | ||
| else if ( | ||
| isObject(sourceValue) && | ||
| !isDate(sourceValue) && | ||
| !isDayjsObject(sourceValue) | ||
| ) { | ||
| result[key] = | ||
| isObject(targetValue) && | ||
| !isDate(targetValue) && | ||
| !isDayjsObject(targetValue) | ||
| ? fieldMergeFn(targetValue, sourceValue) | ||
| : sourceValue; | ||
| } | ||
| // 其他情况（如基本类型）直接赋值 | ||
| else { | ||
| result[key] = sourceValue; | ||
| } | ||
| } | ||
| return true; | ||
| }); | ||
| const filteredFields = fieldMergeFn(fields, form.values); | ||
|
|
||
| return result; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden deep-merge: prevent prototype pollution, avoid for-in over prototypes, and clone arrays to avoid aliasing
- Security: Merging untrusted input can allow writing to proto/prototype/constructor keys, leading to prototype pollution.
- Correctness: for..in walks inherited enumerable props. Prefer Object.keys.
- Immutability: Assigning arrays by reference means subsequent mutations can leak back to the input object.
Apply these changes:
- const fieldMergeFn = (
+ const fieldMergeFn = (
target: Record<string, any>,
source: Record<string, any>,
) => {
const result = { ...target };
- for (const key in source) {
- const targetValue = result[key];
- const sourceValue = source[key];
+ for (const key of Object.keys(source)) {
+ // Guard against prototype pollution
+ if (key === '__proto__' || key === 'prototype' || key === 'constructor') {
+ continue;
+ }
+ const targetValue = result[key];
+ const sourceValue = (source as Record<string, any>)[key];
// 如果 sourceValue 是 null 或 undefined，保留旧值
- if (sourceValue === null || sourceValue === undefined) {
+ if (sourceValue == null) {
continue;
}
// 如果 sourceValue 是数组，直接覆盖
- if (Array.isArray(sourceValue)) {
- result[key] = sourceValue;
+ if (Array.isArray(sourceValue)) {
+ // shallow-clone array to avoid sharing references
+ result[key] = sourceValue.slice();
}
// 如果 sourceValue 是对象（非 Date、非 Dayjs），进行深度合并
else if (
isObject(sourceValue) &&
!isDate(sourceValue) &&
!isDayjsObject(sourceValue)
) {
- result[key] =
- isObject(targetValue) &&
- !isDate(targetValue) &&
- !isDayjsObject(targetValue)
- ? fieldMergeFn(targetValue, sourceValue)
- : sourceValue;
+ result[key] =
+ isObject(targetValue) &&
+ !isDate(targetValue) &&
+ !isDayjsObject(targetValue)
+ ? fieldMergeFn(targetValue, sourceValue)
+ // shallow-clone object to avoid sharing references with source
+ : { ...sourceValue };
}
// 其他情况（如基本类型）直接赋值
else {
result[key] = sourceValue;
}
}
return result;
};📝 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.
| const fieldMergeFn = ( | |
| target: Record<string, any>, | |
| source: Record<string, any>, | |
| ) => { | |
| const result = { ...target }; | |
| for (const key in source) { | |
| const targetValue = result[key]; | |
| const sourceValue = source[key]; | |
| // 如果 sourceValue 是 null 或 undefined，保留旧值 | |
| if (sourceValue === null || sourceValue === undefined) { | |
| continue; | |
| } | |
| // 如果 sourceValue 是数组，直接覆盖 | |
| if (Array.isArray(sourceValue)) { | |
| result[key] = sourceValue; | |
| } | |
| // 如果 sourceValue 是对象（非 Date、非 Dayjs），进行深度合并 | |
| else if ( | |
| isObject(sourceValue) && | |
| !isDate(sourceValue) && | |
| !isDayjsObject(sourceValue) | |
| ) { | |
| result[key] = | |
| isObject(targetValue) && | |
| !isDate(targetValue) && | |
| !isDayjsObject(targetValue) | |
| ? fieldMergeFn(targetValue, sourceValue) | |
| : sourceValue; | |
| } | |
| // 其他情况（如基本类型）直接赋值 | |
| else { | |
| result[key] = sourceValue; | |
| } | |
| } | |
| return true; | |
| }); | |
| const filteredFields = fieldMergeFn(fields, form.values); | |
| return result; | |
| }; | |
| const fieldMergeFn = ( | |
| target: Record<string, any>, | |
| source: Record<string, any>, | |
| ) => { | |
| const result = { ...target }; | |
| for (const key of Object.keys(source)) { | |
| // Guard against prototype pollution | |
| if (key === '__proto__' || key === 'prototype' || key === 'constructor') { | |
| continue; | |
| } | |
| const targetValue = result[key]; | |
| const sourceValue = (source as Record<string, any>)[key]; | |
| // 如果 sourceValue 是 null 或 undefined，保留旧值 | |
| if (sourceValue == null) { | |
| continue; | |
| } | |
| // 如果 sourceValue 是数组，直接覆盖 | |
| if (Array.isArray(sourceValue)) { | |
| // shallow-clone array to avoid sharing references | |
| result[key] = sourceValue.slice(); | |
| } | |
| // 如果 sourceValue 是对象（非 Date、非 Dayjs），进行深度合并 | |
| else if ( | |
| isObject(sourceValue) && | |
| !isDate(sourceValue) && | |
| !isDayjsObject(sourceValue) | |
| ) { | |
| result[key] = | |
| isObject(targetValue) && | |
| !isDate(targetValue) && | |
| !isDayjsObject(targetValue) | |
| ? fieldMergeFn(targetValue, sourceValue) | |
| // shallow-clone object to avoid sharing references with source | |
| : { ...sourceValue }; | |
| } | |
| // 其他情况（如基本类型）直接赋值 | |
| else { | |
| result[key] = sourceValue; | |
| } | |
| } | |
| return result; | |
| }; |
🤖 Prompt for AI Agents
In packages/@core/ui-kit/form-ui/src/form-api.ts around lines 348 to 387, the
deep-merge allows prototype pollution, iterates inherited props with for..in,
and assigns arrays by reference; fix by iterating only own keys (use
Object.keys(source)), skip dangerous keys like "__proto__", "prototype", and
"constructor" (check and continue), clone arrays before assigning (e.g., create
a shallow copy), and keep the existing deep-merge recursion but only when both
source and target are plain objects; ensure you do not traverse inherited
properties and do not assign array references directly to avoid aliasing.
| /** | ||
| * 从对象中提取指定字段路径的子集，支持多级嵌套字段（如 'user.address.city'） | ||
| * | ||
| * @param obj - 要从中提取字段的对象 | ||
| * @param filedNames - 字段路径数组，可以是多级字段（例如 ['user.name', 'user.age']） | ||
| * @returns 返回一个新对象，仅包含 `filedNames` 所指定的字段及其值 | ||
| * | ||
| * @example | ||
| * const obj = { | ||
| * user: { name: 'Alice', age: 25 }, | ||
| * email: '[email protected]' | ||
| * }; | ||
| * pickFields(obj, ['user.name', 'email']); | ||
| * // 返回: { user: { name: 'Alice' }, email: '[email protected]' } | ||
| */ | ||
| function pickFields(obj: Record<string, any>, filedNames: string[]) { | ||
| const result: Record<string, any> = {}; | ||
|
|
||
| for (const path of filedNames) { | ||
| const keys: string[] = path.split('.'); | ||
| let value: any = obj; | ||
| let target: any = result; | ||
|
|
||
| for (let i = 0; i < keys.length; i++) { | ||
| const key = keys[i] as string; | ||
|
|
||
| if (value && typeof value === 'object' && key in value) { | ||
| value = value[key]; | ||
| if (i === keys.length - 1) { | ||
| // 最后一级字段存在才赋值 | ||
| target[key] = value; | ||
| } else { | ||
| // 初始化中间结构 | ||
| target[key] = target[key] || {}; | ||
| target = target[key]; | ||
| } | ||
| } else { | ||
| // 路径不存在则跳过 | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden pickFields: block prototype keys, use hasOwnProperty, and fix typo in parameter name
- Security: Prevent paths containing proto/prototype/constructor.
- Correctness: Use Object.prototype.hasOwnProperty to avoid inherited props.
- Typo: filedNames → fieldNames. Also trim and skip empty paths.
- function pickFields(obj: Record<string, any>, filedNames: string[]) {
+ function pickFields(obj: Record<string, any>, fieldNames: string[]) {
const result: Record<string, any> = {};
- for (const path of filedNames) {
- const keys: string[] = path.split('.');
+ for (const rawPath of fieldNames) {
+ const path = String(rawPath).trim();
+ if (!path) continue;
+ const keys: string[] = path.split('.');
let value: any = obj;
let target: any = result;
for (let i = 0; i < keys.length; i++) {
const key = keys[i] as string;
- if (value && typeof value === 'object' && key in value) {
+ // Block prototype-polluting keys
+ if (key === '__proto__' || key === 'prototype' || key === 'constructor') {
+ break;
+ }
+
+ if (
+ value &&
+ typeof value === 'object' &&
+ Object.prototype.hasOwnProperty.call(value, key)
+ ) {
value = value[key];
if (i === keys.length - 1) {
// 最后一级字段存在才赋值
target[key] = value;
} else {
// 初始化中间结构
- target[key] = target[key] || {};
+ target[key] = isObject(target[key]) ? target[key] : {};
target = target[key];
}
} else {
// 路径不存在则跳过
break;
}
}
}
return result;
}Committable suggestion skipped: line range outside the PR's diff.
|
处理一下ai的提示，然后在playground 增加一下相关demo |
setValues合并算法支持嵌套对象赋值，解决嵌套对象无法正常初始化问题
Summary by CodeRabbit
New Features
Bug Fixes