-
-
Notifications
You must be signed in to change notification settings - Fork 512
fix(form-core): Trigger listeners and validation on setFieldValue
#1680
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
base: main
Are you sure you want to change the base?
fix(form-core): Trigger listeners and validation on setFieldValue
#1680
Conversation
View your CI Pipeline Execution ↗ for commit 724f0a4
☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1680 +/- ##
==========================================
+ Coverage 90.49% 90.61% +0.12%
==========================================
Files 37 37
Lines 1683 1705 +22
Branches 421 435 +14
==========================================
+ Hits 1523 1545 +22
Misses 143 143
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I initially thought I should mark as draft to refactor how form listeners are currently set up. However, it appears that it would be a breaking change since So I suppose this is already done. |
setFieldValue
trigger listeners and validationsetFieldValue
trigger listeners and validation
setFieldValue
trigger listeners and validationsetFieldValue
this.form.setFieldValue( | ||
this.name, | ||
updater as never, | ||
mergeOpts(options, { dontRunListeners: true, dontValidate: true }), |
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.
so if options === undefined
, the way mergeOpts
works, is that it will return dontValidate === true
. therefore, by default, calling setFieldValue
does not validate?
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.
See lines 1360-1366. We are telling setFieldValue
that we're taking care of it ourselves and it doesn't need to invest its work for that.
mergeOpts
simply ensures that the overrides are present for this internal call
Hello just wanted to say I ran into this issue just a few hours ago. I have a form a very complex one 100+ fields. The form is pretty linear in nature so this helps populate fields with the recommended values as things are being setup. The issue I ran into was documented here: As I scrolled through I saw @LeCarbonator ask for testing: I wanted to let you know I installed this version from the PR and I am correctly seeing listeners being triggered from setFieldValue, where I wasn't before (but expected to.) I hope this helps and I look forward to seeing this make it into a new release soon! |
I'm working with a large, 260+ field form with complex dependencies that has become difficult to maintain using React Hook Form. After research, I've narrowed my choices to TanStack Form and Alibaba Formily because of their excellent side-effect management. I intend to test this PR to compare them for my project. Even if I don't end up using TanStack Form, I'm eager to see this PR merged quickly. |
Also adds two meta properties,
dontValidate
anddontRunListeners
. Should help in case someone wants extreme control, but it's more for our sake.