fix(components): requiredWhen decides at submit time, not on the first TRUE it ever saw (#4161) - #4201
Merged
Conversation
…t TRUE it ever saw
A `requiredWhen` predicate that flipped to FALSE after the dialog mounted
updated only the display layer: the asterisk and `aria-required` both
disappeared, while submit stayed refused with "<field> is required" and no
write was ever issued (objectui#4161).
The cause is not a mount-time snapshot, which is what the symptom looks like.
The renderer hands react-hook-form its per-field rules as a `<Controller
rules>` prop, and RHF MERGES that object into the field descriptor it already
holds -- `_f: { ...previous._f, ...options }` -- so a rule key that stops being
spelled is never removed. Rules could be added live (a predicate flipping TRUE
after mount did start enforcing) but never withdrawn: the `validate.required`
entry installed on the first TRUE outlived every later FALSE. The validation
layer was append-only, latched.
The entry is now registered unconditionally and decides required-ness when it
runs, reading the live verdict the renderer publishes each render -- the same
single `resolveFieldRuleState` result that draws the asterisk, not a second
evaluation of the predicate with its own copy of the record assembly.
Both directions pinned, plus the two controls from the report: statically
required fields still enforce, and an edit form whose predicate was already
FALSE at mount still saves.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4161
The symptom, reproduced
A component-level pin renders the reporter's exact shape — a plain field
a, and a fieldbcarryingrequiredWhen: '!(has(record.a) && record.a == "x")'— then drives the reported sequence: predicate TRUE at mount, user setsa = "x", submit.On
origin/main@cb13400, with the tests present and the fix reverted:The failing assertion is the submit handler never being called — the client-side half of the reporter's "zero POST". The same test asserts, one line earlier, that
aria-requiredHAS already disappeared from the control, so what is pinned is the divergence between the two layers, not "the predicate never re-evaluated".Root cause — the mount-time-snapshot hypothesis does not survive measurement
The issue reads "submit-time validation uses the mount-time snapshot". The observable symptom is right; that mechanism is not, and the discriminator is in the run above: the flip-to-TRUE direction is GREEN on unfixed main. A predicate that becomes TRUE after mount does start blocking submit. A mount-time snapshot could not do that.
What actually happens, from
react-hook-form@7.84.0:useControllercallscontrol.register(name, { ...props.rules, value })inside aReact.useRef(...)initializer argument.useRefdiscards the result after the first render, but the argument expression is still evaluated on every render — so registration does run per render, with the current rules. Measured with an instrumented probe: threeregistercalls for the field in the renders following the flip.registerwrites_f: { ...(field && field._f ? field._f : { ref }), name, mount: true, ...options }— an additive merge. A key absent fromoptionsis not deleted.So the rules object could gain entries live but never lose them. The renderer only put
validate.requiredintoruleswhilerequiredwas true (form.tsx, the oldif (required) { ... }block); once the predicate went FALSE the key simply stopped being spelled, the merge left the previous validator in place, and the field stayed required forever. Same probe, after the flip:The validation layer was append-only, latched on the first TRUE the field ever produced — not snapshotted at mount. The two hypotheses agree on every control the reporter ran (star flips; edit dialog already-FALSE at mount saves; direct POST 201) and disagree only on the TRUE direction, which is why the filing landed where it did.
The fix
packages/components/src/renderers/form/form.tsx, one call site:validate.requiredentry is registered unconditionally and decides required-ness when it runs. Always spelling the key gives RHF's merge something to overwrite; deciding at call time means the answer is right even for the closure RHF captured at mount, so the fix does not rest on thatuseRefargument re-evaluation continuing to happen.Map— and it is the sameresolveFieldRuleStateresult that draws the asterisk andaria-required, taken from the same variable. One evaluation, two consumers.Deliberately not done: re-evaluating the predicate inside the validator. That would be a second evaluation site needing its own copy of the record assembly (the
nullseeding of declared fields, thepreviousRecordoverlay) — the exact drift this issue is about. No change to@object-ui/core;resolveFieldRuleStatewas always correct, and nothing new was added to its contract.Scope and adjacency
requiredWhen+ a runtimedefaultValuestill deadlocks a create form (the conditional half of #4069) #4085 is untouched. Its case is a predicate that is TRUE at create alongside a runtimedefaultValue; the predicate never flips, so the verdict published on the first render is the one enforced, exactly as before. The behaviour delta of this PR is confined to fields whose verdict changes after mount. No test ofrequiredWhen+ a runtimedefaultValuestill deadlocks a create form (the conditional half of #4069) #4085's shape changes colour.WizardForm'smissingRequiredByStepalready evaluatesresolveFieldRuleStatelive at submit and is unaffected.check:i18n-keys/check:i18n-driftsteps both passed.Verification
Reverse verification ran in the predicted direction (predicted before running): revert
form.tsxonly, keep the tests → the two flip-to-FALSE pins go red with the filing's signature (submit handler uncalled), the symmetric TRUE pin and both controls stay green. Output quoted at the top of this description.Local, repo root, on the pushed tree:
CI on
ec6a9ee— all 20 checks concluded, zero failures: Lint ✓ (this is where the family gates run), Type Check ✓, Test shards 1/4 ✓ 2/4 ✓ 3/4 ✓ 4/4 ✓ (shard 1 alone: 297 files / 3731 tests passed), Build & E2E ✓, Build Docs ✓, Bundle Analysis ✓, Control Byte Scan ✓, Changeset Declaration / Bump Policy / Fixed Group ✓, Internal Docs Link Check ✓, Skill Guide Path Check ✓, Live E2E (informational) ✓.The whole-package
packages/components/local run was queued on the shared verification lock behind another agent's whole-repo run for the length of the task and was cancelled as redundant once CI's four shards — which run every file in that package on this exact commit — came back green.Generated by Claude Code