Skip to content

Commit a5421d3

Browse files
Fix 4796 by simplifying Form onChange error processing (#4799)
* 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 # Conflicts: # CHANGELOG.md * - Updated `v6.x upgrade guide.md` to document the changes mentioned above * - Updated `CHANGELOG.md` to merge a few comments * - More `CHANGELOG.md` changes * Update packages/docs/docs/migration-guides/v6.x upgrade guide.md Co-authored-by: Nick Grosenbacher <[email protected]> --------- Co-authored-by: Nick Grosenbacher <[email protected]>
1 parent 4616bb6 commit a5421d3

File tree

14 files changed

+334
-157
lines changed

14 files changed

+334
-157
lines changed

CHANGELOG.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ should change the heading of the (upcoming) version to include a major version b
3737
- Updated `ArrayField` and `ObjectField` to check whether it `shouldRenderOptionalData()` and if true, calls `ObjectDataControlsField` and passes the result to its associated render template as `optionalDataControl`
3838
- Updated `ArrayFieldTemplate`, `ObjectFieldTemplate`, `TitleField` to add support for the new `optionalDataControl` feature
3939
- Added the new `OptionalDataControlTemplate` to the theme, adding it to the `templates` list
40+
- Updated `Form` as follows to fix [#4796](https://github.com/rjsf-team/react-jsonschema-form/issues/4796)
41+
- Refactored the `liveValidate()` and `mergeErrors()` functions out of `getStateFromProp()` and `processPendingChange()`
42+
- Added new, optional `customErrors?: ErrorSchemaBuilder<T>` to the `FormState`, updating the `IChangeEvent` interface to remove all of the private variables
43+
- 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
44+
- This rework resulted in any custom errors passed from custom widgets/fields will now be remembered during the validation stage
45+
- Removed the now unused `getPreviousCustomValidateErrors()` and `filterErrorsBasedOnSchema()` methods
46+
- 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)
47+
- Updated `NullField` to pass `fieldPathId.path` for the `onChange()` instead of `[name]`
4048

4149
## @rjsf/daisyui
4250

@@ -99,16 +107,18 @@ should change the heading of the (upcoming) version to include a major version b
99107
- Updated `getDefaultFormState` to fix an issue where optional array props had their default set to an empty array when they shouldn't be
100108
- Updated the `TranslatableString` enum to add three new strings in support of the new feature: `OptionalObjectAdd`, `OptionalObjectRemove` and `OptionalObjectEmptyMsg`
101109
- Added four new utility functions: `isFormDataAvailable()`, `isRootSchema()`, `optionalControlsId()`, and `shouldRenderOptionalField()`
110+
- Updated `validationDataMerge()` to add an additional, optional parameter `preventDuplicates = false`, that causes the `mergeObjects()` call to receive `preventDuplicates` instead of `true`
102111

103112
## Dev / docs / playground
104113

105114
- Updated docs for `getDefaultFormState` to reflect addition of the `initialDefaultsGenerated` prop
106-
- Updated `utility-function.me` docs to add documentation for the new functions
115+
- Updated `utility-function.me` docs to add documentation for the new functions and to update the `validationDataMerge()` function's new parameter
107116
- Also updated docs for `retrieveSchema` and `SchemaUtilsType` for the new prop
108117
- Updated `uiSchema.md` to add documentation for the new `enableOptionalDataFieldForType` prop
109-
- Updated the `v6x upgrade guide.md` to document the new feature and utility functions and changes to `retrieveSchema`
110118
- Updated the playground to add a new `Optional Data Controls` example
111119
- Updated the snapshot and jest tests for `Form` to test the new `Optional Data Controls` feature
120+
- Updated `custom-widgets-fields.md` to change the documentation around passing errors via `onChange()` to reflect the new reality
121+
- Updated the `v6x upgrade guide.md` to document the new feature, utility functions and changes to existing method parameters
112122

113123
# 6.0.0-beta-20
114124

packages/core/src/components/Form.tsx

Lines changed: 143 additions & 133 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
@@ -3337,6 +3337,38 @@ describe('ArrayField', () => {
33373337
expect(errorMessages).to.have.length(0);
33383338
});
33393339

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

33893453
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
@@ -1055,6 +1055,7 @@ If no `additionalErrorSchema` is passed, then `validationData` is returned.
10551055

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

10591060
#### Returns
10601061

0 commit comments

Comments
 (0)