fix: ensure fieldName is passed to custom validation logic functions#2127
Merged
harry-whorlow merged 3 commits intoTanStack:mainfrom Apr 21, 2026
Merged
Conversation
📝 WalkthroughWalkthroughFieldApi now supplies Changes
Sequence Diagram(s)sequenceDiagram
participant FieldApi as FieldApi
participant Utils as getSync/AsyncValidatorArray
participant ValidationLogic as validationLogic
participant Validators as ValidatorFns
FieldApi->>Utils: request validator array (include fieldName)
Utils->>ValidationLogic: call validationLogic(event { type, async, fieldName })
ValidationLogic->>Validators: decide/append validators (may use fieldName)
Validators-->>ValidationLogic: return selected validators
ValidationLogic-->>Utils: return final validator list
Utils-->>FieldApi: provide validators / run validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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/DynamicValidation.spec.ts (1)
238-333: Consider adding one async linked-field case to prevent fieldName regressions.A small async linked validation scenario asserting
props.event.fieldNamefor the linked field would lock in the async path as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/form-core/tests/DynamicValidation.spec.ts` around lines 238 - 333, Add an async linked-field scenario to the existing test to cover the async path that depends on props.event.fieldName: update the validationLogic used in the spec to include a case where a validator is async and references a linked field (so props.event.fieldName is set for that linked field) and then exercise that path by triggering the linked-field event (e.g., blur/change/submit on the linked field) and awaiting the validation; specifically modify the test that defines validationLogic, FieldApi instances and their validators ('onDynamic') to add a linked async validator and assertions that await and assert errorMap for the linked field to ensure the props.event.fieldName branch is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/form-core/src/FieldApi.ts`:
- Around line 1828-1832: Linked-field async validators are receiving the
triggering field's name (this.name) instead of the linked field's name; update
the linked-field validation loop where getAsyncValidatorArray is called (using
field.options, field.form, validationLogic) to set fieldName to the linked
field's identifier (e.g., linkedField.name or the loop variable representing the
linked field) rather than this.name so event.fieldName in the custom async
validationLogic reflects the linked field.
---
Nitpick comments:
In `@packages/form-core/tests/DynamicValidation.spec.ts`:
- Around line 238-333: Add an async linked-field scenario to the existing test
to cover the async path that depends on props.event.fieldName: update the
validationLogic used in the spec to include a case where a validator is async
and references a linked field (so props.event.fieldName is set for that linked
field) and then exercise that path by triggering the linked-field event (e.g.,
blur/change/submit on the linked field) and awaiting the validation;
specifically modify the test that defines validationLogic, FieldApi instances
and their validators ('onDynamic') to add a linked async validator and
assertions that await and assert errorMap for the linked field to ensure the
props.event.fieldName branch is exercised.
🪄 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: 18dbf798-0f75-4e1b-8636-58dd478bdd92
📒 Files selected for processing (5)
.changeset/whole-views-wear.mdpackages/form-core/src/FieldApi.tspackages/form-core/src/ValidationLogic.tspackages/form-core/src/utils.tspackages/form-core/tests/DynamicValidation.spec.ts
This was always part of the types for custom validation functions, it just wasn't wired up. This update is pretty straightforward - it just wires up the field, and adds a test to prove that custom field-level dynamic validation is now possible. The example in the test mostly follows a basic version of what's outlined in TanStack#1861. Fixes TanStack#1861
mwg-ofx
force-pushed
the
dynamic-validation-fieldname
branch
from
ad2cd71 to
72c0b37
Compare
Member
|
Whoops! My bad for having this bug in the first place! Good catch. The code generally LGTM at a quick glance, but I want to have another maintainer take a closer look at the tests in particular since I'm deep in Form Group land and just got an email so I thought I'd take a look. I'll have it prioritized given the quality of the PR. Thanks! |
|
View your CI Pipeline Execution ↗ for commit b3a5729
☁️ 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 #2127 +/- ##
==========================================
- Coverage 90.35% 90.25% -0.10%
==========================================
Files 38 49 +11
Lines 1752 2043 +291
Branches 444 532 +88
==========================================
+ Hits 1583 1844 +261
- Misses 149 179 +30
Partials 20 20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
This pr is pretty solid! I'm just trying to reason through some stuff, but I want to get this merged by the end of the weekend. 🚀 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/form-core/tests/DynamicValidation.spec.ts (1)
427-502: Restore real timers after this test completes.The test calls
vi.useFakeTimers()on line 428 but never restores them, which can cause test pollution for subsequent tests. Wrap the test body intry/finallyand callvi.useRealTimers()in the finally block.Proposed fix
it('should support async validation', async () => { vi.useFakeTimers() + try { const form = new FormApi({ defaultValues: { firstName: '', @@ fieldLastName.handleChange('Smith') await vi.runAllTimersAsync() expect(fieldLastName.state.meta.errorMap.onDynamic).toBe(undefined) + } finally { + vi.useRealTimers() + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/form-core/tests/DynamicValidation.spec.ts` around lines 427 - 502, The test "should support async validation" uses vi.useFakeTimers() but never restores timers; wrap the test body (the async function passed to it) in a try/finally and call vi.useRealTimers() in the finally block so timers are restored even on failure. Locate the test by the it(...) with description 'should support async validation', add a try/finally around its existing logic and invoke vi.useRealTimers() in the finally to undo vi.useFakeTimers().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/form-core/tests/DynamicValidation.spec.ts`:
- Around line 306-310: The test currently uses
expect.arrayContaining(['one','one',undefined,undefined]) which doesn't assert
exact call counts; update the assertion after await form.handleSubmit() to
explicitly verify that validate was invoked twice for the 'one' field and that
two undefined entries exist (or assert strict array equality of submitTarget to
the expected array). Concretely, replace the arrayContaining check on
submitTarget with assertions that count occurrences of 'one' (expect length 2)
and occurrences of undefined (expect length 2), or assert submitTarget === the
exact expected array; reference submitTarget, form.handleSubmit,
validateAllFields and validate when locating the failing expectation.
---
Nitpick comments:
In `@packages/form-core/tests/DynamicValidation.spec.ts`:
- Around line 427-502: The test "should support async validation" uses
vi.useFakeTimers() but never restores timers; wrap the test body (the async
function passed to it) in a try/finally and call vi.useRealTimers() in the
finally block so timers are restored even on failure. Locate the test by the
it(...) with description 'should support async validation', add a try/finally
around its existing logic and invoke vi.useRealTimers() in the finally to undo
vi.useFakeTimers().
🪄 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: ed0e51ae-7c36-4d25-a980-9be21a21df25
📒 Files selected for processing (1)
packages/form-core/tests/DynamicValidation.spec.ts
Contributor
|
LGTM 🚀 |
Merged
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
3 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.
🎯 Changes
This was always part of the types for custom validation functions (see #1622), it just wasn't wired up. This update is pretty straightforward - it just wires up the field, and adds a test to prove that custom field-level dynamic validation is now possible. The example in the test mostly follows a basic version of what's outlined in #1861.
Fixes #1861
✅ Checklist
pnpm test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Tests