Skip to content
Open
Show file tree
Hide file tree
Changes from 10 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 @@ -911,6 +911,8 @@ export class FormStore {
? nameList.map(getNamePath)
: [];

const removeListNameStrList: string[] = [];

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

Expand All @@ -921,9 +923,14 @@ 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 (field.isList() && namePathList.some(name => matchNamePath(name, fieldNamePath, true))) {
removeListNameStrList.push(fieldNamePath.toString());
Copy link
Member

Choose a reason for hiding this comment

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

这个逻辑很奇怪,这里 matchNamePath 一次后,下面又从列表里匹配一遍。
应该是提取一个 filledPathList 专门存需要留档的路径出来,避免多轮遍历。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

要用2个 list吗?也行吧,就是 push 的地方要写好几处地方,比较麻烦。

我现在实现的,就是单纯记录 List 这层的 name,然后在提交时候过滤。

}
namePathList.push(fieldNamePath);
Comment on lines 925 to 936

Choose a reason for hiding this comment

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

medium

This logic seems a bit complex. It might be beneficial to add a comment explaining the purpose of this if condition and how it contributes to resolving the issue. Specifically, clarify why you're checking if field.isList() and if namePathList already contains a matching name path, and why this necessitates removing the list name.

}

// Skip if without rule
Expand All @@ -936,7 +943,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 +1006,10 @@ export class FormStore {
const returnPromise: Promise<Store | ValidateErrorEntity | string[]> = summaryPromise
.then((): Promise<Store | string[]> => {
if (this.lastValidatePromise === summaryPromise) {
return Promise.resolve(this.getFieldsValue(namePathList));
const filterListNameList = namePathList.filter(
name => !removeListNameStrList.some(nameStr => nameStr === name.toString()),
);

Choose a reason for hiding this comment

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

medium

It would be helpful to add a comment explaining why removeListNameStrList is being used to filter namePathList. What is the purpose of filtering these names, and what problem does it solve?

return Promise.resolve(this.getFieldsValue(filterListNameList));
}
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();
});
});