[Form lib] Fix issue where serializer on fields are called on every change (#75166)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Sébastien Loix 2020-08-18 16:05:10 +02:00 committed by GitHub
parent 0e28dae410
commit 196cb7f865
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 200 additions and 43 deletions

View file

@ -17,9 +17,10 @@
* under the License.
*/
import React, { useEffect } from 'react';
import { act } from 'react-dom/test-utils';
import { registerTestBed } from '../shared_imports';
import { OnUpdateHandler } from '../types';
import { registerTestBed, TestBed } from '../shared_imports';
import { FormHook, OnUpdateHandler, FieldConfig } from '../types';
import { useForm } from '../hooks/use_form';
import { Form } from './form';
import { UseField } from './use_field';
@ -62,4 +63,91 @@ describe('<UseField />', () => {
lastName: 'Snow',
});
});
describe('serializer(), deserializer(), formatter()', () => {
interface MyForm {
name: string;
}
const serializer = jest.fn();
const deserializer = jest.fn();
const formatter = jest.fn();
const fieldConfig: FieldConfig = {
defaultValue: '',
serializer,
deserializer,
formatters: [formatter],
};
let formHook: FormHook<MyForm> | null = null;
beforeEach(() => {
formHook = null;
serializer.mockReset().mockImplementation((value) => `${value}-serialized`);
deserializer.mockReset().mockImplementation((value) => `${value}-deserialized`);
formatter.mockReset().mockImplementation((value: string) => value.toUpperCase());
});
const onFormHook = (_form: FormHook<MyForm>) => {
formHook = _form;
};
const TestComp = ({ onForm }: { onForm: (form: FormHook<MyForm>) => void }) => {
const { form } = useForm<MyForm>({ defaultValue: { name: 'John' } });
useEffect(() => {
onForm(form);
}, [form]);
return (
<Form form={form}>
<UseField path="name" config={fieldConfig} data-test-subj="myField" />
</Form>
);
};
test('should call each handler at expected lifecycle', async () => {
const setup = registerTestBed(TestComp, {
memoryRouter: { wrapComponent: false },
defaultProps: { onForm: onFormHook },
});
const testBed = setup() as TestBed;
if (!formHook) {
throw new Error(
`formHook is not defined. Use the onForm() prop to update the reference to the form hook.`
);
}
const { form } = testBed;
expect(deserializer).toBeCalled();
expect(serializer).not.toBeCalled();
expect(formatter).not.toBeCalled();
let formData = formHook.getFormData({ unflatten: false });
expect(formData.name).toEqual('John-deserialized');
await act(async () => {
form.setInputValue('myField', 'Mike');
});
expect(formatter).toBeCalled(); // Formatters are executed on each value change
expect(serializer).not.toBeCalled(); // Serializer are executed *only** when outputting the form data
formData = formHook.getFormData();
expect(serializer).toBeCalled();
expect(formData.name).toEqual('MIKE-serialized');
// Make sure that when we reset the form values, we don't serialize the fields
serializer.mockReset();
await act(async () => {
formHook!.reset();
});
expect(serializer).not.toBeCalled();
});
});
});

View file

@ -118,15 +118,13 @@ export const useField = <T>(
setIsChangingValue(true);
}
const newValue = serializeOutput(value);
// Notify listener
if (valueChangeListener) {
valueChangeListener(newValue as T);
valueChangeListener(value);
}
// Update the form data observable
__updateFormDataAt(path, newValue);
__updateFormDataAt(path, value);
// Validate field(s) and update form.isValid state
await __validateFields(fieldsToValidateOnChange ?? [path]);
@ -153,7 +151,6 @@ export const useField = <T>(
}
}
}, [
serializeOutput,
valueChangeListener,
errorDisplayDelay,
path,
@ -442,13 +439,7 @@ export const useField = <T>(
if (resetValue) {
setValue(initialValue);
/**
* Having to call serializeOutput() is a current bug of the lib and will be fixed
* in a future PR. The serializer function should only be called when outputting
* the form data. If we need to continuously format the data while it changes,
* we need to use the field `formatter` config.
*/
return serializeOutput(initialValue);
return initialValue;
}
},
[setValue, serializeOutput, initialValue]

View file

@ -22,7 +22,7 @@ import { act } from 'react-dom/test-utils';
import { registerTestBed, getRandomString, TestBed } from '../shared_imports';
import { Form, UseField } from '../components';
import { FormSubmitHandler, OnUpdateHandler } from '../types';
import { FormSubmitHandler, OnUpdateHandler, FormHook, ValidationFunc } from '../types';
import { useForm } from './use_form';
interface MyForm {
@ -123,6 +123,71 @@ describe('use_form() hook', () => {
expect(formData).toEqual(expectedData);
});
test('should not build the object if the form is not valid', async () => {
let formHook: FormHook<MyForm> | null = null;
const onFormHook = (_form: FormHook<MyForm>) => {
formHook = _form;
};
const TestComp = ({ onForm }: { onForm: (form: FormHook<MyForm>) => void }) => {
const { form } = useForm<MyForm>({ defaultValue: { username: 'initialValue' } });
const validator: ValidationFunc = ({ value }) => {
if (value === 'wrongValue') {
return { message: 'Error on the field' };
}
};
useEffect(() => {
onForm(form);
}, [form]);
return (
<Form form={form}>
<UseField
path="username"
config={{ validations: [{ validator }] }}
data-test-subj="myField"
/>
</Form>
);
};
const setup = registerTestBed(TestComp, {
defaultProps: { onForm: onFormHook },
memoryRouter: { wrapComponent: false },
});
const {
form: { setInputValue },
} = setup() as TestBed;
if (!formHook) {
throw new Error(
`formHook is not defined. Use the onForm() prop to update the reference to the form hook.`
);
}
let data;
let isValid;
await act(async () => {
({ data, isValid } = await formHook!.submit());
});
expect(isValid).toBe(true);
expect(data).toEqual({ username: 'initialValue' });
setInputValue('myField', 'wrongValue'); // Validation will fail
await act(async () => {
({ data, isValid } = await formHook!.submit());
});
expect(isValid).toBe(false);
expect(data).toEqual({}); // Don't build the object (and call the serializers()) when invalid
});
});
describe('form.subscribe()', () => {

View file

@ -140,7 +140,7 @@ export function useForm<T extends FormData = FormData>(
return Object.entries(fieldsRefs.current).reduce(
(acc, [key, field]) => ({
...acc,
[key]: field.__serializeOutput(),
[key]: field.value,
}),
{} as T
);
@ -233,8 +233,7 @@ export function useForm<T extends FormData = FormData>(
fieldsRefs.current[field.path] = field;
if (!{}.hasOwnProperty.call(getFormData$().value, field.path)) {
const fieldValue = field.__serializeOutput();
updateFormDataAt(field.path, fieldValue);
updateFormDataAt(field.path, field.value);
}
},
[getFormData$, updateFormDataAt]
@ -301,7 +300,7 @@ export function useForm<T extends FormData = FormData>(
setSubmitting(true);
const isFormValid = await validateAllFields();
const formData = getFormData();
const formData = isFormValid ? getFormData() : ({} as T);
if (onSubmit) {
await onSubmit(formData, isFormValid!);

View file

@ -111,14 +111,17 @@ export const CreateField = React.memo(function CreateFieldComponent({
{/* Field subType (if any) */}
<FormDataProvider pathsToWatch="type">
{({ type }) => (
<SubTypeParameter
key={type}
type={type}
isMultiField={isMultiField ?? false}
isRootLevelField={isRootLevelField}
/>
)}
{({ type }) => {
const [fieldType] = type;
return (
<SubTypeParameter
key={fieldType.value}
type={fieldType.value}
isMultiField={isMultiField ?? false}
isRootLevelField={isRootLevelField}
/>
);
}}
</FormDataProvider>
</EuiFlexGroup>
);
@ -188,7 +191,10 @@ export const CreateField = React.memo(function CreateFieldComponent({
<FormDataProvider pathsToWatch={['type', 'subType']}>
{({ type, subType }) => {
const ParametersForm = getParametersFormForType(type, subType);
const ParametersForm = getParametersFormForType(
type?.[0].value,
subType?.[0].value
);
if (!ParametersForm) {
return null;

View file

@ -98,15 +98,15 @@ export const EditField = React.memo(({ form, field, allFields, exitEdit, updateF
<FormDataProvider pathsToWatch={['type', 'subType']}>
{({ type, subType }) => {
const linkDocumentation =
documentationService.getTypeDocLink(subType) ||
documentationService.getTypeDocLink(type);
documentationService.getTypeDocLink(subType?.[0].value) ||
documentationService.getTypeDocLink(type?.[0].value);
if (!linkDocumentation) {
return null;
}
const typeDefinition = TYPE_DEFINITION[type as MainType];
const subTypeDefinition = TYPE_DEFINITION[subType as SubType];
const typeDefinition = TYPE_DEFINITION[type[0].value as MainType];
const subTypeDefinition = TYPE_DEFINITION[subType?.[0].value as SubType];
return (
<EuiFlexItem grow={false}>
@ -148,7 +148,7 @@ export const EditField = React.memo(({ form, field, allFields, exitEdit, updateF
<FormDataProvider pathsToWatch={['type', 'subType']}>
{({ type, subType }) => {
const ParametersForm = getParametersFormForType(type, subType);
const ParametersForm = getParametersFormForType(type?.[0].value, subType?.[0].value);
if (!ParametersForm) {
return null;

View file

@ -36,15 +36,18 @@ export const EditFieldHeaderForm = React.memo(
{/* Field subType (if any) */}
<FormDataProvider pathsToWatch="type">
{({ type }) => (
<SubTypeParameter
key={type}
type={type}
defaultValueType={defaultValue.type}
isMultiField={isMultiField}
isRootLevelField={isRootLevelField}
/>
)}
{({ type }) => {
const [fieldType] = type;
return (
<SubTypeParameter
key={fieldType.value}
type={fieldType.value}
defaultValueType={defaultValue.type}
isMultiField={isMultiField}
isRootLevelField={isRootLevelField}
/>
);
}}
</FormDataProvider>
</EuiFlexGroup>
@ -52,7 +55,7 @@ export const EditFieldHeaderForm = React.memo(
<FieldDescriptionSection isMultiField={isMultiField}>
<FormDataProvider pathsToWatch={['type', 'subType']}>
{({ type, subType }) => {
const typeDefinition = TYPE_DEFINITION[type as MainType];
const typeDefinition = TYPE_DEFINITION[type[0].value as MainType];
const hasSubType = typeDefinition.subTypes !== undefined;
if (hasSubType) {

View file

@ -335,6 +335,11 @@ export const reducer = (state: State, action: Action): State => {
return {
...state,
fields: updatedFields,
documentFields: {
...state.documentFields,
// If we removed the last field, show the "Create field" form
status: updatedFields.rootLevelFields.length === 0 ? 'creatingField' : 'idle',
},
// If we have a search in progress, we reexecute the search to update our result array
search: Boolean(state.search.term)
? {