fix: remove errant Field.Field usage and useForm().useField()#2149
fix: remove errant Field.Field usage and useForm().useField()#2149crutchcorn merged 13 commits intomainfrom
Field.Field usage and useForm().useField()#2149Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (6)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughThe PR removes Field sub-component augmentation from useField across frameworks, updates type/imports, simplifies useField to return FieldApi directly, switches Svelte createForm to expose a Field component constructor, and adjusts Vue useForm wiring to drop extendedApi.useField. ChangesuseField Hook API Simplification (React + Vue + Svelte + Solid)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
View your CI Pipeline Execution ↗ for commit 2e14fe3
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
![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
🤖 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/react-form/src/useField.tsx`:
- Line 275: The PR removed the ReactFieldApi augmentation and the
extendedApi.Field assignment which breaks consumers using field.Field (e.g., in
<form.Field>{(field) => <field.Field .../>}</form.Field>); update the release
notes and migration guide and ensure the commit includes a Conventional Commits
breaking change marker (either use fix!: or add a BREAKING CHANGE: footer) that
explains the removal, cites the affected symbols (ReactFieldApi,
extendedApi.Field, field.Field), and documents the recommended replacements (use
the outer form.Field or call useField directly) with a short example and any
migration steps.
🪄 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: 25919c7d-28c0-465a-abc8-bcd99ea17f73
📒 Files selected for processing (1)
packages/react-form/src/useField.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2149 +/- ##
===========================================
- Coverage 90.35% 73.66% -16.70%
===========================================
Files 38 32 -6
Lines 1752 467 -1285
Branches 444 61 -383
===========================================
- Hits 1583 344 -1239
+ Misses 149 107 -42
+ Partials 20 16 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
crutchcorn
force-pushed
the
remove-dupe-field
branch
from
426097e to
3097a2d
Compare
crutchcorn
force-pushed
the
remove-dupe-field
branch
from
3097a2d to
13bc04a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/vue-form/src/useField.tsx (2)
246-254: 💤 Low valueThe IIFE no longer serves any purpose — collapse it.
After removing the
extendedApiaugmentation, the IIFE simply instantiates and immediately returns aFieldApi. The wrapper can be dropped.♻️ Proposed simplification
- const fieldApi = (() => { - const api = new FieldApi({ - ...opts, - form: opts.form, - name: opts.name, - }) - - return api - })() + const fieldApi = new FieldApi({ + ...opts, + form: opts.form, + name: opts.name, + })🤖 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/vue-form/src/useField.tsx` around lines 246 - 254, The IIFE around fieldApi is now redundant—replace the self-invoking function with a direct instantiation: create fieldApi by assigning new FieldApi({...opts, form: opts.form, name: opts.name}) directly (remove the (()=>{...})() wrapper); ensure you keep the same variable name fieldApi and use the same opts/Form/name properties passed to the FieldApi constructor.
157-185: ⚡ Quick win
VueFieldApiis a misleading dead export that doesn't match the actualuseFieldreturn value.
VueFieldApideclaresField: FieldComponent<...>, butuseFieldreturns{ api: fieldApi, state: fieldState }wherefieldApiis a bareFieldApiinstance with noFieldproperty. While theFieldcomponent is exported separately from the same file, it is never attached to theapiobject. Any code expectingapi.Fieldbased on theVueFieldApitype will silently fail at runtime.Either remove
VueFieldApiif it's not needed, or update the interface to reflect whatuseFieldactually provides.🤖 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/vue-form/src/useField.tsx` around lines 157 - 185, The VueFieldApi type is incorrect: it claims a Field property (Field: FieldComponent<...>) but useField actually returns { api: fieldApi, state: fieldState } where fieldApi is a bare FieldApi instance and Field is exported separately; update VueFieldApi to match the actual return shape (e.g., export interface VueFieldApi<...> { api: FieldApi<...>; state: FieldState<...>; } using the existing FieldApi and FieldState types) or remove the VueFieldApi export entirely if it's unused so consumers don't expect api.Field at runtime; adjust any related exports or usages to the corrected type name (VueFieldApi) or the real Field component as needed.packages/vue-form/src/useForm.tsx (1)
4-4: 💤 Low valueMerge the two
'./useField'imports into one.Lines 4 and 20 both import from
'./useField'; they can be combined using the inlinetypemodifier.♻️ Proposed consolidation
-import { Field } from './useField' ... -import type { FieldComponent } from './useField' +import { Field, type FieldComponent } from './useField'Also applies to: 20-20
🤖 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/vue-form/src/useForm.tsx` at line 4, Two separate imports from './useField' should be merged into a single import statement; locate both import lines (the one importing Field and the other import at line 20) and consolidate them into one import from './useField', using the inline type modifier for type-only imports (e.g., import type { Field } along with any value exports) so that type/value imports are correctly distinguished and no duplicate module specifiers remain.
🤖 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.
Nitpick comments:
In `@packages/vue-form/src/useField.tsx`:
- Around line 246-254: The IIFE around fieldApi is now redundant—replace the
self-invoking function with a direct instantiation: create fieldApi by assigning
new FieldApi({...opts, form: opts.form, name: opts.name}) directly (remove the
(()=>{...})() wrapper); ensure you keep the same variable name fieldApi and use
the same opts/Form/name properties passed to the FieldApi constructor.
- Around line 157-185: The VueFieldApi type is incorrect: it claims a Field
property (Field: FieldComponent<...>) but useField actually returns { api:
fieldApi, state: fieldState } where fieldApi is a bare FieldApi instance and
Field is exported separately; update VueFieldApi to match the actual return
shape (e.g., export interface VueFieldApi<...> { api: FieldApi<...>; state:
FieldState<...>; } using the existing FieldApi and FieldState types) or remove
the VueFieldApi export entirely if it's unused so consumers don't expect
api.Field at runtime; adjust any related exports or usages to the corrected type
name (VueFieldApi) or the real Field component as needed.
In `@packages/vue-form/src/useForm.tsx`:
- Line 4: Two separate imports from './useField' should be merged into a single
import statement; locate both import lines (the one importing Field and the
other import at line 20) and consolidate them into one import from './useField',
using the inline type modifier for type-only imports (e.g., import type { Field
} along with any value exports) so that type/value imports are correctly
distinguished and no duplicate module specifiers remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a490d4dc-ecf7-46cd-8a50-2965084fbcd4
📒 Files selected for processing (3)
packages/react-form/src/useField.tsxpackages/vue-form/src/useField.tsxpackages/vue-form/src/useForm.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-form/src/useField.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/svelte-form/src/createForm.svelte.ts`:
- Line 4: The public API removed the createField method from SvelteFormApi (so
form.createField will be undefined for consumers); restore a compatibility
wrapper by adding a createField method back onto the object returned by
createForm (or the SvelteFormApi implementation) that delegates to the
standalone createField export, or re-export createField from the module so
existing form.createField calls still work; update the SvelteFormApi type to
include createField and ensure the method simply calls the imported createField
(preserving current behavior) while marking it as deprecated in comments.
🪄 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: 6a910576-b7f3-47db-98fe-f90543fe5829
📒 Files selected for processing (5)
packages/svelte-form/src/Field.sveltepackages/svelte-form/src/createForm.svelte.tspackages/svelte-form/src/createFormCreator.svelte.tspackages/vue-form/src/useField.tsxpackages/vue-form/src/useForm.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/svelte-form/src/createFormCreator.svelte.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/vue-form/src/useForm.tsx
- packages/vue-form/src/useField.tsx
|
I'm very suspicious about removing these changes. Because we have so many of these instances in our code it makes me think that we added this purposefully at some point, but I can't fathom why. |
crutchcorn
changed the title
Field.Field usageField.Field usage and useForm().useField()
crutchcorn
force-pushed
the
remove-dupe-field
branch
from
13bc04a to
898223f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/solid-form/src/createField.tsx`:
- Line 240: The return type advertises SolidFieldApi.Field but extendedApi
(created in createField and passed into makeFieldReactive) never gets a Field
property, so consumers see field().Field at compile-time but it's undefined at
runtime; fix by adding a proper Field implementation onto extendedApi before
passing it to makeFieldReactive (implement a FieldComponent matching
SolidFieldApi's Field signature and assign extendedApi.Field = <that
component>), or alternatively remove SolidFieldApi from the
createField/makeFieldReactive return type so the type matches runtime — update
the symbols extendedApi, createField, makeFieldReactive, and SolidFieldApi
accordingly to keep type and runtime consistent.
🪄 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: a745f6a1-a5d9-4889-ae57-7705bb6914db
📒 Files selected for processing (6)
packages/react-form/src/createFormHook.tsxpackages/solid-form/src/createField.tsxpackages/solid-form/src/createForm.tsxpackages/svelte-form/src/createForm.svelte.tspackages/svelte-form/src/index.tspackages/svelte-form/src/types.ts
💤 Files with no reviewable changes (1)
- packages/svelte-form/src/types.ts
✅ Files skipped from review due to trivial changes (1)
- packages/react-form/src/createFormHook.tsx
| > = api as never | ||
|
|
||
| extendedApi.Field = Field as never | ||
| const extendedApi: typeof api = api as never |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify mismatch: runtime assignment removed, but type still includes SolidFieldApi in createField path.
rg -n -C3 "const extendedApi: typeof api = api as never|extendedApi\\.Field\\s*=|function makeFieldReactive|SolidFieldApi<" packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 1119
🏁 Script executed:
# First, let's see the full SolidFieldApi interface definition
sed -n '23,52p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 1112
🏁 Script executed:
# Now let's see the full context around line 240 and how extendedApi is used after
sed -n '235,280p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 1033
🏁 Script executed:
# Check if extendedApi.Field is assigned anywhere in the file
rg -n "extendedApi\.Field\s*=" packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 39
🏁 Script executed:
# Also check the return type and return statement of makeFieldReactive
sed -n '150,160p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 258
🏁 Script executed:
# Get the complete return statement starting from line 250+
sed -n '260,300p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 1054
🏁 Script executed:
# Check all uses of extendedApi in the file
rg -n "extendedApi" packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 138
🏁 Script executed:
# Also, let's see the makeFieldReactive function's return statement
sed -n '155,180p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 774
🏁 Script executed:
# Get the createField function signature with return type
sed -n '167,210p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 1952
🏁 Script executed:
# Get the return type annotation of createField
sed -n '207,225p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 439
🏁 Script executed:
# Get more of the function signature to find the return type
sed -n '220,240p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 462
🏁 Script executed:
# Search for the return type annotation more precisely
rg -n "^\) " packages/solid-form/src/createField.tsx | head -5Repository: TanStack/form
Length of output: 73
🏁 Script executed:
# Also check the full createField signature with better context
sed -n '174,236p' packages/solid-form/src/createField.tsx | grep -n "=>"Repository: TanStack/form
Length of output: 94
🏁 Script executed:
# Get a cleaner view of the full createField function signature
awk '/^export function createField</,/^\) \{/' packages/solid-form/src/createField.tsx | tail -30Repository: TanStack/form
Length of output: 694
🏁 Script executed:
# Check if Field property is ever added anywhere in the createField function
sed -n '238,290p' packages/solid-form/src/createField.tsxRepository: TanStack/form
Length of output: 1183
Type contract advertises field().Field, but runtime never provides it.
createField passes extendedApi to makeFieldReactive without adding a Field property, yet the return type signature includes SolidFieldApi, which declares Field: FieldComponent<...>. This creates a compile-time-success / runtime-undefined trap: consumers can write field().Field and TypeScript accepts it, but the property is never defined at runtime.
The root cause is that extendedApi on line 240 is never enriched with the Field property before being passed to makeFieldReactive on line 288. Meanwhile, makeFieldReactive's return type contract (() => FieldApi<...> & SolidFieldApi<...>) still advertises Field via SolidFieldApi.
Either add the Field property to extendedApi before passing it to makeFieldReactive, or remove SolidFieldApi from the return type to match the actual runtime behavior.
🤖 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/solid-form/src/createField.tsx` at line 240, The return type
advertises SolidFieldApi.Field but extendedApi (created in createField and
passed into makeFieldReactive) never gets a Field property, so consumers see
field().Field at compile-time but it's undefined at runtime; fix by adding a
proper Field implementation onto extendedApi before passing it to
makeFieldReactive (implement a FieldComponent matching SolidFieldApi's Field
signature and assign extendedApi.Field = <that component>), or alternatively
remove SolidFieldApi from the createField/makeFieldReactive return type so the
type matches runtime — update the symbols extendedApi, createField,
makeFieldReactive, and SolidFieldApi accordingly to keep type and runtime
consistent.
LeCarbonator
left a comment
LeCarbonator
left a comment
There was a problem hiding this comment.
Looks good! Resolve the note since it may affect React 17/18. Otherwise, feel free to merge anytime.
This reverts commit e421837. # Conflicts: # packages/react-form/src/createFormHook.tsx
2 participants
This PR removes the ability to do:
And:
Which I think was errantly added, as I can't think of any reliable reason why we'd want to support this.
Summary by CodeRabbit
Breaking Changes
Chores