From a039b23bcfd36aad2924d9bce50446058ac00353 Mon Sep 17 00:00:00 2001 From: zombiej Date: Thu, 4 Feb 2021 10:36:09 +0800 Subject: [PATCH 1/4] chore: Update comment --- src/useForm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/useForm.ts b/src/useForm.ts index ca5e8f8a..efe2585d 100644 --- a/src/useForm.ts +++ b/src/useForm.ts @@ -539,7 +539,7 @@ export class FormStore { return (isListField?: boolean, preserve?: boolean) => { this.fieldEntities = this.fieldEntities.filter(item => item !== entity); - // Clean up store value if preserve + // Clean up store value if not preserve const mergedPreserve = preserve !== undefined ? preserve : this.preserve; if (mergedPreserve === false && !isListField) { const namePath = entity.getNamePath(); From 8b40c5163d6720f8aad97974e841f237bba7643b Mon Sep 17 00:00:00 2001 From: zombiej Date: Thu, 4 Feb 2021 10:52:02 +0800 Subject: [PATCH 2/4] test: Test driven --- src/Field.tsx | 14 ++-- src/useForm.ts | 3 +- tests/preserve.test.tsx | 164 ++++++++++++++++++++++++++-------------- 3 files changed, 120 insertions(+), 61 deletions(-) diff --git a/src/Field.tsx b/src/Field.tsx index 1f8580f6..f5371563 100644 --- a/src/Field.tsx +++ b/src/Field.tsx @@ -553,11 +553,15 @@ function WrapperField({ name, ...restProps }: FieldProps) key = `_${(namePath || []).join('_')}`; } - if (process.env.NODE_ENV !== 'production') { - warning( - restProps.preserve !== false || !restProps.isListField, - '`preserve` should not apply on Form.List fields.', - ); + // Warning if it's a directly list field. + // We can still support multiple level field preserve. + if ( + process.env.NODE_ENV !== 'production' && + restProps.preserve === false && + restProps.isListField && + namePath.length <= 1 + ) { + warning(false, '`preserve` should not apply on Form.List fields.'); } return ; diff --git a/src/useForm.ts b/src/useForm.ts index efe2585d..17802e9d 100644 --- a/src/useForm.ts +++ b/src/useForm.ts @@ -541,8 +541,9 @@ export class FormStore { // Clean up store value if not preserve const mergedPreserve = preserve !== undefined ? preserve : this.preserve; + const namePath = entity.getNamePath(); + if (mergedPreserve === false && !isListField) { - const namePath = entity.getNamePath(); const defaultValue = getValue(this.initialValues, namePath); if ( diff --git a/tests/preserve.test.tsx b/tests/preserve.test.tsx index dd1e4861..d40ab331 100644 --- a/tests/preserve.test.tsx +++ b/tests/preserve.test.tsx @@ -99,7 +99,7 @@ describe('Form.Preserve', () => { expect(onFinish).toHaveBeenCalledWith({ test: 'light' }); }); - it('form perishable but field !perishable', async () => { + it('form preserve but field !preserve', async () => { const onFinish = jest.fn(); const wrapper = mount( , @@ -117,67 +117,121 @@ describe('Form.Preserve', () => { await matchTest(false, { keep: 233, remove: 666 }); }); - it('form perishable should not crash Form.List', async () => { - let form: FormInstance; + describe('Form.List', () => { + it('form preserve should not crash', async () => { + let form: FormInstance; - const wrapper = mount( -
{ - form = instance; - }} - > - - {(fields, { remove }) => { - return ( -
- {fields.map(field => ( - - - - ))} -
- ); + const wrapper = mount( + { + form = instance; }} - -
, - ); + > + + {(fields, { remove }) => { + return ( +
+ {fields.map(field => ( + + + + ))} +
+ ); + }} + + , + ); - wrapper.find('button').simulate('click'); - wrapper.update(); + wrapper.find('button').simulate('click'); + wrapper.update(); - expect(form.getFieldsValue()).toEqual({ list: ['bamboo', 'little'] }); - }); + expect(form.getFieldsValue()).toEqual({ list: ['bamboo', 'little'] }); + }); - it('warning when Form.List use preserve', () => { - const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); - - mount( -
- - {fields => - fields.map(field => ( - - - - )) - } - -
, - ); + it('warning when Form.List use preserve', () => { + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); - expect(errorSpy).toHaveBeenCalledWith( - 'Warning: `preserve` should not apply on Form.List fields.', - ); + mount( +
+ + {fields => + fields.map(field => ( + + + + )) + } + +
, + ); + + expect(errorSpy).toHaveBeenCalledWith( + 'Warning: `preserve` should not apply on Form.List fields.', + ); + + errorSpy.mockRestore(); + }); + + it('multiple level field can use preserve', async () => { + let form: FormInstance; + + const wrapper = mount( +
{ + form = instance; + }} + > + + {fields => { + return fields.map(field => ( +
+ + + + + {(_, __, { getFieldValue }) => + getFieldValue(['list', field.name, 'type']) === 'light' ? ( + + + + ) : ( + + + + ) + } + +
+ )); + }} + +
, + ); + + // Change type + wrapper + .find('input') + .first() + .simulate('change', { target: { value: 'bamboo' } }); + + wrapper + .find('input') + .last() + .simulate('change', { target: { value: '903' } }); - errorSpy.mockRestore(); + expect(form.getFieldsValue()).toEqual(233); + }); }); it('nest render props should not clean full store', () => { From 8702e765d6a44b4afbe2c2d18c5f1ce5fa84a413 Mon Sep 17 00:00:00 2001 From: zombiej Date: Thu, 4 Feb 2021 16:19:46 +0800 Subject: [PATCH 3/4] fix: preserve can delete nest field --- src/Field.tsx | 10 +++++++--- src/useForm.ts | 6 +++--- tests/preserve.test.tsx | 24 ++++++++++++++++++++---- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/Field.tsx b/src/Field.tsx index f5371563..c2d211c8 100644 --- a/src/Field.tsx +++ b/src/Field.tsx @@ -107,7 +107,11 @@ class Field extends React.Component implements F resetCount: 0, }; - private cancelRegisterFunc: (isListField?: boolean, preserve?: boolean) => void | null = null; + private cancelRegisterFunc: ( + isListField?: boolean, + preserve?: boolean, + namePath?: InternalNamePath, + ) => void | null = null; private mounted = false; @@ -162,10 +166,10 @@ class Field extends React.Component implements F } public cancelRegister = () => { - const { preserve, isListField } = this.props; + const { preserve, isListField, name } = this.props; if (this.cancelRegisterFunc) { - this.cancelRegisterFunc(isListField, preserve); + this.cancelRegisterFunc(isListField, preserve, getNamePath(name)); } this.cancelRegisterFunc = null; }; diff --git a/src/useForm.ts b/src/useForm.ts index 17802e9d..1ce9af65 100644 --- a/src/useForm.ts +++ b/src/useForm.ts @@ -536,14 +536,14 @@ export class FormStore { } // un-register field callback - return (isListField?: boolean, preserve?: boolean) => { + return (isListField?: boolean, preserve?: boolean, subNamePath: InternalNamePath = []) => { this.fieldEntities = this.fieldEntities.filter(item => item !== entity); // Clean up store value if not preserve const mergedPreserve = preserve !== undefined ? preserve : this.preserve; - const namePath = entity.getNamePath(); - if (mergedPreserve === false && !isListField) { + if (mergedPreserve === false && (!isListField || subNamePath.length > 1)) { + const namePath = entity.getNamePath(); const defaultValue = getValue(this.initialValues, namePath); if ( diff --git a/tests/preserve.test.tsx b/tests/preserve.test.tsx index d40ab331..d0b65e1a 100644 --- a/tests/preserve.test.tsx +++ b/tests/preserve.test.tsx @@ -186,7 +186,7 @@ describe('Form.Preserve', () => { const wrapper = mount(
{ form = instance; @@ -202,11 +202,21 @@ describe('Form.Preserve', () => { {(_, __, { getFieldValue }) => getFieldValue(['list', field.name, 'type']) === 'light' ? ( - + ) : ( - + ) @@ -219,6 +229,12 @@ describe('Form.Preserve', () => { , ); + // Change value + wrapper + .find('input') + .last() + .simulate('change', { target: { value: '1128' } }); + // Change type wrapper .find('input') @@ -230,7 +246,7 @@ describe('Form.Preserve', () => { .last() .simulate('change', { target: { value: '903' } }); - expect(form.getFieldsValue()).toEqual(233); + expect(form.getFieldsValue()).toEqual({ list: [{ type: 'bamboo', bamboo: '903' }] }); }); }); From 931cb8ce00cdced9bcbbe319878442505c9d18c8 Mon Sep 17 00:00:00 2001 From: zombiej Date: Thu, 4 Feb 2021 18:17:54 +0800 Subject: [PATCH 4/4] support omit value --- package.json | 2 +- src/useForm.ts | 5 +- src/utils/valueUtil.ts | 9 ++- tests/preserve.test.tsx | 120 ++++++++++++++++++++++++++-------------- 4 files changed, 89 insertions(+), 47 deletions(-) diff --git a/package.json b/package.json index 5864777b..37bc8425 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "dependencies": { "@babel/runtime": "^7.8.4", "async-validator": "^3.0.3", - "rc-util": "^5.0.0" + "rc-util": "^5.8.0" }, "devDependencies": { "@types/enzyme": "^3.10.5", diff --git a/src/useForm.ts b/src/useForm.ts index 1ce9af65..86e959fe 100644 --- a/src/useForm.ts +++ b/src/useForm.ts @@ -544,7 +544,8 @@ export class FormStore { if (mergedPreserve === false && (!isListField || subNamePath.length > 1)) { const namePath = entity.getNamePath(); - const defaultValue = getValue(this.initialValues, namePath); + + const defaultValue = isListField ? undefined : getValue(this.initialValues, namePath); if ( namePath.length && @@ -555,7 +556,7 @@ export class FormStore { !matchNamePath(field.getNamePath(), namePath), ) ) { - this.store = setValue(this.store, namePath, defaultValue); + this.store = setValue(this.store, namePath, defaultValue, true); } } }; diff --git a/src/utils/valueUtil.ts b/src/utils/valueUtil.ts index 851699eb..7f4d0bfa 100644 --- a/src/utils/valueUtil.ts +++ b/src/utils/valueUtil.ts @@ -19,8 +19,13 @@ export function getValue(store: Store, namePath: InternalNamePath) { return value; } -export function setValue(store: Store, namePath: InternalNamePath, value: StoreValue): Store { - const newStore = set(store, namePath, value); +export function setValue( + store: Store, + namePath: InternalNamePath, + value: StoreValue, + removeIfUndefined = false, +): Store { + const newStore = set(store, namePath, value, removeIfUndefined); return newStore; } diff --git a/tests/preserve.test.tsx b/tests/preserve.test.tsx index d0b65e1a..c496a2f5 100644 --- a/tests/preserve.test.tsx +++ b/tests/preserve.test.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { mount } from 'enzyme'; import Form, { FormInstance } from '../src'; -import InfoField from './common/InfoField'; +import InfoField, { Input } from './common/InfoField'; import timeout from './common/timeout'; describe('Form.Preserve', () => { @@ -159,17 +159,32 @@ describe('Form.Preserve', () => { it('warning when Form.List use preserve', () => { const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + let form: FormInstance; - mount( -
+ const wrapper = mount( + { + form = instance; + }} + initialValues={{ list: ['bamboo'] }} + > - {fields => - fields.map(field => ( - - - - )) - } + {(fields, { remove }) => ( + <> + {fields.map(field => ( + + + + ))} + + + )}
, ); @@ -179,6 +194,10 @@ describe('Form.Preserve', () => { ); errorSpy.mockRestore(); + + // Remove should not work + wrapper.find('button').simulate('click'); + expect(form.getFieldsValue()).toEqual({ list: [] }); }); it('multiple level field can use preserve', async () => { @@ -193,43 +212,54 @@ describe('Form.Preserve', () => { }} > - {fields => { - return fields.map(field => ( -
- - - - - {(_, __, { getFieldValue }) => - getFieldValue(['list', field.name, 'type']) === 'light' ? ( - - - - ) : ( - - - - ) - } - -
- )); + {(fields, { remove }) => { + return ( + <> + {fields.map(field => ( +
+ + + + + {(_, __, { getFieldValue }) => + getFieldValue(['list', field.name, 'type']) === 'light' ? ( + + + + ) : ( + + + + ) + } + +
+ ))} + + + ); }} , ); - // Change value + // Change light value wrapper .find('input') .last() @@ -241,12 +271,18 @@ describe('Form.Preserve', () => { .first() .simulate('change', { target: { value: 'bamboo' } }); + // Change bamboo value wrapper .find('input') .last() .simulate('change', { target: { value: '903' } }); expect(form.getFieldsValue()).toEqual({ list: [{ type: 'bamboo', bamboo: '903' }] }); + + // ============== Remove Test ============== + // Remove field + wrapper.find('button').simulate('click'); + expect(form.getFieldsValue()).toEqual({ list: [] }); }); });