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

chore: Avoid Form retrieveSchema twice when liveValidate is true #3959

Merged
merged 2 commits into from
Nov 16, 2023

Conversation

igorbrasileiro
Copy link
Contributor

@igorbrasileiro igorbrasileiro commented Nov 14, 2023

Reasons for making this change

When utilizing the Form with liveValidate, the getStateFromProps function triggers the retrieveSchema function twice. Profiling timings indicate that the retrieveSchema function accounts for approximately 20% (18ms out of 99ms) of the total execution time for getStateFromProps.

image (getStateFromProps from the form first render)

This issue becomes more pronounced, particularly when the form is "controlled" with liveValidate. In this scenario, each key press triggers the getStateFromProps function, resulting in two calls to retrieveSchema. Given that retrieveSchema consumes more than 20% of getStateFromProps execution time, this pull request addresses the inefficiency by optimizing the retrieval process, ultimately improving the overall performance of the form while typing.

Related to #1961

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run 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
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@heath-freenome
Copy link
Member

@igorbrasileiro Can you please run npm run cs-format on the code along with updating the CHANGELOG.md with my feedback?

CHANGELOG.md Outdated Show resolved Hide resolved
@igorbrasileiro
Copy link
Contributor Author

igorbrasileiro commented Nov 15, 2023

@igorbrasileiro Can you please run npm run cs-format on the code along with updating the CHANGELOG.md with my feedback?

Sure! Done, thank you for the feedback

@heath-freenome
Copy link
Member

@igorbrasileiro Sorry for the extra work but can your rebase and resolve conflicts?

@igorbrasileiro
Copy link
Contributor Author

igorbrasileiro commented Nov 15, 2023

@igorbrasileiro Sorry for the extra work but can your rebase and resolve conflicts?

Rebased

@igorbrasileiro
Copy link
Contributor Author

igorbrasileiro commented Nov 16, 2023

@heath-freenome do I have something from my side? Thank you for the attention

About the problem, I'm not fully sure if exists this same behavior in other components like array, multischema or object field template.

@heath-freenome
Copy link
Member

heath-freenome commented Nov 16, 2023

@heath-freenome do I have something from my side? Thank you for the attention

I have to approve and merge, which I just did

About the problem, I'm not fully sure if exists this same behavior in other components like array, multischema or object field template.

Retrieve schema has necessary calls in the object field and array field templates

@heath-freenome heath-freenome merged commit 2a3c7c8 into rjsf-team:main Nov 16, 2023
4 checks passed
@magaton magaton mentioned this pull request Jan 19, 2024
4 tasks
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.

3 participants