Skip to content

Commit

Permalink
Remove the condition that prevent the field validation to run when va…
Browse files Browse the repository at this point in the history
…lue === defaultValue

I introduced a regression when I added an "if" condition to prevent validation when the field value === the initial value. This if was added to prevent validating after resetting the field value. This created a regression as now the validation was never run on empty fields.
  • Loading branch information
sebelga committed Sep 1, 2020
1 parent ef7246f commit efa9700
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,93 @@ describe('<UseField />', () => {
});
});

describe('validation', () => {
let formHook: FormHook | null = null;

beforeEach(() => {
formHook = null;
});

const onFormHook = (form: FormHook) => {
formHook = form;
};

const getTestComp = (fieldConfig: FieldConfig) => {
const TestComp = ({ onForm }: { onForm: (form: FormHook) => void }) => {
const { form } = useForm<any>();

useEffect(() => {
onForm(form);
}, [onForm, form]);

return (
<Form form={form}>
<UseField path="name" config={fieldConfig} data-test-subj="myField" />
</Form>
);
};
return TestComp;
};

const setup = (fieldConfig: FieldConfig) => {
return registerTestBed(getTestComp(fieldConfig), {
memoryRouter: { wrapComponent: false },
defaultProps: { onForm: onFormHook },
})() as TestBed;
};

test('should update the form validity whenever the field value changes', async () => {
const fieldConfig: FieldConfig = {
defaultValue: '', // empty string, which is not valid
validations: [
{
validator: ({ value }) => {
// Validate that string is not empty
if ((value as string).trim() === '') {
return { message: 'Error: field is empty.' };
}
},
},
],
};

// Mount our TestComponent
const {
form: { setInputValue },
} = setup(fieldConfig);

if (formHook === null) {
throw new Error('FormHook object has not been set.');
}

let { isValid } = formHook;
expect(isValid).toBeUndefined(); // Initially the form validity is undefined...

await act(async () => {
await formHook!.validate(); // ...until we validate the form
});

({ isValid } = formHook);
expect(isValid).toBe(false);

// Change to a non empty string to pass validation
await act(async () => {
setInputValue('myField', 'changedValue');
});

({ isValid } = formHook);
expect(isValid).toBe(true);

// Change back to an empty string to fail validation
await act(async () => {
setInputValue('myField', '');
});

({ isValid } = formHook);
expect(isValid).toBe(false);
});
});

describe('serializer(), deserializer(), formatter()', () => {
interface MyForm {
name: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,12 @@ export const useField = <T>(
const [isChangingValue, setIsChangingValue] = useState(false);
const [isValidated, setIsValidated] = useState(false);

const isMounted = useRef<boolean>(false);
const validateCounter = useRef(0);
const changeCounter = useRef(0);
const hasBeenReset = useRef<boolean>(false);
const inflightValidation = useRef<Promise<any> | null>(null);
const debounceTimeout = useRef<NodeJS.Timeout | null>(null);
const isMounted = useRef<boolean>(false);

// -- HELPERS
// ----------------------------------
Expand Down Expand Up @@ -144,9 +145,7 @@ export const useField = <T>(
// Validate field(s) (that will update form.isValid state)
// We only validate if the value is different than the initial or default value
// to avoid validating after a form.reset() call.
if (value !== initialValue && value !== defaultValue) {
await __validateFields(fieldsToValidateOnChange ?? [path]);
}
await __validateFields(fieldsToValidateOnChange ?? [path]);

if (isMounted.current === false) {
return;
Expand All @@ -172,8 +171,6 @@ export const useField = <T>(
}, [
path,
value,
defaultValue,
initialValue,
valueChangeListener,
errorDisplayDelay,
fieldsToValidateOnChange,
Expand Down Expand Up @@ -468,6 +465,7 @@ export const useField = <T>(
setErrors([]);

if (resetValue) {
hasBeenReset.current = true;
const newValue = deserializeValue(updatedDefaultValue ?? defaultValue);
setValue(newValue);
return newValue;
Expand Down Expand Up @@ -539,6 +537,13 @@ export const useField = <T>(
}, [path, __removeField]);

useEffect(() => {
// If the field value has been reset, we don't want to call the "onValueChange()"
// as it will set the "isPristine" state to true or validate the field, which initially we don't want.
if (hasBeenReset.current) {
hasBeenReset.current = false;
return;
}

if (!isMounted.current) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ import { act } from 'react-dom/test-utils';
import { registerTestBed, getRandomString, TestBed } from '../shared_imports';

import { Form, UseField } from '../components';
import { FormSubmitHandler, OnUpdateHandler, FormHook, ValidationFunc } from '../types';
import {
FormSubmitHandler,
OnUpdateHandler,
FormHook,
ValidationFunc,
FieldConfig,
} from '../types';
import { useForm } from './use_form';

interface MyForm {
Expand Down Expand Up @@ -441,5 +447,57 @@ describe('useForm() hook', () => {
deeply: { nested: { value: '' } }, // Fallback to empty string as no config was provided
});
});

test('should not validate the fields after resetting its value (form validity should be undefined)', async () => {
const fieldConfig: FieldConfig = {
defaultValue: '',
validations: [
{
validator: ({ value }) => {
if ((value as string).trim() === '') {
return { message: 'Error: empty string' };
}
},
},
],
};

const TestResetComp = () => {
const { form } = useForm();

useEffect(() => {
formHook = form;
}, [form]);

return (
<Form form={form}>
<UseField path="username" config={fieldConfig} data-test-subj="myField" />
</Form>
);
};

const {
form: { setInputValue },
} = registerTestBed(TestResetComp, {
memoryRouter: { wrapComponent: false },
})() as TestBed;

let { isValid } = formHook!;
expect(isValid).toBeUndefined();

await act(async () => {
setInputValue('myField', 'changedValue');
});
({ isValid } = formHook!);
expect(isValid).toBe(true);

await act(async () => {
// When we reset the form, value is back to "", which is invalid for the field
formHook!.reset();
});

({ isValid } = formHook!);
expect(isValid).toBeUndefined(); // Make sure it is "undefined" and not "false".
});
});
});

0 comments on commit efa9700

Please sign in to comment.