Open
Conversation
|
@Huliiiiii is attempting to deploy a commit to the Open Circle Team on Vercel. A member of the Team first needs to authorize it. |
dosubot
bot
added
the
size:L
📝 WalkthroughWalkthroughThe FormProps Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Author
|
@fabian-hiller Could you please check whether there are any errors? This should be safe. I've manually checked it, and there were no issues in any case. |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Form component onSubmit prop typing (and corresponding docs) to improve TypeScript inference by using a single SubmitEventHandler<TSchema> type instead of a SubmitHandler | SubmitEventHandler union.
Changes:
- Changed
onSubmit/onsubmitprop types across framework Form components toSubmitEventHandler<TSchema>(and Qwik toQRL<SubmitEventHandler<TSchema>>). - Updated website API property definitions to match the new
onSubmittype shape. - Removed now-unused
SubmitHandlerimports from framework Form components.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| website/src/routes/(docs)/vue/api/(components)/Form/properties.ts | Docs: onSubmit now documented as SubmitEventHandler<TSchema> |
| website/src/routes/(docs)/svelte/api/(components)/Form/properties.ts | Docs: onsubmit now documented as SubmitEventHandler<TSchema> |
| website/src/routes/(docs)/solid/api/(components)/Form/properties.ts | Docs: onSubmit now documented as SubmitEventHandler<TSchema> |
| website/src/routes/(docs)/react/api/(components)/Form/properties.ts | Docs: onSubmit now documented as SubmitEventHandler<TSchema> |
| website/src/routes/(docs)/qwik/api/(components)/Form/properties.ts | Docs: onSubmit$ now documented as QRL<SubmitEventHandler<TSchema>> |
| website/src/routes/(docs)/preact/api/(components)/Form/properties.ts | Docs: onSubmit now documented as SubmitEventHandler<TSchema> |
| frameworks/vue/src/components/Form/Form.vue | Vue FormProps onSubmit narrowed to SubmitEventHandler<TSchema> |
| frameworks/svelte/src/components/Form/Form.svelte | Svelte FormProps onsubmit narrowed to SubmitEventHandler<TSchema> |
| frameworks/solid/src/components/Form/Form.tsx | Solid FormProps onSubmit narrowed to SubmitEventHandler<TSchema> |
| frameworks/react/src/components/Form/Form.tsx | React FormProps onSubmit narrowed to SubmitEventHandler<TSchema> |
| frameworks/qwik/src/components/Form/Form.tsx | Qwik FormProps onSubmit$ narrowed to QRL<SubmitEventHandler<TSchema>> |
| frameworks/preact/src/components/Form/Form.tsx | Preact FormProps onSubmit narrowed to SubmitEventHandler<TSchema> |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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 (5)
frameworks/solid/src/components/Form/Form.tsx (1)
13-29: Consider usinginterfaceforFormProps.Per coding guidelines,
interfaceis preferred overtypefor defining object shapes.♻️ Suggested refactor
-export type FormProps<TSchema extends Schema = Schema> = Omit< - JSX.FormHTMLAttributes<HTMLFormElement>, - 'onSubmit' | 'novalidate' | 'noValidate' -> & { +export interface FormProps<TSchema extends Schema = Schema> + extends Omit< + JSX.FormHTMLAttributes<HTMLFormElement>, + 'onSubmit' | 'novalidate' | 'noValidate' + > { /** * The form store instance. */ readonly of: FormStore<TSchema>; /** * The child elements to render within the form. */ readonly children: JSX.Element; /** * The submit handler called when the form is submitted and validation succeeds. */ readonly onSubmit: SubmitEventHandler<TSchema>; -}; +}As per coding guidelines: "Prefer
interfaceovertypefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frameworks/solid/src/components/Form/Form.tsx` around lines 13 - 29, The declaration for FormProps should be an interface rather than a type alias; replace the "export type FormProps<TSchema extends Schema = Schema> = Omit<JSX.FormHTMLAttributes<HTMLFormElement>, 'onSubmit' | 'novalidate' | 'noValidate'> & { ... }" with an exported interface named FormProps that extends Omit<JSX.FormHTMLAttributes<HTMLFormElement>, 'onSubmit' | 'novalidate' | 'noValidate'> and preserves the generic parameter TSchema (with its default), readonly properties (of: FormStore<TSchema>, children: JSX.Element) and the onSubmit: SubmitEventHandler<TSchema> member so the shape and types remain identical.frameworks/react/src/components/Form/Form.tsx (1)
13-25: Consider usinginterfaceforFormProps.Per coding guidelines,
interfaceis preferred overtypefor defining object shapes in TypeScript. SinceFormPropsdefines an object shape, consider converting it.♻️ Suggested refactor
-export type FormProps<TSchema extends Schema = Schema> = Omit< - FormHTMLAttributes<HTMLFormElement>, - 'onSubmit' | 'novalidate' | 'noValidate' -> & { +export interface FormProps<TSchema extends Schema = Schema> + extends Omit< + FormHTMLAttributes<HTMLFormElement>, + 'onSubmit' | 'novalidate' | 'noValidate' + > { /** * The form store instance. */ readonly of: FormStore<TSchema>; /** * The submit handler called when the form is submitted and validation succeeds. */ readonly onSubmit: SubmitEventHandler<TSchema>; -}; +}As per coding guidelines: "Prefer
interfaceovertypefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frameworks/react/src/components/Form/Form.tsx` around lines 13 - 25, Convert the FormProps type alias into an interface while preserving its generic parameter and the omitted form attributes; specifically, replace "export type FormProps<TSchema extends Schema = Schema> = Omit<...> & { ... }" with an "export interface FormProps<TSchema extends Schema = Schema> extends Omit<FormHTMLAttributes<HTMLFormElement>, 'onSubmit' | 'novalidate' | 'noValidate'> { readonly of: FormStore<TSchema>; readonly onSubmit: SubmitEventHandler<TSchema>; }" so the shape, readonly modifiers, generic constraint, and referenced symbols (FormProps, Schema, FormStore, SubmitEventHandler) remain identical but use interface syntax.frameworks/svelte/src/components/Form/Form.svelte (1)
15-31: Consider usinginterfaceforFormProps.Per coding guidelines,
interfaceis preferred overtypefor defining object shapes.♻️ Suggested refactor
- export type FormProps<TSchema extends Schema = Schema> = Omit< - HTMLFormAttributes, - 'on:submit' | 'onsubmit' | 'novalidate' - > & { + export interface FormProps<TSchema extends Schema = Schema> + extends Omit< + HTMLFormAttributes, + 'on:submit' | 'onsubmit' | 'novalidate' + > { /** * The form store instance. */ of: FormStore<TSchema>; /** * The submit handler called when the form is submitted and validation succeeds. */ onsubmit: SubmitEventHandler<TSchema>; /** * The child elements to render within the form. */ children: Snippet; - }; + }As per coding guidelines: "Prefer
interfaceovertypefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frameworks/svelte/src/components/Form/Form.svelte` around lines 15 - 31, Replace the exported FormProps type alias with an exported interface named FormProps that keeps the generic parameter TSchema extends Schema and extends the same Omit<HTMLFormAttributes, 'on:submit' | 'onsubmit' | 'novalidate'>, and include the same properties (of: FormStore<TSchema>, onsubmit: SubmitEventHandler<TSchema>, children: Snippet) so all references to FormProps, FormStore, SubmitEventHandler continue to work; essentially convert the "export type FormProps<...> = ..." declaration into an "export interface FormProps<...> extends ..." declaration preserving the exact shape and generics.frameworks/preact/src/components/Form/Form.tsx (1)
13-25: Consider usinginterfaceforFormProps.Per coding guidelines,
interfaceis preferred overtypefor defining object shapes.♻️ Suggested refactor
-export type FormProps<TSchema extends Schema = Schema> = Omit< - JSX.FormHTMLAttributes<HTMLFormElement>, - 'onSubmit' | 'novalidate' | 'noValidate' -> & { +export interface FormProps<TSchema extends Schema = Schema> + extends Omit< + JSX.FormHTMLAttributes<HTMLFormElement>, + 'onSubmit' | 'novalidate' | 'noValidate' + > { /** * The form store instance. */ readonly of: FormStore<TSchema>; /** * The submit handler called when the form is submitted and validation succeeds. */ readonly onSubmit: SubmitEventHandler<TSchema>; -}; +}As per coding guidelines: "Prefer
interfaceovertypefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frameworks/preact/src/components/Form/Form.tsx` around lines 13 - 25, Replace the FormProps type alias with an equivalent interface to follow the coding guideline; keep the same shape and generics (TSchema extends Schema) and preserve the inheritance from Omit<JSX.FormHTMLAttributes<HTMLFormElement>, 'onSubmit' | 'novalidate' | 'noValidate'> as well as the readonly properties of of: FormStore<TSchema> and onSubmit: SubmitEventHandler<TSchema>; update any exports/usages to reference the new FormProps interface name but leave FormStore, SubmitEventHandler, and Schema identifiers unchanged.frameworks/qwik/src/components/Form/Form.tsx (1)
14-26: Prefer aninterfacefor exported props.
FormPropsis an exported object shape, and this block is already being touched.♻️ Suggested refactor
-export type FormProps<TSchema extends Schema = Schema> = Omit< - PropsOf<'form'>, - 'onSubmit$' | 'noValidate' -> & { +export interface FormProps<TSchema extends Schema = Schema> + extends Omit<PropsOf<'form'>, 'onSubmit$' | 'noValidate'> { /** * The form store instance. */ readonly of: FormStore<TSchema>; /** * The submit handler called when the form is submitted and validation succeeds. */ readonly onSubmit$: QRL<SubmitEventHandler<TSchema>>; -}; +}As per coding guidelines, "Prefer
interfaceovertypefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frameworks/qwik/src/components/Form/Form.tsx` around lines 14 - 26, Replace the exported type alias FormProps<TSchema extends Schema = Schema> with an exported interface declaration so the prop shape uses an interface; declare "export interface FormProps<TSchema extends Schema = Schema> extends Omit<PropsOf<'form'>, 'onSubmit$' | 'noValidate'> { readonly of: FormStore<TSchema>; readonly onSubmit$: QRL<SubmitEventHandler<TSchema>>; }" preserving the same generic constraint, Omit, readonly modifiers, and member names (FormProps, TSchema, Schema, PropsOf<'form'>, FormStore, QRL, SubmitEventHandler) so the public API and types remain identical while switching to an interface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frameworks/react/src/components/Form/Form.tsx`:
- Line 24: Add a migration doc entry explaining the breaking change where Form's
onSubmit prop type was tightened from SubmitHandler<TSchema> |
SubmitEventHandler<TSchema> to only SubmitEventHandler<TSchema>; state that
handlers must now accept the event parameter (the form submit event) so callers
should change signatures from (data) => { ... } to (data, event) => { ... };
include a short rationale referencing the Form component and its onSubmit prop
and mention that `@formisch/methods`' handleSubmit still accepts both
SubmitHandler and SubmitEventHandler for backward compatibility; provide a small
migration snippet suggestion and a note that this change was introduced in
v0.4.0 (CHANGELOG: "Update `Form` component submit handler types").
---
Nitpick comments:
In `@frameworks/preact/src/components/Form/Form.tsx`:
- Around line 13-25: Replace the FormProps type alias with an equivalent
interface to follow the coding guideline; keep the same shape and generics
(TSchema extends Schema) and preserve the inheritance from
Omit<JSX.FormHTMLAttributes<HTMLFormElement>, 'onSubmit' | 'novalidate' |
'noValidate'> as well as the readonly properties of of: FormStore<TSchema> and
onSubmit: SubmitEventHandler<TSchema>; update any exports/usages to reference
the new FormProps interface name but leave FormStore, SubmitEventHandler, and
Schema identifiers unchanged.
In `@frameworks/qwik/src/components/Form/Form.tsx`:
- Around line 14-26: Replace the exported type alias FormProps<TSchema extends
Schema = Schema> with an exported interface declaration so the prop shape uses
an interface; declare "export interface FormProps<TSchema extends Schema =
Schema> extends Omit<PropsOf<'form'>, 'onSubmit$' | 'noValidate'> { readonly of:
FormStore<TSchema>; readonly onSubmit$: QRL<SubmitEventHandler<TSchema>>; }"
preserving the same generic constraint, Omit, readonly modifiers, and member
names (FormProps, TSchema, Schema, PropsOf<'form'>, FormStore, QRL,
SubmitEventHandler) so the public API and types remain identical while switching
to an interface.
In `@frameworks/react/src/components/Form/Form.tsx`:
- Around line 13-25: Convert the FormProps type alias into an interface while
preserving its generic parameter and the omitted form attributes; specifically,
replace "export type FormProps<TSchema extends Schema = Schema> = Omit<...> & {
... }" with an "export interface FormProps<TSchema extends Schema = Schema>
extends Omit<FormHTMLAttributes<HTMLFormElement>, 'onSubmit' | 'novalidate' |
'noValidate'> { readonly of: FormStore<TSchema>; readonly onSubmit:
SubmitEventHandler<TSchema>; }" so the shape, readonly modifiers, generic
constraint, and referenced symbols (FormProps, Schema, FormStore,
SubmitEventHandler) remain identical but use interface syntax.
In `@frameworks/solid/src/components/Form/Form.tsx`:
- Around line 13-29: The declaration for FormProps should be an interface rather
than a type alias; replace the "export type FormProps<TSchema extends Schema =
Schema> = Omit<JSX.FormHTMLAttributes<HTMLFormElement>, 'onSubmit' |
'novalidate' | 'noValidate'> & { ... }" with an exported interface named
FormProps that extends Omit<JSX.FormHTMLAttributes<HTMLFormElement>, 'onSubmit'
| 'novalidate' | 'noValidate'> and preserves the generic parameter TSchema (with
its default), readonly properties (of: FormStore<TSchema>, children:
JSX.Element) and the onSubmit: SubmitEventHandler<TSchema> member so the shape
and types remain identical.
In `@frameworks/svelte/src/components/Form/Form.svelte`:
- Around line 15-31: Replace the exported FormProps type alias with an exported
interface named FormProps that keeps the generic parameter TSchema extends
Schema and extends the same Omit<HTMLFormAttributes, 'on:submit' | 'onsubmit' |
'novalidate'>, and include the same properties (of: FormStore<TSchema>,
onsubmit: SubmitEventHandler<TSchema>, children: Snippet) so all references to
FormProps, FormStore, SubmitEventHandler continue to work; essentially convert
the "export type FormProps<...> = ..." declaration into an "export interface
FormProps<...> extends ..." declaration preserving the exact shape and generics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 191ddf9b-d264-48c3-87d4-731b39faffce
📒 Files selected for processing (12)
frameworks/preact/src/components/Form/Form.tsxframeworks/qwik/src/components/Form/Form.tsxframeworks/react/src/components/Form/Form.tsxframeworks/solid/src/components/Form/Form.tsxframeworks/svelte/src/components/Form/Form.svelteframeworks/vue/src/components/Form/Form.vuewebsite/src/routes/(docs)/preact/api/(components)/Form/properties.tswebsite/src/routes/(docs)/qwik/api/(components)/Form/properties.tswebsite/src/routes/(docs)/react/api/(components)/Form/properties.tswebsite/src/routes/(docs)/solid/api/(components)/Form/properties.tswebsite/src/routes/(docs)/svelte/api/(components)/Form/properties.tswebsite/src/routes/(docs)/vue/api/(components)/Form/properties.ts
Member
|
That's actually a good idea! Thanks for the PR! |
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
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.
Alternative to #64
Summary by CodeRabbit
Breaking Changes
onSubmitproperty type has been simplified across all frameworks (Preact, React, Qwik, Solid, Svelte, Vue). The property now strictly accepts a single handler type.Documentation
onSubmitproperty changes across all framework implementations.