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. |
LeCarbonator
changed the title
setFieldValue trigger listeners and validationsetFieldValue trigger listeners and validation
LeCarbonator
changed the title
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.
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.
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. |
|
I appear to have found a workaround for this. I have tested it and it seems to be working but maybe there is some hidden reason not to do it this way. Example say you have a fields that have a "recommended" value, but can be changed from the recommended to something else (although this is rare.) Field A - Independent field (Has a listener that sets field B to the recommended value.) Field B - used in truth table (Has a listener that sets E to the recommended value.) Field E - has a recommended value from B,C,D. When the user selects a value from field "A" the listener runs and sets field "B" // Field A's listener
fieldApi.form.setFieldValue("B", "new value"); // does not trigger B's listener.This has a cascade effect and now B's listener doesn't evaluate the truth table to set "E" to the recommended value. This PR does fix my issue and works well for my use case and it much less verbose than the workaround I just found. Workaround is to get the instance and call handleChange from the listeners. It seems to work the same as this PR's changes and I haven't noticed any issues, but maybe this workaround is not good. // fieldApi.form.setFieldValue("B", "new value");
fieldApi.form.fieldInfo.B.instance?.handleChange("new value"); // this does trigger "B"'s listeners.Field's B,C,D use the same syntax for setting "E" after evaluating to come up with what it should be eg: // B,C,D's listeners.
fieldApi.form.fieldInfo.E.instance?.handleChange("new value");I will use this workaround for now until setFieldValue works the same way. |
|
The reason it wasn't done was mostly because it was used internally. With the overrides in place in the PR, I expect this to get merged by the end of the week. |
Also adds two meta properties,
dontValidateanddontRunListeners. Should help in case someone wants extreme control, but it's more for our sake.