feat: add ability to get form type from Svelte#2159
Conversation
📝 WalkthroughWalkthroughThis PR introduces a ChangesgetFormType Form Prop Typing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 57953ed
☁️ 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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2159 +/- ##
==========================================
- Coverage 90.35% 82.66% -7.69%
==========================================
Files 38 8 -30
Lines 1752 75 -1677
Branches 444 4 -440
==========================================
- Hits 1583 62 -1521
+ Misses 149 12 -137
+ Partials 20 1 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/svelte-form/src/createFormCreator.svelte.ts (1)
270-315: ⚡ Quick winAdd a JSDoc warning that
getFormTypemust only be used in a type position.The function returns
undefined as neverat runtime, but its return type is non-nullable. TypeScript will not warn if a caller accidentally uses the result in a value position (e.g.,formType.AppField(...)), which would produce a silent runtimeTypeError. A JSDoc comment on the function makes the contract explicit.📝 Suggested JSDoc addition
+ /** + * **Type inference helper only — never use the return value at runtime.** + * + * Call this function and use `typeof <result>` to derive the strongly typed + * form prop for a child component. The runtime return value is always + * `undefined`; accessing any property on it will throw a `TypeError`. + * + * `@example` + * const formType = getFormType({ ...myFormOpts }) + * // Only valid in type position: + * const { form }: { form: typeof formType } = $props() + */ function getFormType<🤖 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/svelte-form/src/createFormCreator.svelte.ts` around lines 270 - 315, Add a JSDoc comment immediately above the getFormType function that clearly warns this helper is for TypeScript type positions only and must not be used at runtime (it returns `undefined as never`), e.g. a short paragraph stating "For type-only use: do not call or access the returned value at runtime — use only in type annotations/positions." Reference the function name getFormType and the fact that its return is `undefined as never` so readers know why it would throw if used as a value.docs/framework/svelte/guides/form-composition.md (1)
295-340: ⚡ Quick win"Reusing groups of fields" intentionally retains
ReturnType<typeof createAppForm<...>>— worth a brief callout.Right after introducing
getFormTypeas the preferred approach, this section uses the old pattern without explanation. Readers may assume the section was overlooked. A short note clarifying why theReturnTypealias is still appropriate here (there is no sharedformOptionsto pass togetFormType; the group deliberately accepts any form with the required shape) would prevent confusion.📝 Suggested clarifying note
## Reusing groups of fields in multiple forms Sometimes, a pair of fields are so closely related that it makes sense to group and reuse them — like the password example listed in the [linked fields guide](./linked-fields.md). Instead of repeating this logic across multiple forms, you can create reusable field group components. +> Unlike the `getFormType` approach, which works with a specific shared `formOptions` object, reusable field groups often accept *any* form that satisfies a minimum shape. For that case, `ReturnType<typeof createAppForm<MinimalShape>>` is the right tool: it lets you declare the narrowed type inline without needing a dedicated `formOptions`. + > Unlike form-level components, validators in field groups cannot be strictly typed and could be any value.🤖 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 `@docs/framework/svelte/guides/form-composition.md` around lines 295 - 340, Add a short clarifying note next to the PasswordFields example explaining why the older ReturnType<typeof createAppForm<PasswordFields>> pattern is intentionally used instead of getFormType: state that there are no shared formOptions to pass to getFormType and this field-group intentionally accepts any form matching the PasswordFields shape, so a simple ReturnType alias (AppForm) is appropriate; reference the example names createAppForm, ReturnType, getFormType, PasswordFields, and AppForm so readers understand the rationale.
🤖 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 `@docs/framework/svelte/guides/form-composition.md`:
- Around line 295-340: Add a short clarifying note next to the PasswordFields
example explaining why the older ReturnType<typeof
createAppForm<PasswordFields>> pattern is intentionally used instead of
getFormType: state that there are no shared formOptions to pass to getFormType
and this field-group intentionally accepts any form matching the PasswordFields
shape, so a simple ReturnType alias (AppForm) is appropriate; reference the
example names createAppForm, ReturnType, getFormType, PasswordFields, and
AppForm so readers understand the rationale.
In `@packages/svelte-form/src/createFormCreator.svelte.ts`:
- Around line 270-315: Add a JSDoc comment immediately above the getFormType
function that clearly warns this helper is for TypeScript type positions only
and must not be used at runtime (it returns `undefined as never`), e.g. a short
paragraph stating "For type-only use: do not call or access the returned value
at runtime — use only in type annotations/positions." Reference the function
name getFormType and the fact that its return is `undefined as never` so readers
know why it would throw if used as a value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b07ac358-2fd7-48d6-8bc1-264ab23d160f
📒 Files selected for processing (5)
docs/framework/svelte/guides/form-composition.mdexamples/svelte/large-form/src/features/people/address-fields.svelteexamples/svelte/large-form/src/features/people/emergency-contact.svelteexamples/svelte/large-form/src/runes/form.tspackages/svelte-form/src/createFormCreator.svelte.ts
This PR adds the ability to get the type of
formto be passed via props:Before
After
Summary by CodeRabbit
Documentation
getFormTypehelper for simplified, type-safe derivation of form props in child components, eliminating manual type construction patterns.Refactor