fix(sink): replace date input with calendar picker in next-form to fix useActionState serialization crash#10215
fix(sink): replace date input with calendar picker in next-form to fix useActionState serialization crash#10215Lukas1098 wants to merge 2 commits intoshadcn-ui:mainfrom
Conversation
…x useActionState serialization crash
|
@Lukas1098 is attempting to deploy a commit to the shadcn-pro Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Updates the /sink/next-form example to avoid a useActionState Date serialization crash by moving start-date selection to a client-side calendar picker, and aligns server-side validation with the UI constraint for team size.
Changes:
- Replace the native
type="date"input with aCalendar+Popoverpicker and hidden input submission. - Update
teamSizeZod validation max from 10 to 50 to match the input’smax="50"constraint.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/v4/app/(internal)/sink/(pages)/next-form/example-form.tsx | Swaps date input to a calendar popover picker and submits via a hidden input. |
| apps/v4/app/(internal)/sink/(pages)/schema.ts | Adjusts teamSize validation to allow up to 50. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <input type="hidden" name="startDate" value={date ? date.toISOString() : ""} /> | ||
| <Popover> | ||
| <PopoverTrigger asChild> | ||
| <Button variant="outline" className="w-full justify-start" disabled={pending}> |
There was a problem hiding this comment.
The Popover trigger uses the shared Button component without an explicit type. Since Button renders a plain <button> and doesn’t set a default type, this will default to type="submit" inside the form, so clicking the date picker trigger can submit the form unexpectedly. Set type="button" on this trigger button.
| <Button variant="outline" className="w-full justify-start" disabled={pending}> | |
| <Button type="button" variant="outline" className="w-full justify-start" disabled={pending}> |
| <FieldDescription> | ||
| Choose when your subscription should start | ||
| </FieldDescription> | ||
| <input type="hidden" name="startDate" value={date ? date.toISOString() : ""} /> |
There was a problem hiding this comment.
value={date.toISOString()} can shift the selected calendar day for users in time zones ahead of UTC (local midnight becomes the previous UTC day), which can cause server-side validation (e.g. “in the past”) to fail for a date the user picked. Prefer submitting a date-only string (e.g. yyyy-MM-dd) or otherwise normalizing the value so the server interprets the same calendar day the user selected.
| <input type="hidden" name="startDate" value={date ? date.toISOString() : ""} /> | |
| <input type="hidden" name="startDate" value={date ? format(date, "yyyy-MM-dd") : ""} /> |
| <input type="hidden" name="startDate" value={date ? date.toISOString() : ""} /> | ||
| <Popover> | ||
| <PopoverTrigger asChild> | ||
| <Button variant="outline" className="w-full justify-start" disabled={pending}> |
There was a problem hiding this comment.
<FieldLabel htmlFor="startDate"> no longer points at a visible form control (the only startDate element is a hidden input without an id). This breaks label-to-control association for screen readers and click-to-focus behavior. Consider putting id="startDate" on the trigger button (and/or wiring up aria-invalid/description) so the label targets the interactive control.
| <Button variant="outline" className="w-full justify-start" disabled={pending}> | |
| <Button | |
| id="startDate" | |
| variant="outline" | |
| className="w-full justify-start" | |
| disabled={pending} | |
| aria-invalid={!!formState.errors?.startDate?.length} | |
| > |
mahdirajaee
left a comment
There was a problem hiding this comment.
Solid fix -- useActionState requires all form state to be serializable, and a Date object from <input type="date"> returning through the action response would indeed crash serialization. Replacing it with a hidden input carrying date.toISOString() string is the right approach.
A couple of things worth considering: the date state initializes to new Date() but is completely independent of formState.values.startDate, meaning the calendar will always open to "today" regardless of what the server-side form state holds. After a validation error round-trip, the calendar won't reflect the previously submitted value. It would be more robust to initialize from formState.values.startDate and keep them in sync.
Also, when date is undefined (user clears selection), the hidden input sends value="" -- but the schema still expects a valid date, so this will produce a confusing zod parse error rather than the existing "Please select a start date" message. The removed <FieldDescription> text ("Choose when your subscription should start") was useful UX context and probably shouldn't have been dropped.
The unrelated teamSize max bump from 10 to 50 in schema.ts should be called out in the PR description or split into its own commit.
Updated based on feedback:
Regarding the |
What
Fixes #10214
Replaces the native
<Input type="date">in the/sink/next-formexample with aCalendar+Popoverpicker using local React state.Why
The original implementation crashed on submit because
formState.values.startDateis returned as aDateobject from the server action. AfteruseActionStateserializes and deserializes the return value across the server/client boundary,startDatearrives on the client as astring. Calling.toISOString()on it indefaultValuethrows a client-side exception.Changes
example-form.tsx: replaced<Input type="date">withCalendar+Popoverpicker and a hidden input to submit the valueschema.ts: fixedteamSizemax validation from 10 to 50 to match the input constraint