new: form and a11y package for Solid 2.0#926
Open
davedbase wants to merge 25 commits into
Open
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 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:
📝 WalkthroughWalkthroughThis PR introduces two new Solid.js primitive packages: ChangesA11Y Accessibility Primitives
Form Primitive Package
🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 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 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: 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
davedbase
changed the title
Member
Author
|
@coderabbitai can you please re-review this package |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/form/README.md (1)
17-23: ⚡ Quick winSpecify language for fenced code block.
The installation commands should specify
bashorshellas the language for proper syntax highlighting and better accessibility.📝 Suggested fix
-``` +```bash npm install `@solid-primitives/form` # or yarn add `@solid-primitives/form` # or pnpm add `@solid-primitives/form` ```🤖 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 17 - 23, Update the fenced code block that contains "npm install `@solid-primitives/form`" so it specifies a shell language for syntax highlighting (e.g., add ```bash or ```shell at the opening fence); keep the three install lines unchanged and only modify the opening fence to include the language token for proper highlighting and accessibility.Source: Linters/SAST tools
🤖 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/a11y/README.md`:
- Around line 15-21: Two unlabeled fenced code blocks (the install commands
block beginning with "npm install `@solid-primitives/a11y`" and the example block
starting "// No label registered, no aria-label") need explicit language
identifiers to satisfy markdownlint MD040; update the first fence to ```bash and
the second to ```txt so the snippets become fenced with those language tags.
Ensure only the opening backticks are modified (leave the content and closing
``` untouched) so the install snippet uses bash and the comment/example snippet
uses txt.
---
Nitpick comments:
In `@packages/form/README.md`:
- Around line 17-23: Update the fenced code block that contains "npm install
`@solid-primitives/form`" so it specifies a shell language for syntax highlighting
(e.g., add ```bash or ```shell at the opening fence); keep the three install
lines unchanged and only modify the opening fence to include the language token
for proper highlighting and accessibility.
🪄 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: 31ace794-014e-46bf-ae00-787f13b6fd63
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
.storybook/ui/form-control.tsx.storybook/ui/index.tspackage.jsonpackages/a11y/LICENSEpackages/a11y/README.mdpackages/a11y/package.jsonpackages/a11y/src/announce.tspackages/a11y/src/form-control.tspackages/a11y/src/index.tspackages/a11y/src/reduced-motion.tspackages/a11y/src/types.tspackages/a11y/stories/a11y.stories.tsxpackages/a11y/test/index.test.tsxpackages/a11y/test/server.test.tspackages/a11y/tsconfig.jsonpackages/form/LICENSEpackages/form/README.mdpackages/form/package.jsonpackages/form/src/form-control.tspackages/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
brenelz
approved these changes
Jun 14, 2026
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
2 participants
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.
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
New Features
Documentation
Tests