new: form package for Solid 2.0#926
Conversation
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/form/README.md`:
- Around line 100-115: The README's public API table and method list omit the
public method setValues, which is declared on FormReturn and implemented in
form.ts; add an entry for setValues to the API table and the definition block
describing its signature and behavior (e.g., setValues(newValues?:
Partial<Values>) => void, updates current values and optionally resets
baselines), referencing the same terminology as FormReturn and the
implementation in form.ts so documentation and code remain consistent.
In `@packages/form/src/form.ts`:
- Line 100: The SSR branch currently returns a blank FormData via formData: ()
=> new FormData(), causing mismatch with client behavior which uses
toFormData(values()); change the SSR stub so formData() returns the serialized
initial values by calling toFormData(values()) (ensure toFormData is
imported/available and values() is referenced the same way as in the client
branch) so server consumers receive the same payload as the hydrated client.
🪄 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 Plus
Run ID: e75dd6f3-96a8-4c3e-bca6-67e50d15352b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
packages/form/README.mdpackages/form/package.jsonpackages/form/src/form.tspackages/form/src/index.tspackages/form/src/types.tspackages/form/stories/form.stories.tsxpackages/form/test/index.test.tspackages/form/test/server.test.tspackages/form/tsconfig.json
| | Member | Type | Description | | ||
| |--------|------|-------------| | ||
| | `dirty()` | `Accessor<boolean>` | `true` when any field differs from its initial value | | ||
| | `valid()` | `Accessor<boolean>` | `true` when all fields pass validation and no async validators are pending | | ||
| | `pending()` | `Accessor<boolean>` | `true` while any field has an async validator in flight | | ||
| | `submitting()` | `Accessor<boolean>` | `true` while `onSubmit` is in flight | | ||
| | `submitted()` | `Accessor<boolean>` | `true` after the first submit attempt; reset by `reset()` | | ||
| | `values()` | `Accessor<Values>` | Plain object of all current field values | | ||
| | `errors()` | `Accessor<Partial<Record<...>>>` | Object containing only **field-level** errors (always reflects true validity, regardless of `validateOn`). Cross-field errors registered via `validate()` are not included — access those through the accessor each `validate()` call returns. | | ||
| | `bind(name)` | Ref directive factory | Wires an `<input>` or `<select>` to the named field (see below) | | ||
| | `ref` | Ref callback | Attaches submit handling to a `<form>` element | | ||
| | `validate(fn)` | Cross-field rule | Registers a form-level validation rule (see below) | | ||
| | `setError(name, msg)` | `(name, error: string \| null) => void` | Inject an external error on a named field (e.g. server-side validation). Cleared automatically when that field's value changes. | | ||
| | `formData()` | `() => FormData` | Snapshot of current field values as a `FormData` instance | | ||
| | `reset(newValues?)` | `(newValues?: Partial<Values>) => void` | Reset all fields. If `newValues` is provided, the named fields adopt those as their new baseline (useful for edit forms after a successful save). | | ||
| | `submit()` | `() => Promise<void>` | Programmatically trigger submission | |
There was a problem hiding this comment.
Document setValues in the public API section.
setValues is part of FormReturn in packages/form/src/types.ts and is implemented in packages/form/src/form.ts, but it is missing from both the form-level API table and the definition block here. That leaves a shipped public method undocumented.
Also applies to: 413-429
🤖 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/form/README.md` around lines 100 - 115, The README's public API
table and method list omit the public method setValues, which is declared on
FormReturn and implemented in form.ts; add an entry for setValues to the API
table and the definition block describing its signature and behavior (e.g.,
setValues(newValues?: Partial<Values>) => void, updates current values and
optionally resets baselines), referencing the same terminology as FormReturn and
the implementation in form.ts so documentation and code remain consistent.
| validate: () => () => null, | ||
| setValues: () => void 0, | ||
| setError: () => void 0, | ||
| formData: () => new FormData(), |
There was a problem hiding this comment.
Keep formData() consistent in the SSR branch.
The server stub returns new FormData() even though values() still exposes the configured initial values and the client branch serializes those values through toFormData(values()). Any SSR consumer calling form.formData() gets a different payload than the hydrated client.
Suggested fix
- formData: () => new FormData(),
+ formData: () => toFormData(initialValues),📝 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.
| formData: () => new FormData(), | |
| formData: () => toFormData(initialValues), |
🤖 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/form/src/form.ts` at line 100, The SSR branch currently returns a
blank FormData via formData: () => new FormData(), causing mismatch with client
behavior which uses toFormData(values()); change the SSR stub so formData()
returns the serialized initial values by calling toFormData(values()) (ensure
toFormData is imported/available and values() is referenced the same way as in
the client branch) so server consumers receive the same payload as the hydrated
client.
Summary
createForm<C>andtoFormDatafor Solid 2.0value,error,touched,pending), form-level signals (dirty,valid,submitting,submitted), and DOM binding via the 2.0 two-phase ref directive pattern(value) => string | null | Promise<string | null>functions — no adapter needed for Zod, Valibot, Arktype, etc.validateOn: "change" | "blur" | "submit"controls error display timing, per field or form-wideform.validate(fn); server-side errors viaform.setError(name, msg)Implementation notes (fixes applied during review)
Bug fix — async validators fired 3× per keystroke
The original architecture called each validator from three code paths per value change: once from the
_rawErrormemo, once from the effect's sync-check, and once from the effect's collection loop. For async validators making network requests, this meant 3 API calls per keystroke with only the last result used.Fixed by pre-classifying validators at setup time (one probe call each with
fc.initial): sync validators go into a lazysyncMemothat is the only reactive subscriber tovalue()and never touches async validators; async validators go intoasyncFns. The initial async Promises from classification are saved and reused on the first effect run if the value hasn't changed, avoiding a second API call. When async settles andasyncError()changes,_rawErrorrecomputes by reading signals only — no validator is called. Result: async validators fire once per value change.toFormData—falsenow omittedPreviously
falsewas coerced to the string"false", which doesn't match HTML's native behavior (unchecked checkboxes are absent from form payloads).falseis now omitted alongsidenullandundefined.reset(newValues?)overloadAccepts an optional partial values object. Named fields adopt the new values as their dirty baseline, making edit-form patterns possible without fighting the initial-value comparison. A version counter forces
dirtyto recompute even when the field value and new baseline are identical (no-op signal write wouldn't invalidate the memo otherwise).setError/field.setErrorInjects an external (server-side) error into the reactive graph. Appears in
field.error(),form.errors(), and gatesform.valid(). Cleared automatically when the user edits the field (setValuecalls the wrapped setter which clears it).Documentation fixes
bindwas missing| HTMLTextAreaElementvalidate()note incorrectly said "beforeform.valid()is first read" — the version counter handles late registrationerrors()table row now documents the exclusion of cross-fieldvalidate()errorsSummary by CodeRabbit
Release Notes
createFormprimitive for building reactive forms with integrated validation.