Skip to content

chore: add retrieveSchema at Form state to memoize the result of schemUtils.retrieveSchema #3970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 22, 2023
Merged

chore: add retrieveSchema at Form state to memoize the result of schemUtils.retrieveSchema #3970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b859fb2
doesn't retrieveSchema at onChange
igorbrasileiro Nov 21, 2023
2909483
Update retrieveSchema if changed during onChange
igorbrasileiro Nov 21, 2023
f83000a
Update CHANGELOG
igorbrasileiro Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ should change the heading of the (upcoming) version to include a major version b
-->
# 5.14.3

## @rjsf/core

- add `retrieveSchema` at `Form` state to memoize the result of `schemUtils.retrieveSchema`

## @rjsf/fluentui-rc
- Updated README.md references
- Fixed width of `ArrayFieldItemTemplate` items
Expand Down
33 changes: 23 additions & 10 deletions packages/core/src/components/Form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ export interface FormState<T = any, S extends StrictRJSFSchema = RJSFSchema, F e
* `extraErrors`
*/
schemaValidationErrorSchema: ErrorSchema<T>;
// Private
/** @description result of schemaUtils.retrieveSchema(schema, formData). This a memoized value to avoid re calculate at internal functions (getStateFromProps, onChange) */
retrievedSchema: S;
}

/** The event data passed when changes have been made to the form, includes everything from the `FormState` except
Expand Down Expand Up @@ -303,7 +306,11 @@ export default class Form<
prevState: FormState<T, S, F>
): { nextState: FormState<T, S, F>; shouldUpdate: true } | { shouldUpdate: false } {
if (!deepEquals(this.props, prevProps)) {
const nextState = this.getStateFromProps(this.props, this.props.formData);
const nextState = this.getStateFromProps(
this.props,
this.props.formData,
prevProps.schema !== this.props.schema ? undefined : this.state.retrievedSchema
);
const shouldUpdate = !deepEquals(nextState, prevState);
return { nextState, shouldUpdate };
}
Expand Down Expand Up @@ -352,7 +359,7 @@ export default class Form<
* @param inputFormData - The new or current data for the `Form`
* @returns - The new state for the `Form`
*/
getStateFromProps(props: FormProps<T, S, F>, inputFormData?: T): FormState<T, S, F> {
getStateFromProps(props: FormProps<T, S, F>, inputFormData?: T, retrievedSchema?: S): FormState<T, S, F> {
const state: FormState<T, S, F> = this.state || {};
const schema = 'schema' in props ? props.schema : this.props.schema;
const uiSchema: UiSchema<T, S, F> = ('uiSchema' in props ? props.uiSchema! : this.props.uiSchema!) || {};
Expand All @@ -372,7 +379,7 @@ export default class Form<
schemaUtils = createSchemaUtils<T, S, F>(props.validator, rootSchema, experimental_defaultFormStateBehavior);
}
const formData: T = schemaUtils.getDefaultFormState(schema, inputFormData) as T;
const retrievedSchema = schemaUtils.retrieveSchema(schema, formData);
const _retrievedSchema = retrievedSchema ?? schemaUtils.retrieveSchema(schema, formData);

const getCurrentErrors = (): ValidationData<T> => {
if (props.noValidate) {
Expand All @@ -394,7 +401,7 @@ export default class Form<
let schemaValidationErrors: RJSFValidationError[] = state.schemaValidationErrors;
let schemaValidationErrorSchema: ErrorSchema<T> = state.schemaValidationErrorSchema;
if (mustValidate) {
const schemaValidation = this.validate(formData, schema, schemaUtils, retrievedSchema);
const schemaValidation = this.validate(formData, schema, schemaUtils, _retrievedSchema);
errors = schemaValidation.errors;
errorSchema = schemaValidation.errorSchema;
schemaValidationErrors = errors;
Expand All @@ -410,7 +417,7 @@ export default class Form<
errors = merged.errors;
}
const idSchema = schemaUtils.toIdSchema(
retrievedSchema,
_retrievedSchema,
uiSchema['ui:rootFieldId'],
formData,
props.idPrefix,
Expand All @@ -427,6 +434,7 @@ export default class Form<
errorSchema,
schemaValidationErrors,
schemaValidationErrorSchema,
retrievedSchema: _retrievedSchema,
};
return nextState;
}
Expand Down Expand Up @@ -550,19 +558,21 @@ export default class Form<
*/
onChange = (formData: T | undefined, newErrorSchema?: ErrorSchema<T>, id?: string) => {
const { extraErrors, omitExtraData, liveOmit, noValidate, liveValidate, onChange } = this.props;
const { schemaUtils, schema } = this.state;
const { schemaUtils, schema, retrievedSchema } = this.state;

if (isObject(formData) || Array.isArray(formData)) {
const newState = this.getStateFromProps(this.props, formData);
const newState = this.getStateFromProps(this.props, formData, retrievedSchema);
formData = newState.formData;
}

const mustValidate = !noValidate && liveValidate;
let state: Partial<FormState<T, S, F>> = { formData, schema };
let newFormData = formData;

let _retrievedSchema: S | undefined;
if (omitExtraData === true && liveOmit === true) {
const retrievedSchema = schemaUtils.retrieveSchema(schema, formData);
const pathSchema = schemaUtils.toPathSchema(retrievedSchema, '', formData);
_retrievedSchema = schemaUtils.retrieveSchema(schema, formData);
Copy link
Contributor

@nickgros nickgros Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this work if you declared the newState object outside of its current conditional block?

Suggested change
_retrievedSchema = schemaUtils.retrieveSchema(schema, formData);
_retrievedSchema = newState?.retrievedSchema ?? schemaUtils.retrieveSchema(schema, formData);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I run the test, it fails on some tests with {"omitExtraData":true,"liveOmit":true}. Not fully sure, but probably, because the schema is old with unnecessary properties. 4 tests failed

const pathSchema = schemaUtils.toPathSchema(_retrievedSchema, '', formData);

const fieldNames = this.getFieldNames(pathSchema, formData);

Expand All @@ -573,7 +583,7 @@ export default class Form<
}

if (mustValidate) {
const schemaValidation = this.validate(newFormData);
const schemaValidation = this.validate(newFormData, schema, schemaUtils, retrievedSchema);
let errors = schemaValidation.errors;
let errorSchema = schemaValidation.errorSchema;
const schemaValidationErrors = errors;
Expand All @@ -600,6 +610,9 @@ export default class Form<
errors: toErrorList(errorSchema),
};
}
if (_retrievedSchema) {
state.retrievedSchema = _retrievedSchema;
}
this.setState(state as FormState<T, S, F>, () => onChange && onChange({ ...this.state, ...state }, id));
};

Expand Down
2 changes: 2 additions & 0 deletions packages/core/test/Form.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ describeRepeated('Form common', (createFormComponent) => {
schemaValidationErrors: undefined,
schemaValidationErrorSchema: undefined,
schemaUtils: sinon.match.object,
retrievedSchema: schema,
});
});
});
Expand Down Expand Up @@ -1480,6 +1481,7 @@ describeRepeated('Form common', (createFormComponent) => {
schemaValidationErrors: undefined,
schemaValidationErrorSchema: undefined,
schemaUtils: sinon.match.object,
retrievedSchema: formProps.schema,
});
});
});
Expand Down