Skip to content

fix: preserve duplicated legacy form fields#174

Open
duqigit wants to merge 1 commit into
ant-design:masterfrom
duqigit:fix-compatible-form-duplicate-name-58273
Open

fix: preserve duplicated legacy form fields#174
duqigit wants to merge 1 commit into
ant-design:masterfrom
duqigit:fix-compatible-form-duplicate-name-58273

Conversation

@duqigit

@duqigit duqigit commented Jun 11, 2026

Copy link
Copy Markdown

判断了如果同一次渲染里出现重复字段名,会自动给重复字段加上 preserve: true,避免字段值被清掉。
Fixes ant-design/ant-design#58273

Summary by CodeRabbit

  • Bug Fixes

    • 修复表单重复注册同名字段时的行为:首次注册正常,重复注册时保留已有值且不被后续字段更新覆盖。
  • Tests

    • 新增/更新用例:验证重复字段在交互修改后值的一致性与保留行为。
    • 测试环境增强:添加 window.matchMedia 的全局 mock 以稳定媒体查询相关测试。
  • Chores

    • 包配置更新:在 package.json 中锁定 @types/minimatch 版本覆盖。

@vercel

vercel Bot commented Jun 11, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the pro-components Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 27e7ec75-2c62-401e-b814-f3b0985461b7

📥 Commits

Reviewing files that changed from the base of the PR and between ff70d68 and 7e5fdf9.

📒 Files selected for processing (4)
  • package.json
  • src/form/Form.tsx
  • tests/Form.test.tsx
  • tests/setupAfterEnv.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • tests/Form.test.tsx
  • package.json
  • src/form/Form.tsx

📝 Walkthrough

Walkthrough

此 PR 在 create 中包装 options.mapProps,重写 mappedProps.form.getFieldDecorator,用 Set 跟踪已装饰字段名;首次注册使用原 fieldOptions,重复注册时合并 {preserve: true}。新增测试验证重复字段值同步且不被无关字段更新影响;并在 package.json 中新增 @types/minimatch 覆盖项。

Changes

重复字段装饰器支持

Layer / File(s) Summary
字段装饰器注册包装
src/form/Form.tsx
create 函数中使用 Set 追踪已注册的字段名;首次注册字段时使用原始 fieldOptions;重复注册时强制合并 preserve: true 到选项中,并调用原装饰器。
重复字段行为验证
tests/Form.test.tsx tests/setupAfterEnv.ts
导入 Input 组件和 fireEvent 用于交互模拟。新增测试用例验证同一字段名在多个 Form.Item 中重复出现时,修改其中一个输入会同步值到另一个,且修改其他字段不会影响重复字段的值。测试环境添加 window.matchMedia stub。
package 覆盖项更新
package.json
overrides 中新增 @types/minimatch 的版本覆盖 5.1.2

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Poem

小兔检视表单忙,
重名字段不再慌,
preserve 一并写入,
两处值同心相将,
测试绿灯跳一趟 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题清晰准确地总结了变更的核心目的:修复并保留重复的旧式表单字段,与代码变更直接相关。
Linked Issues check ✅ Passed 代码变更完全满足问题 #58273 的要求:通过在重复字段上设置 preserve: true 来保留字段值,并包含相应的测试验证。
Out of Scope Changes check ✅ Passed 大部分变更都与修复重复字段问题直接相关。package.json 中的 @types/minimatch 覆盖和 setupAfterEnv.ts 中的 matchMedia mock 似乎是为了满足测试环境的依赖需求。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/form/Form.tsx
Comment on lines +298 to +316
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;
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

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;
    },

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 8a846ba and 9b41a08.

📒 Files selected for processing (2)
  • src/form/Form.tsx
  • tests/Form.test.tsx

Comment thread src/form/Form.tsx Outdated
@duqigit duqigit force-pushed the fix-compatible-form-duplicate-name-58273 branch from ff70d68 to 7e5fdf9 Compare June 13, 2026 02:02
@codecov

codecov Bot commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.96%. Comparing base (f56dbe6) to head (7e5fdf9).
⚠️ Report is 1 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@ant-design/compatible Form 结合 antd Input,如果存在同名,则丢失值

1 participant