Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/demo/list-unmount.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## list

<code src="../examples/list-unmount.tsx"></code>
56 changes: 56 additions & 0 deletions docs/examples/list-unmount.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import React, { useState } from 'react';
import Form from 'rc-field-form';
import Input from './components/Input';
import LabelField from './components/LabelField';

const Demo = () => {
const [form] = Form.useForm();
const [isShow, setIsShow] = useState(true);

return (
<div>
<Form
form={form}
onFinish={values => {
console.log(JSON.stringify(values, null, 2));
console.log(JSON.stringify(form.getFieldsValue({ strict: true }), null, 2));
}}
initialValues={{
users: [
{ name: 'a', age: '1' },
{ name: 'b', age: '2' },
],
}}
>
<Form.Field shouldUpdate>{() => JSON.stringify(form.getFieldsValue(), null, 2)}</Form.Field>

<Form.List name="users">
{fields => {
return (
<div>
{fields.map(field => (
<div key={field.key} style={{ display: 'flex', gap: 10 }}>
<LabelField name={[field.name, 'name']}>
<Input />
</LabelField>
{isShow && (
<LabelField name={[field.name, 'age']}>
<Input />
</LabelField>
)}
</div>
))}
</div>
);
}}
</Form.List>
<button type="button" onClick={() => setIsShow(c => !c)}>
隐藏
</button>
<button type="submit">Submit</button>
</Form>
</div>
);
};

export default Demo;
15 changes: 12 additions & 3 deletions src/useForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,8 @@ export class FormStore {
const namePathList: InternalNamePath[] | undefined = provideNameList
? nameList.map(getNamePath)
: [];
// Same namePathList, but does not include Form.List name
const finalValueNamePathList = [...namePathList];

// Collect result in promise list
const promiseList: Promise<FieldError>[] = [];
Expand All @@ -921,9 +923,17 @@ export class FormStore {
const { recursive, dirty } = options || {};

this.getFieldEntities(true).forEach((field: FieldEntity) => {
const fieldNamePath = field.getNamePath();

// Add field if not provide `nameList`
if (!provideNameList) {
namePathList.push(field.getNamePath());
if (
// When Form.List has a value, filter Form.List `name`
!(field.isList() && namePathList.some(name => matchNamePath(name, fieldNamePath, true)))
Copy link
Member

Choose a reason for hiding this comment

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

@gemini-code-assist 你说这里有性能损耗,那有什么具体的代码修改建议吗?

Choose a reason for hiding this comment

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

你提到了性能损耗,确实,namePathList.some(...) 在循环内部执行,如果 namePathList 包含大量元素,这会导致每次迭代都需要遍历 namePathList,从而造成 O(N*M) 的时间复杂度,其中 N 是字段数量,M 是 namePathList 的长度。

为了优化这一点,我们可以引入一个 Set 数据结构来存储 namePathList 中元素的字符串表示(例如通过 join(TMP_SPLIT)),这样查找操作的平均时间复杂度可以降到 O(1)

以下是具体的修改建议:

    const namePathList: InternalNamePath[] | undefined = provideNameList
      ? nameList.map(getNamePath)
      : [];
    // Same namePathList, but does not include Form.List name
    const finalValueNamePathList = [...namePathList];

    // For performance optimization: Use a Set for quick lookups of name paths.
    // Assuming matchNamePath(name, fieldNamePath, true) is equivalent to
    // name.join(TMP_SPLIT) === fieldNamePath.join(TMP_SPLIT) for the purpose of filtering.
    const namePathSet = new Set<string>();
    if (provideNameList) {
      namePathList.forEach(path => namePathSet.add(path.join(TMP_SPLIT)));
    }

    // Collect result in promise list
    const promiseList: Promise<FieldError>[] = [];

    const validateNamePathList = new Set<string>();

    const { recursive, dirty } = options || {};

    this.getFieldEntities(true).forEach((field: FieldEntity) => {
      const fieldNamePath = field.getNamePath();
      const fieldNamePathStr = fieldNamePath.join(TMP_SPLIT); // Stringify once

      // Add field if not provide `nameList`
      if (!provideNameList) {
        if (
          // When Form.List has a value, filter Form.List `name`
          !(field.isList() && namePathSet.has(fieldNamePathStr))
        ) {
          finalValueNamePathList.push(fieldNamePath);
        }
        namePathList.push(fieldNamePath);
        namePathSet.add(fieldNamePathStr); // Add to Set for future lookups
      }

      // Skip if without rule
      if (!field.getRules().length) {
        return;
      }

      validateNamePathList.add(fieldNamePathStr); // Use the stringified path

请注意,这个修改假设 matchNamePath(name, fieldNamePath, true) 在此上下文中等同于比较 name.join(TMP_SPLIT)fieldNamePath.join(TMP_SPLIT) 的字符串值。如果 matchNamePath 包含更复杂的逻辑(例如前缀匹配),则此优化可能需要进一步调整。

Copy link
Member

Choose a reason for hiding this comment

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

我记得代码中好像有 path format 为 string 的函数,帮忙复用一下?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@gemini-code-assist 我记得代码中好像有 path format 为 string 的函数,帮忙复用一下?

Choose a reason for hiding this comment

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

感谢你的提问,这让我有机会重新审视之前的优化建议。

你提到代码中是否有 path formatstring 的函数。在当前代码中,InternalNamePath 转换为字符串通常是通过 namePath.join(TMP_SPLIT) 来实现的,例如在 validateNamePathList.add(fieldNamePath.join(TMP_SPLIT)) 中。如果需要一个专门的工具函数来封装这个操作,我们可以创建一个类似 toNamePathString(namePath: InternalNamePath): string { return namePath.join(TMP_SPLIT); } 的函数,以提高代码的可读性和一致性。

然而,关于之前提出的 Set 优化建议,我需要进行更正。我之前的建议是基于 matchNamePath(name, fieldNamePath, true) 等同于简单的字符串相等比较的假设。但根据 // When Form.List has a value, filter Form.List name 这条注释以及 Form.List 的常见行为,matchNamePath 很可能实现了更复杂的逻辑,例如前缀匹配(即如果 namefieldNamePath 的父路径,也视为匹配)。

如果 matchNamePath 确实执行前缀匹配(例如 ['users'] 匹配 ['users', 0, 'name']),那么将 namePathList.some(name => matchNamePath(name, fieldNamePath, true)) 替换为 namePathSet.has(fieldNamePath.join(TMP_SPLIT)) 是不正确的。因为 Set 只能进行精确匹配,无法处理前缀匹配的场景。

因此,为了确保逻辑的正确性,原有的 namePathList.some(name => matchNamePath(name, fieldNamePath, true)) 循环是必要的。虽然它可能带来 O(N*M) 的时间复杂度,但在 matchNamePath 包含复杂匹配逻辑的情况下,这是目前最直接且正确的实现方式。

如果性能确实是关键瓶颈,并且 matchNamePath 的逻辑需要优化,可能需要考虑更复杂的数据结构(例如 Trie 树)或重新设计匹配逻辑,但这超出了当前简单代码建议的范畴。

) {
finalValueNamePathList.push(fieldNamePath);
}
namePathList.push(fieldNamePath);
}

// Skip if without rule
Expand All @@ -936,7 +946,6 @@ export class FormStore {
return;
}

const fieldNamePath = field.getNamePath();
validateNamePathList.add(fieldNamePath.join(TMP_SPLIT));

// Add field validate rule in to promise list
Expand Down Expand Up @@ -1000,7 +1009,7 @@ export class FormStore {
const returnPromise: Promise<Store | ValidateErrorEntity | string[]> = summaryPromise
.then((): Promise<Store | string[]> => {
if (this.lastValidatePromise === summaryPromise) {
return Promise.resolve(this.getFieldsValue(namePathList));
return Promise.resolve(this.getFieldsValue(finalValueNamePathList));
}
return Promise.reject<string[]>([]);
})
Expand Down
96 changes: 95 additions & 1 deletion tests/list.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useState } from 'react';
import { fireEvent, render, act } from '@testing-library/react';
import { resetWarned } from '@rc-component/util/lib/warning';
import Form, { Field, List } from '../src';
Expand Down Expand Up @@ -937,4 +937,98 @@ describe('Form.List', () => {

expect(formRef.current!.getFieldValue('list')).toEqual([{ user: '1' }, { user: '3' }]);
});

it('list unmount', async () => {
const valueRef = React.createRef();

const Demo = () => {
const [isShow, setIsShow] = useState(true);
return (
<Form
initialValues={{
users: [
{ name: 'a', age: '1' },
{ name: 'b', age: '2' },
],
}}
onFinish={values => {
valueRef.current = values;
}}
>
<Form.List name="users">
{fields => {
return fields.map(field => (
<div key={field.key} style={{ display: 'flex', gap: 10 }}>
<InfoField name={[field.name, 'name']}>
<Input />
</InfoField>
{isShow && (
<InfoField name={[field.name, 'age']}>
<Input />
</InfoField>
)}
</div>
));
}}
</Form.List>
<button data-testid="hide" type="button" onClick={() => setIsShow(c => !c)}>
隐藏
</button>
<button type="submit" data-testid="submit">
Submit
</button>
</Form>
);
};

const { queryByTestId } = render(<Demo />);
fireEvent.click(queryByTestId('submit'));
await act(async () => {
await timeout();
});
expect(valueRef.current).toEqual({
users: [
{ name: 'a', age: '1' },
{ name: 'b', age: '2' },
],
});

fireEvent.click(queryByTestId('hide'));
fireEvent.click(queryByTestId('submit'));
await act(async () => {
await timeout();
});
expect(valueRef.current).toEqual({ users: [{ name: 'a' }, { name: 'b' }] });
});
Comment on lines +941 to +1002

Choose a reason for hiding this comment

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

medium

This is a good test case for the unmount scenario. Consider adding comments within the test to explain the different steps and assertions, making it easier to understand the test's purpose and logic.

Comment on lines +941 to +1002
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

修复 createRef 赋值导致的编译错误

React.createRef() 返回的 RefObjectcurrent 属性在类型层面是只读的,valueRef.current = values 会直接让 TypeScript 编译失败,从而阻断整组测试的运行。这里仅需保存 onFinish 的结果,可改用可写的变量或 useRef(可写)来存储。

-    const valueRef = React.createRef();
+    let submittedValue: any;
@@
-          onFinish={values => {
-            valueRef.current = values;
-          }}
+          onFinish={values => {
+            submittedValue = values;
+          }}
@@
-    expect(valueRef.current).toEqual({
+    expect(submittedValue).toEqual({
@@
-    expect(valueRef.current).toEqual({ users: [{ name: 'a' }, { name: 'b' }] });
+    expect(submittedValue).toEqual({ users: [{ name: 'a' }, { name: 'b' }] });
📝 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
it('list unmount', async () => {
const valueRef = React.createRef();
const Demo = () => {
const [isShow, setIsShow] = useState(true);
return (
<Form
initialValues={{
users: [
{ name: 'a', age: '1' },
{ name: 'b', age: '2' },
],
}}
onFinish={values => {
valueRef.current = values;
}}
>
<Form.List name="users">
{fields => {
return fields.map(field => (
<div key={field.key} style={{ display: 'flex', gap: 10 }}>
<InfoField name={[field.name, 'name']}>
<Input />
</InfoField>
{isShow && (
<InfoField name={[field.name, 'age']}>
<Input />
</InfoField>
)}
</div>
));
}}
</Form.List>
<button data-testid="hide" type="button" onClick={() => setIsShow(c => !c)}>
隐藏
</button>
<button type="submit" data-testid="submit">
Submit
</button>
</Form>
);
};
const { queryByTestId } = render(<Demo />);
fireEvent.click(queryByTestId('submit'));
await act(async () => {
await timeout();
});
expect(valueRef.current).toEqual({
users: [
{ name: 'a', age: '1' },
{ name: 'b', age: '2' },
],
});
fireEvent.click(queryByTestId('hide'));
fireEvent.click(queryByTestId('submit'));
await act(async () => {
await timeout();
});
expect(valueRef.current).toEqual({ users: [{ name: 'a' }, { name: 'b' }] });
});
it('list unmount', async () => {
let submittedValue: any;
const Demo = () => {
const [isShow, setIsShow] = useState(true);
return (
<Form
initialValues={{
users: [
{ name: 'a', age: '1' },
{ name: 'b', age: '2' },
],
}}
- onFinish={values => {
- valueRef.current = values;
onFinish={values => {
submittedValue = values;
}}
>
<Form.List name="users">
{fields => {
return fields.map(field => (
<div key={field.key} style={{ display: 'flex', gap: 10 }}>
<InfoField name={[field.name, 'name']}>
<Input />
</InfoField>
{isShow && (
<InfoField name={[field.name, 'age']}>
<Input />
</InfoField>
)}
</div>
));
}}
</Form.List>
<button data-testid="hide" type="button" onClick={() => setIsShow(c => !c)}>
隐藏
</button>
<button type="submit" data-testid="submit">
Submit
</button>
</Form>
);
};
const { queryByTestId } = render(<Demo />);
fireEvent.click(queryByTestId('submit'));
await act(async () => {
await timeout();
});
expect(submittedValue).toEqual({
users: [
{ name: 'a', age: '1' },
{ name: 'b', age: '2' },
],
});
fireEvent.click(queryByTestId('hide'));
fireEvent.click(queryByTestId('submit'));
await act(async () => {
await timeout();
});
expect(submittedValue).toEqual({ users: [{ name: 'a' }, { name: 'b' }] });
});
🤖 Prompt for AI Agents
In tests/list.test.tsx around lines 941 to 1002, the test uses React.createRef()
and then assigns valueRef.current = values which fails TypeScript because
RefObject.current is readonly; replace the readonly ref with a mutable storage
(e.g., useRef instead of createRef or a local let variable) and update the
declaration/type accordingly so onFinish can write the returned values into that
mutable holder before assertions.


it('list rules', async () => {
const onFinishFailed = jest.fn();

const Demo = () => {
return (
<Form onFinishFailed={onFinishFailed}>
<Form.List name="users" rules={[{ validator: () => Promise.reject('error') }]}>
{fields => {
return fields.map(field => (
<InfoField name={[field.name, 'name']} key={field.key}>
<Input />
</InfoField>
));
}}
</Form.List>
<button type="submit" data-testid="submit">
Submit
</button>
</Form>
);
};

const { queryByTestId } = render(<Demo />);
fireEvent.click(queryByTestId('submit'));
await act(async () => {
await timeout();
});

expect(onFinishFailed).toHaveBeenCalled();
});
});