fix(form-core): create complete field meta on setFieldValue for uninitialized fields#2245
Open
xianjianlf2 wants to merge 1 commit into
Open
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
📝 WalkthroughWalkthrough
ChangesField metadata initialization
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 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.mcszy.xyz/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.
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
1 participant
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.
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