Skip to content
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.

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

Bug: Issue with formData not updating when dependencies change #4388

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

abdalla-rko
Copy link
Contributor

Reasons for making this change

Fixes #4325

Checklist

  • I'm adding or updating code
    • I've added and/or updated tests. I've run npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR

packages/utils/src/mergeDefaultsWithFormData.ts Outdated Show resolved Hide resolved
packages/utils/src/mergeDefaultsWithFormData.ts Outdated Show resolved Hide resolved
packages/utils/src/mergeDefaultsWithFormData.ts Outdated Show resolved Hide resolved
packages/utils/src/schema/getDefaultFormState.ts Outdated Show resolved Hide resolved
packages/utils/src/schema/getDefaultFormState.ts Outdated Show resolved Hide resolved
packages/utils/src/schema/getDefaultFormState.ts Outdated Show resolved Hide resolved
packages/utils/src/schema/getDefaultFormState.ts Outdated Show resolved Hide resolved
packages/utils/test/schema/getDefaultFormStateTest.ts Outdated Show resolved Hide resolved
@heath-freenome
Copy link
Member

@abdalla-rko Can you fix the tests?

Copy link
Member

@heath-freenome heath-freenome left a comment

Choose a reason for hiding this comment

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

A few questions and one suggested fix and I believe we are good to go

CHANGELOG.md Outdated
@@ -16,6 +16,12 @@ should change the heading of the (upcoming) version to include a major version b

-->

# 5.24.0
Copy link
Member

Choose a reason for hiding this comment

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

Should this be 5.23.3 since there isn't a new feature?

}
return formData;
return overrideFormDataWithDefaults ? defaults : formData;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to optimize this logic so that we have:

if (/*conditions for defaults*/) {
  return defaults;
}
return formData;

Comment on lines 355 to 356
* Ensure that the formData matches the given schema. If it's not matching in the case of a selectField, we change it to match the schema.
* @param validator - an implementation of the `ValidatorType` interface that will be used when necessary
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Ensure that the formData matches the given schema. If it's not matching in the case of a selectField, we change it to match the schema.
* @param validator - an implementation of the `ValidatorType` interface that will be used when necessary
* Ensure that the formData matches the given schema. If it's not matching in the case of a selectField, we change it to match the schema.
*
* @param validator - an implementation of the `ValidatorType` interface that will be used when necessary

packages/utils/src/schema/getDefaultFormState.ts Outdated Show resolved Hide resolved
packages/utils/src/schema/getDefaultFormState.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default values not changing when dependencies updated
3 participants