Skip to content

Commit 49ad23c

Browse files
zombieJzombieJ
committed
fix: Do not return null when invalidate children
1 parent c65e768 commit 49ad23c

File tree

3 files changed

+141
-34
lines changed

3 files changed

+141
-34
lines changed

‎.prettierrc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"endOfLine": "lf",
3+
"semi": true,
4+
"singleQuote": true,
5+
"tabWidth": 2,
6+
"trailingComma": "all"
7+
}

‎src/Field.tsx‎

Lines changed: 73 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import toChildrenArray from 'rc-util/lib/Children/toArray';
2+
import warning from 'rc-util/lib/warning';
23
import * as React from 'react';
34
import {
45
FieldEntity,
@@ -33,7 +34,11 @@ interface ChildProps {
3334
export interface FieldProps {
3435
children?:
3536
| React.ReactElement
36-
| ((control: ChildProps, meta: Meta, form: FormInstance) => React.ReactNode);
37+
| ((
38+
control: ChildProps,
39+
meta: Meta,
40+
form: FormInstance,
41+
) => React.ReactNode);
3742
/**
3843
* Set up `dependencies` field.
3944
* When dependencies field update and current field is touched,
@@ -42,11 +47,19 @@ export interface FieldProps {
4247
dependencies?: NamePath[];
4348
getValueFromEvent?: (...args: EventArgs) => StoreValue;
4449
name?: NamePath;
45-
normalize?: (value: StoreValue, prevValue: StoreValue, allValues: Store) => StoreValue;
50+
normalize?: (
51+
value: StoreValue,
52+
prevValue: StoreValue,
53+
allValues: Store,
54+
) => StoreValue;
4655
rules?: Rule[];
4756
shouldUpdate?:
4857
| true
49-
| ((prevValues: Store, nextValues: Store, info: { source?: string }) => boolean);
58+
| ((
59+
prevValues: Store,
60+
nextValues: Store,
61+
info: { source?: string },
62+
) => boolean);
5063
trigger?: string;
5164
validateTrigger?: string | string[] | false;
5265
valuePropName?: string;
@@ -58,7 +71,8 @@ export interface FieldState {
5871
}
5972

6073
// We use Class instead of Hooks here since it will cost much code by using Hooks.
61-
class Field extends React.Component<FieldProps, FieldState> implements FieldEntity {
74+
class Field extends React.Component<FieldProps, FieldState>
75+
implements FieldEntity {
6276
public static contextType = FieldContext;
6377

6478
public static defaultProps = {
@@ -164,10 +178,15 @@ class Field extends React.Component<FieldProps, FieldState> implements FieldEnti
164178
const prevValue = this.getValue(prevStore);
165179
const curValue = this.getValue();
166180

167-
const namePathMatch = namePathList && containsNamePath(namePathList, namePath);
181+
const namePathMatch =
182+
namePathList && containsNamePath(namePathList, namePath);
168183

169184
// `setFieldsValue` is a quick access to update related status
170-
if (info.type === 'valueUpdate' && info.source === 'external' && prevValue !== curValue) {
185+
if (
186+
info.type === 'valueUpdate' &&
187+
info.source === 'external' &&
188+
prevValue !== curValue
189+
) {
171190
this.touched = true;
172191
this.validatePromise = null;
173192
this.errors = [];
@@ -216,7 +235,9 @@ class Field extends React.Component<FieldProps, FieldState> implements FieldEnti
216235
const dependencyList = dependencies.map(getNamePath);
217236
if (
218237
namePathMatch ||
219-
dependencyList.some(dependency => containsNamePath(info.relatedFields, dependency))
238+
dependencyList.some(dependency =>
239+
containsNamePath(info.relatedFields, dependency),
240+
)
220241
) {
221242
this.reRender();
222243
return;
@@ -237,7 +258,11 @@ class Field extends React.Component<FieldProps, FieldState> implements FieldEnti
237258
containsNamePath(namePathList, getNamePath(dependency)),
238259
) ||
239260
(typeof shouldUpdate === 'function'
240-
? shouldUpdate(prevStore, values, 'source' in info ? { source: info.source } : {})
261+
? shouldUpdate(
262+
prevStore,
263+
values,
264+
'source' in info ? { source: info.source } : {},
265+
)
241266
: prevValue !== curValue)
242267
) {
243268
this.reRender();
@@ -267,7 +292,12 @@ class Field extends React.Component<FieldProps, FieldState> implements FieldEnti
267292
});
268293
}
269294

270-
const promise = validateRules(namePath, this.getValue(), filteredRules, options);
295+
const promise = validateRules(
296+
namePath,
297+
this.getValue(),
298+
filteredRules,
299+
options,
300+
);
271301
this.validatePromise = promise;
272302
this.errors = [];
273303

@@ -309,22 +339,28 @@ class Field extends React.Component<FieldProps, FieldState> implements FieldEnti
309339
public getOnlyChild = (
310340
children:
311341
| React.ReactNode
312-
| ((control: ChildProps, meta: Meta, context: FormInstance) => React.ReactNode),
313-
): { child: React.ReactElement | null; isFunction: boolean } => {
342+
| ((
343+
control: ChildProps,
344+
meta: Meta,
345+
context: FormInstance,
346+
) => React.ReactNode),
347+
): { child: React.ReactNode | null; isFunction: boolean } => {
314348
// Support render props
315349
if (typeof children === 'function') {
316350
const meta = this.getMeta();
317351

318352
return {
319-
...this.getOnlyChild(children(this.getControlled(), meta, this.context)),
353+
...this.getOnlyChild(
354+
children(this.getControlled(), meta, this.context),
355+
),
320356
isFunction: true,
321357
};
322358
}
323359

324360
// Filed element only
325361
const childList = toChildrenArray(children);
326362
if (childList.length !== 1 || !React.isValidElement(childList[0])) {
327-
return { child: null, isFunction: false };
363+
return { child: childList, isFunction: false };
328364
}
329365

330366
return { child: childList[0], isFunction: false };
@@ -338,9 +374,18 @@ class Field extends React.Component<FieldProps, FieldState> implements FieldEnti
338374
};
339375

340376
public getControlled = (childProps: ChildProps = {}) => {
341-
const { trigger, validateTrigger, getValueFromEvent, normalize, valuePropName } = this.props;
377+
const {
378+
trigger,
379+
validateTrigger,
380+
getValueFromEvent,
381+
normalize,
382+
valuePropName,
383+
} = this.props;
342384
const namePath = this.getNamePath();
343-
const { getInternalHooks, getFieldsValue }: InternalFormInstance = this.context;
385+
const {
386+
getInternalHooks,
387+
getFieldsValue,
388+
}: InternalFormInstance = this.context;
344389
const { dispatch } = getInternalHooks(HOOK_MARK);
345390
const value = this.getValue();
346391

@@ -412,19 +457,24 @@ class Field extends React.Component<FieldProps, FieldState> implements FieldEnti
412457
const { children } = this.props;
413458

414459
const { child, isFunction } = this.getOnlyChild(children);
415-
if (!child) {
416-
// Return origin `children` if is not a function
417-
return isFunction ? child : children;
418-
}
419460

420461
// Not need to `cloneElement` since user can handle this in render function self
421-
const returnChildNode = isFunction
422-
? child
423-
: React.cloneElement(child, this.getControlled(child.props));
462+
let returnChildNode: React.ReactNode;
463+
if (isFunction) {
464+
returnChildNode = child;
465+
} else if (React.isValidElement(child)) {
466+
returnChildNode = React.cloneElement(
467+
child as React.ReactElement,
468+
this.getControlled((child as React.ReactElement).props),
469+
);
470+
} else {
471+
warning(!child, '`children` of Field is not validate ReactElement.');
472+
returnChildNode = child;
473+
}
424474

425475
// Force render a new component to reset all the data
426476
if (reset) {
427-
return React.createElement(() => returnChildNode);
477+
return React.createElement(() => <>{returnChildNode}</>);
428478
}
429479

430480
return returnChildNode;

‎tests/index.test.js‎

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React from 'react';
22
import { mount } from 'enzyme';
3+
import { resetWarned } from 'rc-util/lib/warning';
34
import Form, { Field } from '../src';
45
import InfoField, { Input } from './common/InfoField';
56
import { changeValue, getField, matchError } from './common';
@@ -41,7 +42,9 @@ describe('Form.Basic', () => {
4142

4243
it('use component', () => {
4344
const MyComponent = ({ children }) => <div>{children}</div>;
44-
const wrapper = mount(<Form component={MyComponent}>{renderContent()}</Form>);
45+
const wrapper = mount(
46+
<Form component={MyComponent}>{renderContent()}</Form>,
47+
);
4548
expect(wrapper.find('form').length).toBe(0);
4649
expect(wrapper.find(MyComponent).length).toBe(1);
4750
expect(wrapper.find('input').length).toBe(2);
@@ -108,7 +111,11 @@ describe('Form.Basic', () => {
108111
form = instance;
109112
}}
110113
>
111-
<Field name="username" rules={[{ required: true }]} onReset={onReset}>
114+
<Field
115+
name="username"
116+
rules={[{ required: true }]}
117+
onReset={onReset}
118+
>
112119
<Input />
113120
</Field>
114121
</Form>
@@ -130,7 +137,9 @@ describe('Form.Basic', () => {
130137

131138
await changeValue(getField(wrapper, 'username'), '');
132139
expect(form.getFieldValue('username')).toEqual('');
133-
expect(form.getFieldError('username')).toEqual(["'username' is required"]);
140+
expect(form.getFieldError('username')).toEqual([
141+
"'username' is required",
142+
]);
134143
expect(form.isFieldTouched('username')).toBeTruthy();
135144

136145
expect(onReset).not.toHaveBeenCalled();
@@ -174,7 +183,9 @@ describe('Form.Basic', () => {
174183
expect(form.getFieldError('username')).toEqual([]);
175184
expect(form.isFieldTouched('username')).toBeFalsy();
176185
expect(form.getFieldValue('password')).toEqual('');
177-
expect(form.getFieldError('password')).toEqual(["'password' is required"]);
186+
expect(form.getFieldError('password')).toEqual([
187+
"'password' is required",
188+
]);
178189
expect(form.isFieldTouched('password')).toBeTruthy();
179190
});
180191
});
@@ -313,8 +324,13 @@ describe('Form.Basic', () => {
313324
);
314325

315326
await changeValue(getField(wrapper), 'Bamboo');
316-
expect(onValuesChange).toHaveBeenCalledWith({ username: 'Bamboo' }, { username: 'Bamboo' });
317-
expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ target: { value: 'Bamboo' } }));
327+
expect(onValuesChange).toHaveBeenCalledWith(
328+
{ username: 'Bamboo' },
329+
{ username: 'Bamboo' },
330+
);
331+
expect(onChange).toHaveBeenCalledWith(
332+
expect.objectContaining({ target: { value: 'Bamboo' } }),
333+
);
318334
});
319335

320336
it('submit', async () => {
@@ -393,11 +409,15 @@ describe('Form.Basic', () => {
393409
</div>,
394410
);
395411

396-
wrapper.find('input[type="checkbox"]').simulate('change', { target: { checked: true } });
412+
wrapper
413+
.find('input[type="checkbox"]')
414+
.simulate('change', { target: { checked: true } });
397415
await timeout();
398416
expect(form.getFieldsValue()).toEqual({ check: true });
399417

400-
wrapper.find('input[type="checkbox"]').simulate('change', { target: { checked: false } });
418+
wrapper
419+
.find('input[type="checkbox"]')
420+
.simulate('change', { target: { checked: false } });
401421
await timeout();
402422
expect(form.getFieldsValue()).toEqual({ check: false });
403423
});
@@ -417,7 +437,8 @@ describe('Form.Basic', () => {
417437
<Field shouldUpdate>
418438
{(_, __, { getFieldsError, isFieldsTouched }) => {
419439
isAllTouched = isFieldsTouched(true);
420-
hasError = getFieldsError().filter(({ errors }) => errors.length).length;
440+
hasError = getFieldsError().filter(({ errors }) => errors.length)
441+
.length;
421442

422443
return null;
423444
}}
@@ -458,7 +479,14 @@ describe('Form.Basic', () => {
458479
</div>,
459480
);
460481

461-
form.setFields([{ name: 'username', touched: false, validating: true, errors: ['Set It!'] }]);
482+
form.setFields([
483+
{
484+
name: 'username',
485+
touched: false,
486+
validating: true,
487+
errors: ['Set It!'],
488+
},
489+
]);
462490
wrapper.update();
463491

464492
matchError(wrapper, 'Set It!');
@@ -504,7 +532,10 @@ describe('Form.Basic', () => {
504532
form = instance;
505533
}}
506534
>
507-
<Field name="normal" rules={[{ validator: () => new Promise(() => {}) }]}>
535+
<Field
536+
name="normal"
537+
rules={[{ validator: () => new Promise(() => {}) }]}
538+
>
508539
{(control, meta) => {
509540
currentMeta = meta;
510541
return <Input {...control} />;
@@ -548,4 +579,23 @@ describe('Form.Basic', () => {
548579
expect(form.getFieldError('normal')).toEqual([]);
549580
expect(currentMeta.validating).toBeFalsy();
550581
});
582+
583+
it('warning if invalidate element', () => {
584+
resetWarned();
585+
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
586+
mount(
587+
<div>
588+
<Form>
589+
<Field>
590+
<h1 key="1">Light</h1>
591+
<h2 key="2">Bamboo</h2>
592+
</Field>
593+
</Form>
594+
</div>,
595+
);
596+
expect(errorSpy).toHaveBeenCalledWith(
597+
'Warning: `children` of Field is not validate ReactElement.',
598+
);
599+
errorSpy.mockRestore();
600+
});
551601
});

0 commit comments

Comments
 (0)