fix(Form): use FormState type to support nullable/custom initial state values#6553
fix(Form): use FormState type to support nullable/custom initial state values#6553rdjanuar wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAdds a new exported generic type FormState Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 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
🤖 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 `@src/runtime/types/form.ts`:
- Around line 37-39: The FormState mapped type currently widens every field to
unknown because of the `| unknown`; replace that with a proper optional mapping
to preserve per-field types: make FormState use Partial<InferInput<S>> or `{ [K
in keyof InferInput<S>]?: InferInput<S>[K] }` (remove `| unknown`) so fields
keep their schema types; reference symbols: FormState, FormSchema, InferInput.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00db3156-0877-4c9f-9026-9f5a5ef98743
📒 Files selected for processing (2)
src/runtime/components/Form.vuesrc/runtime/types/form.ts
commit: |
rdjanuar
force-pushed
the
fix/missmatch-type-state-form
branch
from
0a2a8e1 to
157f92b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/runtime/types/form.ts (1)
37-39: ⚡ Quick winAdd JSDoc documentation explaining the type safety tradeoff.
The
FormStatetype intentionally usesanyto support runtime state values that differ from schema input types. This significant design decision should be documented with a JSDoc comment explaining:
- Why
anyis used (runtime values like""for number fields,File | nullfor file fields)- The type safety tradeoff (no compile-time checking for field values)
- Reference to the issues it resolves (
#5204,#6309)📝 Suggested JSDoc documentation
+/** + * Form state type that accepts nullable, mixed, or custom initial values + * differing from schema input types. + * + * Uses `any` for field values to support common runtime patterns where initial + * state differs from schema (e.g., `File | null` for file inputs, `number | ""` + * for empty numeric inputs). Schema validation occurs at runtime. + * + * `@example` + * // Schema expects { age: number }, but state can initialize as: + * const state: FormState<MySchema> = { age: "" } // empty input + * + * `@see` https://github.com/nuxt/ui/issues/5204 + * `@see` https://github.com/nuxt/ui/issues/6309 + */ export type FormState<S extends FormSchema> = { [K in keyof InferInput<S>]?: any }🤖 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 `@src/runtime/types/form.ts` around lines 37 - 39, Add a JSDoc block above the FormState type explaining that FormState<S extends FormSchema> intentionally uses `any` for values (e.g., string "" for numeric inputs, File | null for file inputs) to support runtime representations that differ from schema input types, note the tradeoff that this disables compile-time value checking for field values, and reference issues `#5204` and `#6309`; mention related types FormSchema and InferInput to clarify context.
🤖 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 `@src/runtime/types/form.ts`:
- Around line 37-39: The FormState type currently uses `any` which removes
compile-time safety; change the value type to `unknown` in the FormState
definition so it reads as {[K in keyof InferInput<S>]?: unknown} (referencing
FormState, FormSchema and InferInput) and then update any downstream usages
(e.g., Form.vue validate callbacks and custom validators) to perform explicit
type narrowing or assertions when reading fields to satisfy the new constraint.
---
Nitpick comments:
In `@src/runtime/types/form.ts`:
- Around line 37-39: Add a JSDoc block above the FormState type explaining that
FormState<S extends FormSchema> intentionally uses `any` for values (e.g.,
string "" for numeric inputs, File | null for file inputs) to support runtime
representations that differ from schema input types, note the tradeoff that this
disables compile-time value checking for field values, and reference issues
`#5204` and `#6309`; mention related types FormSchema and InferInput to clarify
context.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1436db9c-402b-4627-afd7-93b917cbef87
📒 Files selected for processing (1)
src/runtime/types/form.ts
| export type FormState<S extends FormSchema> = { | ||
| [K in keyof InferInput<S>]?: any | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Consider unknown instead of any to preserve minimal type safety.
While any achieves the PR objective of accepting runtime values that differ from schema types (e.g., File | null, number | ""), it completely removes compile-time type checking for field values. Developers lose autocomplete, IntelliSense, and protection against typos or type errors.
Using unknown instead would still allow any runtime value but require explicit type narrowing or assertions before use, providing a safer middle ground.
🔒 Alternative using `unknown` for safer typing
export type FormState<S extends FormSchema> = {
- [K in keyof InferInput<S>]?: any
+ [K in keyof InferInput<S>]?: unknown
}Note: This change would require downstream consumers (Form.vue validate callbacks, custom validation functions) to use type assertions or guards when accessing state fields. Evaluate whether the added safety justifies the ergonomic cost.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type FormState<S extends FormSchema> = { | |
| [K in keyof InferInput<S>]?: any | |
| } | |
| export type FormState<S extends FormSchema> = { | |
| [K in keyof InferInput<S>]?: unknown | |
| } |
🤖 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 `@src/runtime/types/form.ts` around lines 37 - 39, The FormState type currently
uses `any` which removes compile-time safety; change the value type to `unknown`
in the FormState definition so it reads as {[K in keyof InferInput<S>]?:
unknown} (referencing FormState, FormSchema and InferInput) and then update any
downstream usages (e.g., Form.vue validate callbacks and custom validators) to
perform explicit type narrowing or assertions when reading fields to satisfy the
new constraint.
1 participant
🔗 Linked issue
Resolves #5204, #6309
❓ Type of change
📝 Description
The
statestrictly inferred all field types directly from the schema (based onInferInput<S>).While this seemed ideal, it caused significant friction because HTML/DOM inputs naturally handle unvalidated string states (like binding
""to an<input type="number">). Developers were forced to either cast their reactive state toanyor write overly complex schemas (e.g.,z.number().or(z.string()).pipe(...)) just to satisfy TypeScript before the data was actually submitted and coerced.📝 Checklist