fix(form-core): create complete field meta on setFieldValue for uninitialized fields - #2245
Conversation
…tialized fields When `setFieldValue` targets a field that has not been mounted yet (e.g. a conditionally rendered field pre-populated before render), the meta updater spread an `undefined` previous meta, producing a partial object missing required fields such as `isValidating`, `isBlurred` and `errorSourceMap`. Default the previous meta to `defaultFieldMeta` so the newly created meta is structurally complete and consistent with a normally mounted field, matching the pattern already used by other `setFieldMeta` call sites. Closes TanStack#1078
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesField metadata initialization
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/form-core/tests/FormApi.spec.ts (1)
102-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlso assert touched/dirty state preservation.
The production change explicitly preserves
isTouchedandisDirty, but this regression test does not verify either field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/form-core/tests/FormApi.spec.ts` around lines 102 - 110, Extend the metadata assertions in FormApi.spec.ts to verify that the preserved isTouched and isDirty values remain defined and retain their expected state alongside isValidating, isBlurred, and errorSourceMap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/form-core/tests/FormApi.spec.ts`:
- Line 92: Update the test around form.mount() to capture its returned cleanup
function and invoke that cleanup in a finally block, ensuring listeners are
removed even when the test fails.
---
Nitpick comments:
In `@packages/form-core/tests/FormApi.spec.ts`:
- Around line 102-110: Extend the metadata assertions in FormApi.spec.ts to
verify that the preserved isTouched and isDirty values remain defined and retain
their expected state alongside isValidating, isBlurred, and errorSourceMap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8aef8188-71ed-4945-96e1-c8181d1d2a35
📒 Files selected for processing (2)
packages/form-core/src/FormApi.tspackages/form-core/tests/FormApi.spec.ts
| availableFor: 'longStay', | ||
| } as { availableFor: string; fees?: number }, | ||
| }) | ||
| form.mount() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Clean up the form mount in the test.
form.mount() registers listeners and returns a cleanup function. Capture and invoke it in finally to prevent cross-test listener leaks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/form-core/tests/FormApi.spec.ts` at line 92, Update the test around
form.mount() to capture its returned cleanup function and invoke that cleanup in
a finally block, ensuring listeners are removed even when the test fails.
There was a problem hiding this comment.
Added the touched/dirty assertions from the review summary in 51d4103 (verified the runtime values by running the test). On the mount cleanup: skipping this one — the suite has ~136 form.mount() calls and none capture the cleanup, and the event-client listeners are keyed by each form's unique _formId, so there's no cross-test interference. Making just this test different from the rest of the file felt inconsistent; happy to do a suite-wide cleanup pass in a separate PR if maintainers want it.
MILLERMARRU
left a comment
MILLERMARRU
left a comment
There was a problem hiding this comment.
Traced setFieldMeta to confirm this actually reproduces the bug the way I expected. It calls functionalUpdate(updater, prev.fieldMetaBase[field] as never), so for a field that's never been touched, the updater genuinely gets called with undefined, not some partial object. Before this fix, prev?.errorMap made that safe from throwing, but {...prev, isTouched: true, isDirty: true, errorMap: {...}} on an undefined prev spreads to nothing, so the resulting meta only ever had isTouched, isDirty and errorMap set, every other documented field (isValidating, isBlurred, isPristine, isValid, errors, errorSourceMap, etc.) came out undefined instead of the real default.
Using defaultFieldMeta as the parameter default is the right fix and it's not a new idiom, bumpArrayVersion in metaHelper.ts already does formApi.getFieldMeta(field) ?? defaultFieldMeta for the same reason, so this brings setFieldValue in line with how the rest of the file already handles a possibly-uninitialized meta object.
Test directly checks the fields that would have silently come back undefined before (isValidating, isBlurred, errorSourceMap), which is the part that actually matters here, a shallow check on just isTouched/isDirty would have passed both before and after the fix and not caught anything.
2 participants
What & why
FormApi.setFieldValueon a field that has never mounted built its meta with(prev) => ({ ...prev, isTouched, isDirty, errorMap })whereprevisundefined, producing a partial meta object missing required fields declared onAnyFieldLikeMetaBase(isValidating,isBlurred,errorSourceMap,_arrayVersion,_pendingValidationsCount). This is the incomplete-meta problem reported in #1078.Fix
Default the updater argument to the shared
defaultFieldMeta:in
packages/form-core/src/FormApi.ts, mirroring the exact pattern already used at the two othersetFieldMetacall sites in the same file. The meta created for a never-mounted field is now structurally identical to a mounted field's.Tests
packages/form-core/tests/FormApi.spec.ts— added a failing-first test asserting the created meta is complete (isValidating: false,isBlurred: false,errors: [],errorSourceMap: {}, noundefinedfields) aftersetFieldValueon an uninitialized field. Confirmed red on original code → green after the fix.Full
@tanstack/form-coresuite: 504 passed / 3 todo, no type errors.Closes #1078
Summary by CodeRabbit
Bug Fixes
Tests
setFieldValueon an unmounted field, verifying default error arrays/maps and related flags are consistently populated.