-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
fix: formData change clear errorMessage #4429
Conversation
formData change should behave the same as onChange method, I have implemented this |
fix: merge errorSchema fix: merge errorSchema
185669f
to
d4faff8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JinYuSha0 Please update the CHANGELOG.md
to add a new 5.24.0
section since you are adding a new feature to @rjsf/utils
. And make the few suggested changes. Also, provide 100% unit tests for the getChangedFields()
function.
Also, these unit test, when written, should help you fix the test failures that exist in your PR.
* const changedFields = getChangedFields(a, b); | ||
* console.log(changedFields); // Output: ['age'] | ||
*/ | ||
export default function getChangedFields(a: any, b: any): string[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better Typescript compatibility, unknown
is preferred over any
export default function getChangedFields(a: any, b: any): string[] { | |
export default function getChangedFields(a: unknown, b: unknown): string[] { |
* @param {any} a - The first object, representing the original data to compare. | ||
* @param {any} b - The second object, representing the updated data to compare. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @param {any} a - The first object, representing the original data to compare. | |
* @param {any} b - The second object, representing the updated data to compare. | |
* @param a - The first object, representing the original data to compare. | |
* @param b - The second object, representing the updated data to compare. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm done with this
@JinYuSha0 I just merged another fix to the method you are changing... Can you first check to see if that fix also fixes the issue you are attempting to fix with this PR? Thanks. If not, then rebase that change into your PR first. |
I pulled the latest commit in the main branch and executed build, it seems that this problem has not been solved, thank you very much for your attention to this issue.
|
@JinYuSha0 In that case, would you be willing to rebase your changes on the latest and deal with the conflicts so that we can get your fix in? |
Also, it looks like your changes are breaking a |
I fix the breaking
|
Reasons for making this change
fixes #4426
If this is related to existing tickets, include links to them as well. Use the syntax
fixes #[issue number]
(ex:fixes #123
).If your PR is non-trivial and you'd like to schedule a synchronous review, please add it to the weekly meeting agenda: https://docs.google.com/document/d/12PjTvv21k6LIky6bNQVnsplMLLnmEuypTLQF8a-8Wss/edit
Checklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update
to update snapshots, if needed.