fix: preserve duplicated legacy form fields#174
Conversation
|
Someone is attempting to deploy a commit to the pro-components Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough此 PR 在 create 中包装 options.mapProps,重写 mappedProps.form.getFieldDecorator,用 Set 跟踪已装饰字段名;首次注册使用原 fieldOptions,重复注册时合并 {preserve: true}。新增测试验证重复字段值同步且不被无关字段更新影响;并在 package.json 中新增 Changes重复字段装饰器支持
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the Form.create method to automatically preserve values for duplicated fields by wrapping form.getFieldDecorator and setting preserve: true for duplicate field names, accompanied by a new test case. The reviewer identified a critical issue where mutating form.getFieldDecorator on every render causes infinite recursion and memory leaks, and suggested caching the original function on the form instance to prevent this.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| mapProps(props: any) { | ||
| const mappedProps = mapProps ? mapProps.call(this, props) : props; | ||
| const { form } = mappedProps; | ||
| const fieldNames = new Set<string>(); | ||
| const getFieldDecorator = form.getFieldDecorator; | ||
|
|
||
| form.getFieldDecorator = (name: string, fieldOptions?: GetFieldDecoratorOptions) => { | ||
| const fieldName = String(name); | ||
| const duplicated = fieldNames.has(fieldName); | ||
| fieldNames.add(fieldName); | ||
|
|
||
| return getFieldDecorator( | ||
| name, | ||
| duplicated ? { preserve: true, ...fieldOptions } : fieldOptions, | ||
| ); | ||
| }; | ||
|
|
||
| return mappedProps; | ||
| }, |
There was a problem hiding this comment.
Directly mutating form.getFieldDecorator on every render causes a critical issue: since form is typically the same persistent object instance across renders, form.getFieldDecorator will be wrapped repeatedly on every render. This leads to an infinite recursion chain (stack overflow) and severe memory leaks when getFieldDecorator is called on subsequent renders.
To prevent this, we should cache the original getFieldDecorator function on the form instance (e.g., using a private property like __originalGetFieldDecorator__) and always wrap that original function instead of the previously wrapped one.
mapProps(props: any) {
const mappedProps = mapProps ? mapProps.call(this, props) : props;
const { form } = mappedProps;
const fieldNames = new Set<string>();
if (!form.__originalGetFieldDecorator__) {
form.__originalGetFieldDecorator__ = form.getFieldDecorator;
}
form.getFieldDecorator = (name: string, fieldOptions?: GetFieldDecoratorOptions) => {
const fieldName = String(name);
const duplicated = fieldNames.has(fieldName);
fieldNames.add(fieldName);
return form.__originalGetFieldDecorator__(
name,
duplicated ? { preserve: true, ...fieldOptions } : fieldOptions,
);
};
return mappedProps;
},
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 `@src/form/Form.tsx`:
- Around line 309-312: The current call to getFieldDecorator uses { preserve:
true, ...fieldOptions } so a user can override preserve by setting preserve:
false in fieldOptions; if you want to force preserved values for duplicated
fields (per PR description) change the merge order so preserve is applied after
user options (i.e., ensure preserve: true wins when duplicated is true) by
applying the preserve flag after spreading fieldOptions in the duplicated branch
(identify getFieldDecorator, duplicated, and fieldOptions to update); if user
opt-out via fieldOptions should be allowed, leave as-is.
🪄 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: 0daf9069-d607-42af-b2f2-9ac9702d427b
📒 Files selected for processing (2)
src/form/Form.tsxtests/Form.test.tsx
ff70d68 to
7e5fdf9
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #174 +/- ##
===========================================
+ Coverage 43.46% 73.96% +30.49%
===========================================
Files 21 21
Lines 398 411 +13
Branches 101 103 +2
===========================================
+ Hits 173 304 +131
+ Misses 225 107 -118 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
判断了如果同一次渲染里出现重复字段名,会自动给重复字段加上
preserve: true,避免字段值被清掉。Fixes ant-design/ant-design#58273
Summary by CodeRabbit
Bug Fixes
Tests
Chores
@types/minimatch版本覆盖。