Skip to content

Commit e826b09

Browse files
Fix 4796 by simplifying Form onChange error processing
Fixes #4796 by simplifying `Form.onChange` error processing and removing old code in `LayoutGridField` - In `@rjsf/utils` - Updated `validationDataMerge()` to add an additional, optional parameter `preventDuplicates = false`, that causes the `mergeObjects()` call to receive `preventDuplicates` instead of `true` - In `@rjsf/core`, updated `Form` as follows to fix [#4796](#4796) - Refactored the `liveValidate()` and `mergeErrors()` functions out of `getStateFromProp()` and `processPendingChange()` - Added new, optional `customErrors?: ErrorSchemaBuilder<T>` to the `FormState`, updating the `IChangeEvent` interface to remove all of the private variables - Reworked the `newErrorSchema` handling in `processPendingChange()` to simplify the handling since `newErrorSchema` is now path-specific, adding `newErrorSchema` to `customErrors` when they don't match an existing validator-based validation - This rework resulted in any custom errors passed from custom widgets/fields will now be remembered during the validation stage - Removed the now unused `getPreviousCustomValidateErrors()` and `filterErrorsBasedOnSchema()` methods - Also, updated `LayoutGridField` to simplify `onFieldChange()` to just return the given `errorSchema` now that it is path-specific, fixing [#4796](#4796) - Also, updated `NullField` to pass `fieldPathId.path` for the `onChange()` instead of `[name]` - Updated the tests for `StringField`, `ArrayField` and `ObjectField` to verify all of the new error processing logic in `onChange()` - Updated `utility-functions.md` to update the `validationDataMerge()` function's new parameter - Updated `custom-widgets-fields.md` to change the documentation around passing errors via `onChange()` to reflect the new reality - Updated the `CHANGELOG.md` accordingly
1 parent f4cd0a5 commit e826b09

File tree

13 files changed

+333
-158
lines changed

13 files changed

+333
-158
lines changed

CHANGELOG.md

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,28 @@ should change the heading of the (upcoming) version to include a major version b
2020
## @rjsf/core
2121

2222
- Added `initialDefaultsGenerated` flag to state, which indicates whether the initial generation of defaults has been completed
23-
- Added `ObjectField` tests for additionalProperties with defaults
23+
- Added `ObjectField` tests for additionalProperties with defaults
24+
- Updated `Form` as follows to fix [#4796](https://github.com/rjsf-team/react-jsonschema-form/issues/4796)
25+
- Refactored the `liveValidate()` and `mergeErrors()` functions out of `getStateFromProp()` and `processPendingChange()`
26+
- Added new, optional `customErrors?: ErrorSchemaBuilder<T>` to the `FormState`, updating the `IChangeEvent` interface to remove all of the private variables
27+
- Reworked the `newErrorSchema` handling in `processPendingChange()` to simplify the handling since `newErrorSchema` is now path-specific, adding `newErrorSchema` to `customErrors` when they don't match an existing validator-based validation
28+
- This rework resulted in any custom errors passed from custom widgets/fields will now be remembered during the validation stage
29+
- Removed the now unused `getPreviousCustomValidateErrors()` and `filterErrorsBasedOnSchema()` methods
30+
- Updated `LayoutGridField` to simplify `onFieldChange()` to just return the given `errorSchema` now that it is path-specific, fixing [#4796](https://github.com/rjsf-team/react-jsonschema-form/issues/4796)
31+
- Updated `NullField` to pass `fieldPathId.path` for the `onChange()` instead of `[name]`
2432

2533
## @rjsf/utils
2634

2735
- Updated `getDefaultFormState` to add a new `initialDefaultsGenerated` prop flag, along with type definitions, fixing uneditable & permanent defaults with additional properties [3759](https://github.com/rjsf-team/react-jsonschema-form/issues/3759)
2836
- Updated `createSchemaUtils` definition to reflect addition of `initialDefaultsGenerated`
2937
- Updated existing tests where `getDefaultFormState` is used to reflect addition of `initialDefaultsGenerated`
38+
- Updated `validationDataMerge()` to add an additional, optional parameter `preventDuplicates = false`, that causes the `mergeObjects()` call to receive `preventDuplicates` instead of `true`
3039

31-
## @rjsf/docs
40+
## Dev / docs / playground
3241

3342
- Updated docs for `getDefaultFormState` to reflect addition of `initialDefaultsGenerated` prop
43+
- Updated `utility-functions.md` to update the `validationDataMerge()` function's new parameter
44+
- Updated `custom-widgets-fields.md` to change the documentation around passing errors via `onChange()` to reflect the new reality
3445

3546
# 6.0.0-beta-20
3647

packages/core/src/components/Form.tsx

Lines changed: 143 additions & 134 deletions
Large diffs are not rendered by default.

packages/core/src/components/fields/LayoutGridField.tsx

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import {
2626
UiSchema,
2727
ITEMS_KEY,
2828
} from '@rjsf/utils';
29-
import cloneDeep from 'lodash/cloneDeep';
3029
import each from 'lodash/each';
3130
import flatten from 'lodash/flatten';
3231
import get from 'lodash/get';
@@ -705,13 +704,8 @@ export default class LayoutGridField<
705704
*/
706705
onFieldChange = (dottedPath: string) => {
707706
return (value: T | undefined, path: FieldPathList, errSchema?: ErrorSchema<T>, id?: string) => {
708-
const { onChange, errorSchema } = this.props;
709-
let newErrorSchema = errorSchema;
710-
if (errSchema && errorSchema) {
711-
newErrorSchema = cloneDeep(errorSchema);
712-
set(newErrorSchema, dottedPath, errSchema);
713-
}
714-
onChange(value, path, newErrorSchema, id);
707+
const { onChange } = this.props;
708+
onChange(value, path, errSchema, id);
715709
};
716710
};
717711

packages/core/src/components/fields/NullField.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ import { FieldProps, FormContextType, RJSFSchema, StrictRJSFSchema } from '@rjsf
99
function NullField<T = any, S extends StrictRJSFSchema = RJSFSchema, F extends FormContextType = any>(
1010
props: FieldProps<T, S, F>,
1111
) {
12-
const { name, formData, onChange } = props;
12+
const { formData, onChange, fieldPathId } = props;
1313
useEffect(() => {
1414
if (formData === undefined) {
15-
onChange(null as unknown as T, [name]);
15+
onChange(null as unknown as T, fieldPathId.path);
1616
}
17-
}, [name, formData, onChange]);
17+
}, [fieldPathId, formData, onChange]);
1818

1919
return null;
2020
}

packages/core/test/ArrayField.test.jsx

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3336,6 +3336,38 @@ describe('ArrayField', () => {
33363336
expect(errorMessages).to.have.length(0);
33373337
});
33383338

3339+
it('should clear an error if value is entered correctly', () => {
3340+
const { node } = createFormComponent({
3341+
schema,
3342+
formData: [
3343+
{
3344+
text: 'y',
3345+
},
3346+
],
3347+
templates,
3348+
fields: {
3349+
ArrayField: ArrayFieldTest,
3350+
},
3351+
});
3352+
3353+
const inputs = node.querySelectorAll('.rjsf-field-string input[type=text]');
3354+
act(() => {
3355+
fireEvent.change(inputs[0], { target: { value: 'test' } });
3356+
});
3357+
3358+
let errorMessages = node.querySelectorAll('#root_0_text__error');
3359+
expect(errorMessages).to.have.length(1);
3360+
const errorMessageContent = node.querySelector('#root_0_text__error .text-danger').textContent;
3361+
expect(errorMessageContent).to.contain('Value must be "Appie"');
3362+
3363+
act(() => {
3364+
fireEvent.change(inputs[0], { target: { value: 'Appie' } });
3365+
});
3366+
3367+
errorMessages = node.querySelectorAll('#root_0_text__error');
3368+
expect(errorMessages).to.have.length(0);
3369+
});
3370+
33393371
it('raise an error and check if the error is displayed using custom text widget', () => {
33403372
const { node } = createFormComponent({
33413373
schema,
@@ -3383,6 +3415,38 @@ describe('ArrayField', () => {
33833415
const errorMessages = node.querySelectorAll('#root_0_text__error');
33843416
expect(errorMessages).to.have.length(0);
33853417
});
3418+
3419+
it('should clear an error if value is entered correctly using custom text widget', () => {
3420+
const { node } = createFormComponent({
3421+
schema,
3422+
formData: [
3423+
{
3424+
text: 'y',
3425+
},
3426+
],
3427+
templates,
3428+
widgets: {
3429+
TextWidget: TextWidgetTest,
3430+
},
3431+
});
3432+
3433+
const inputs = node.querySelectorAll('.rjsf-field-string input[type=text]');
3434+
act(() => {
3435+
fireEvent.change(inputs[0], { target: { value: 'hello' } });
3436+
});
3437+
3438+
let errorMessages = node.querySelectorAll('#root_0_text__error');
3439+
expect(errorMessages).to.have.length(1);
3440+
const errorMessageContent = node.querySelector('#root_0_text__error .text-danger').textContent;
3441+
expect(errorMessageContent).to.contain('Value must be "test"');
3442+
3443+
act(() => {
3444+
fireEvent.change(inputs[0], { target: { value: 'test' } });
3445+
});
3446+
3447+
errorMessages = node.querySelectorAll('#root_0_text__error');
3448+
expect(errorMessages).to.have.length(0);
3449+
});
33863450
});
33873451

33883452
describe('Dynamic uiSchema.items function', () => {

packages/core/test/LayoutGridField.test.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,7 +1448,7 @@ describe('LayoutGridField', () => {
14481448
expect(props.onFocus).toHaveBeenCalledWith(fieldId, '');
14491449
// Type to trigger the onChange
14501450
await userEvent.type(input, 'foo');
1451-
expect(props.onChange).toHaveBeenCalledWith('foo', fieldPathId.path, props.errorSchema, fieldId);
1451+
expect(props.onChange).toHaveBeenCalledWith('foo', fieldPathId.path, undefined, fieldId);
14521452
// Tab out of the input field to cause the blur
14531453
await userEvent.tab();
14541454
expect(props.onBlur).toHaveBeenCalledWith(fieldId, 'foo');
@@ -1474,7 +1474,7 @@ describe('LayoutGridField', () => {
14741474
expect(props.onFocus).toHaveBeenCalledWith(fieldId, '');
14751475
// Type to trigger the onChange
14761476
await userEvent.type(input, 'foo');
1477-
expect(props.onChange).toHaveBeenCalledWith('foo', fieldPathId.path, props.errorSchema, fieldId);
1477+
expect(props.onChange).toHaveBeenCalledWith('foo', fieldPathId.path, undefined, fieldId);
14781478
// Tab out of the input field to cause the blur
14791479
await userEvent.tab();
14801480
expect(props.onBlur).toHaveBeenCalledWith(fieldId, 'foo');
@@ -1500,7 +1500,7 @@ describe('LayoutGridField', () => {
15001500
expect(props.onFocus).toHaveBeenCalledWith(fieldId, '');
15011501
// Type to trigger the onChange
15021502
await userEvent.type(input, 'foo');
1503-
expect(props.onChange).toHaveBeenCalledWith('foo', fieldPathId.path, props.errorSchema, fieldId);
1503+
expect(props.onChange).toHaveBeenCalledWith('foo', fieldPathId.path, undefined, fieldId);
15041504
// Tab out of the input field to cause the blur
15051505
await userEvent.tab();
15061506
expect(props.onBlur).toHaveBeenCalledWith(fieldId, 'foo');
@@ -1654,8 +1654,7 @@ describe('LayoutGridField', () => {
16541654
const input = within(fields[0]).getByRole('textbox');
16551655
expect(input).toHaveValue(props.formData[fieldName]);
16561656
await userEvent.type(input, '!');
1657-
const expectedErrors = new ErrorSchemaBuilder().addErrors(ERRORS, fieldName).ErrorSchema;
1658-
expect(props.onChange).toHaveBeenCalledWith('foo!', fieldPathId.path, expectedErrors, fieldId);
1657+
expect(props.onChange).toHaveBeenCalledWith('foo!', fieldPathId.path, EXTRA_ERROR, fieldId);
16591658
});
16601659
test('renderCondition, condition fails, field and null value, NONE operator, no data', () => {
16611660
const gridProps = { operator: Operators.NONE, field: 'simpleString', value: null };

packages/core/test/ObjectField.test.jsx

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,32 @@ describe('ObjectField', () => {
376376
expect(errorMessages).to.have.length(0);
377377
});
378378

379+
it('should clear an error if value is entered correctly', () => {
380+
const { node } = createFormComponent({
381+
schema,
382+
fields: {
383+
ObjectField: ObjectFieldTest,
384+
},
385+
});
386+
387+
const inputs = node.querySelectorAll('.rjsf-field-string input[type=text]');
388+
act(() => {
389+
fireEvent.change(inputs[0], { target: { value: 'hello' } });
390+
});
391+
392+
let errorMessages = node.querySelectorAll('#root_foo__error');
393+
expect(errorMessages).to.have.length(1);
394+
const errorMessageContent = node.querySelector('#root_foo__error .text-danger').textContent;
395+
expect(errorMessageContent).to.contain('Value must be "test"');
396+
397+
act(() => {
398+
fireEvent.change(inputs[0], { target: { value: 'test' } });
399+
});
400+
401+
errorMessages = node.querySelectorAll('#root_foo__error');
402+
expect(errorMessages).to.have.length(0);
403+
});
404+
379405
it('raise an error and check if the error is displayed using custom text widget', () => {
380406
const { node } = createFormComponent({
381407
schema,
@@ -411,6 +437,31 @@ describe('ObjectField', () => {
411437
const errorMessages = node.querySelectorAll('#root_foo__error');
412438
expect(errorMessages).to.have.length(0);
413439
});
440+
441+
it('should clear an error if value is entered correctly using custom text widget', () => {
442+
const { node } = createFormComponent({
443+
schema,
444+
widgets: {
445+
TextWidget: TextWidgetTest,
446+
},
447+
});
448+
449+
const inputs = node.querySelectorAll('.rjsf-field-string input[type=text]');
450+
act(() => {
451+
fireEvent.change(inputs[0], { target: { value: 'hello' } });
452+
});
453+
454+
let errorMessages = node.querySelectorAll('#root_foo__error');
455+
expect(errorMessages).to.have.length(1);
456+
const errorMessageContent = node.querySelector('#root_foo__error .text-danger').textContent;
457+
expect(errorMessageContent).to.contain('Value must be "test"');
458+
act(() => {
459+
fireEvent.change(inputs[0], { target: { value: 'test' } });
460+
});
461+
462+
errorMessages = node.querySelectorAll('#root_foo__error');
463+
expect(errorMessages).to.have.length(0);
464+
});
414465
});
415466

416467
describe('fields ordering', () => {

packages/core/test/StringField.test.jsx

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,32 @@ describe('StringField', () => {
351351
expect(errorMessages).to.have.length(0);
352352
});
353353

354+
it('should clear an error if value is entered correctly', () => {
355+
const { node } = createFormComponent({
356+
schema: { type: 'string' },
357+
fields: {
358+
StringField: StringFieldTest,
359+
},
360+
});
361+
362+
const inputs = node.querySelectorAll('.rjsf-field-string input[type=text]');
363+
act(() => {
364+
fireEvent.change(inputs[0], { target: { value: 'hello' } });
365+
});
366+
367+
let errorMessages = node.querySelectorAll('#root__error');
368+
expect(errorMessages).to.have.length(1);
369+
const errorMessageContent = node.querySelector('#root__error .text-danger').textContent;
370+
expect(errorMessageContent).to.contain('Value must be "test"');
371+
372+
act(() => {
373+
fireEvent.change(inputs[0], { target: { value: 'test' } });
374+
});
375+
376+
errorMessages = node.querySelectorAll('#root__error');
377+
expect(errorMessages).to.have.length(0);
378+
});
379+
354380
it('raise an error and check if the error is displayed using custom text widget', () => {
355381
const { node } = createFormComponent({
356382
schema: { type: 'string' },

packages/docs/docs/advanced-customization/custom-widgets-fields.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,11 @@ The default widgets you can override are:
100100
## Raising errors from within a custom widget or field
101101

102102
You can raise custom 'live validation' errors by overriding the `onChange` method to provide feedback while users are actively changing the form data.
103-
Note that these errors are temporary and are not recognized during the form validation process.
103+
If you do set errors this way, you must also clear them this way by passing `undefined` to the `onChange()` for the `errorSchema` parameter.
104104

105105
:::warning
106106

107-
This method of raising errors _only_ runs during `onChange`, i.e. when the user is changing data. This will not catch errors `onSubmit`, i.e when submitting the form.
108-
If you wish to add generic validation logic for your component, you should use the [`customValidate` Form prop](../api-reference/form-props.md#customvalidate).
107+
While these errors are retained during validation, it is still preferred for you to use the [`customValidate` Form prop](../api-reference/form-props.md#customvalidate) mechanism instead.
109108

110109
:::
111110

packages/docs/docs/api-reference/utility-functions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,7 @@ If no `additionalErrorSchema` is passed, then `validationData` is returned.
996996

997997
- validationData: ValidationData&lt;T> - The current `ValidationData` into which to merge the additional errors
998998
- [additionalErrorSchema]: ErrorSchema&lt;T> | undefined - The optional additional set of errors in an `ErrorSchema`
999+
- [preventDuplicates=false]: boolean - Optional flag, if true, will call `mergeObjects()` with `preventDuplicates`
9991000

10001001
#### Returns
10011002

0 commit comments

Comments
 (0)