Form Groups#2128
Open
crutchcorn wants to merge 126 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds FormGroup support across core: new FormGroupApi class and types, FieldApi onGroupSubmit hook and group-aware validate flow, FormApi validateOpts and group meta derivation, utils helper isFieldInGroup, tests, docs, and multi-framework example apps. ChangesForm Groups Feature
Sequence DiagramsequenceDiagram
participant Caller
participant GroupApi as FormGroupApi
participant FieldApis as FieldApi Instances
participant FormApi
Caller->>GroupApi: handleSubmit()
GroupApi->>GroupApi: reset flags, increment submissionAttempts, mark related fields touched, set isSubmitting = true
GroupApi->>FieldApis: validate each field for 'submit'
FieldApis-->>GroupApi: field validity
alt any group field invalid
GroupApi->>GroupApi: set isSubmitting = false
GroupApi->>GroupApi: invoke onGroupSubmitInvalid
GroupApi-->>Caller: early return
else group fields valid
GroupApi->>FormApi: validate('submit', { dontUpdateFormErrorMap, filterFieldNames })
FormApi-->>GroupApi: form validation result
alt form invalid for those fields
GroupApi->>GroupApi: set isSubmitting = false
GroupApi->>GroupApi: invoke onGroupSubmitInvalid
GroupApi-->>Caller: early return
else all valid
GroupApi->>FieldApis: trigger onGroupSubmit listeners on fields
GroupApi->>GroupApi: invoke options.listeners?.onSubmit and options.onGroupSubmit()
alt submission succeeds
GroupApi->>GroupApi: set isSubmitted = true, isSubmitSuccessful = true
else submission fails
GroupApi->>GroupApi: set isSubmitSuccessful = false
GroupApi->>GroupApi: set isSubmitting = false
GroupApi-->>Caller: rethrow error
end
GroupApi->>GroupApi: set isSubmitting = false
GroupApi-->>Caller: resolve
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:sherif,test:knip,tes... |
❌ Failed | 2m | View ↗ |
nx run-many --target=build --exclude=examples/** |
✅ Succeeded | 38s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-05-27 17:08:37 UTC
Contributor
🚀 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 #2128 +/- ##
==========================================
- Coverage 90.35% 88.19% -2.16%
==========================================
Files 38 65 +27
Lines 1752 3109 +1357
Branches 444 765 +321
==========================================
+ Hits 1583 2742 +1159
- Misses 149 328 +179
- Partials 20 39 +19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/form-core/tests/FormGroupApi.spec.ts (1)
73-76: Consider adding assertion for field error state.The test verifies callback invocations but doesn't assert that the field actually has errors after validation. Adding such an assertion would strengthen the test:
expect(step1NameField.state.meta.errors.length).toBeGreaterThan(0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/form-core/tests/FormGroupApi.spec.ts` around lines 73 - 76, Add an assertion to verify the field's error state after validation: after invoking validation callbacks in the test, assert that step1NameField.state.meta.errors contains at least one error (e.g., use an assertion like checking length > 0) so the test not only checks callback invocation but also that FieldApi (step1NameField) actually recorded validation errors.packages/form-core/src/FormGroupApi.ts (2)
124-126: Emptymount()method may need cleanup logic later.The mount method currently does nothing and returns an empty cleanup function. As the implementation matures, consider whether mount should:
- Subscribe to form state changes
- Initialize group-specific state
- Register the group with the form for coordinated lifecycle management
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/form-core/src/FormGroupApi.ts` around lines 124 - 126, The mount() method on FormGroupApi is currently a no-op; replace it with initialization and teardown logic: when mounting the group, perform any group-specific state initialization, subscribe to the parent form's change events (or call the form's registration API such as registerGroup or similar), and return a cleanup function that unsubscribes and deregisters the group (e.g., call unregisterGroup/removeGroup) to avoid leaks; ensure you reference the FormGroupApi instance state and the form-level APIs used to subscribe/register so the returned function reverses those actions.
216-220: Placeholder values (value: 0) need to be replaced with actual group values.Multiple callback invocations use
value: 0as a placeholder. This is noted in comments but tracking for completeness. Consider using a more explicit placeholder likeundefinedor extracting the actual group value subset.Also applies to: 234-238, 275-278, 281-285
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/form-core/src/FormGroupApi.ts` around lines 216 - 220, The callback invocations currently pass a placeholder value (value: 0) instead of the actual group values; replace those placeholders in the onGroup* calls (e.g., this.options.onGroupSubmitInvalid, the similar calls at the ranges 234-238, 275-278, 281-285) with the real subset of form state for this group by extracting the group's values from this.state.values (for example build a groupValues object by selecting the group's field keys from this.state.values) and pass that object as value, or if no values exist pass undefined explicitly; ensure you reference the group field list used by this class when building the subset so the callbacks receive accurate group data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/form-core/src/FormGroupApi.ts`:
- Around line 274-285: Remove the duplicate invocation of
this.options.onGroupSubmit in FormGroupApi: delete the redundant first call and
keep a single call that passes the real submit payload (use this.state.values
instead of 0 and submitMetaArg instead of {} if those are the intended
variables). Ensure only one this.options.onGroupSubmit is called and it uses the
correct arguments (this.state.values, this.options.form, submitMetaArg).
- Around line 128-131: The startsWith check in _isFieldNamePartOfGroup
incorrectly matches names like "step10" for group "step1"; update
_isFieldNamePartOfGroup (and use this.options.name) to verify a proper boundary
after the prefix: return true only if fieldName === prefix or fieldName
startsWith(prefix + '.') or fieldName startsWith(prefix + '[') (i.e., check that
the character immediately after the prefix is absent or is a separator like '.'
or '[') so unrelated names aren't included.
- Around line 225-241: The code validates group-scoped fields via
this.options.form.validate(..., filterFieldNames: this._isFieldNamePartOfGroup)
but then checks the entire form state with this.options.form.state.isValid;
replace that check with a call to this._isFieldsValid() so the submission
decision uses the same group-scoped validation logic (keeping the done() call
and the this.options.onGroupSubmitInvalid invocation intact), i.e., use
_isFieldsValid() instead of reading form.state.isValid after the filtered
validate call.
In `@packages/form-core/tests/FormGroupApi.spec.ts`:
- Around line 49-60: The validator in the test's onSubmit returns a nested
fields object (step1: { name: { required: true } }) but
GlobalFormValidationError expects flat field paths mapping to ValidationError;
update the onSubmit return to use flat keys like 'step1.name': { required: true
} (i.e., fields: { 'step1.name': { required: true } }) so
Object.keys(fieldErrors) yields the full field path and each value is a proper
ValidationError per the GlobalFormValidationError /
Partial<Record<DeepKeys<TFormData>, ValidationError>> shape.
---
Nitpick comments:
In `@packages/form-core/src/FormGroupApi.ts`:
- Around line 124-126: The mount() method on FormGroupApi is currently a no-op;
replace it with initialization and teardown logic: when mounting the group,
perform any group-specific state initialization, subscribe to the parent form's
change events (or call the form's registration API such as registerGroup or
similar), and return a cleanup function that unsubscribes and deregisters the
group (e.g., call unregisterGroup/removeGroup) to avoid leaks; ensure you
reference the FormGroupApi instance state and the form-level APIs used to
subscribe/register so the returned function reverses those actions.
- Around line 216-220: The callback invocations currently pass a placeholder
value (value: 0) instead of the actual group values; replace those placeholders
in the onGroup* calls (e.g., this.options.onGroupSubmitInvalid, the similar
calls at the ranges 234-238, 275-278, 281-285) with the real subset of form
state for this group by extracting the group's values from this.state.values
(for example build a groupValues object by selecting the group's field keys from
this.state.values) and pass that object as value, or if no values exist pass
undefined explicitly; ensure you reference the group field list used by this
class when building the subset so the callbacks receive accurate group data.
In `@packages/form-core/tests/FormGroupApi.spec.ts`:
- Around line 73-76: Add an assertion to verify the field's error state after
validation: after invoking validation callbacks in the test, assert that
step1NameField.state.meta.errors contains at least one error (e.g., use an
assertion like checking length > 0) so the test not only checks callback
invocation but also that FieldApi (step1NameField) actually recorded validation
errors.
🪄 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
Run ID: ac7e0310-fbaa-4485-b402-aece0b9fb522
📒 Files selected for processing (5)
packages/form-core/src/FieldApi.tspackages/form-core/src/FormApi.tspackages/form-core/src/FormGroupApi.tspackages/form-core/src/index.tspackages/form-core/tests/FormGroupApi.spec.ts
This is wrong because instead of keeping the state inside of `FormGroupApi` itself, we need to follow the same pattern of `FieldApi` and keep the state in `FormApi` and reference that state in a derived way.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This reverts commit 756ddf8.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2128 +/- ##
==========================================
- Coverage 90.35% 88.31% -2.05%
==========================================
Files 38 65 +27
Lines 1752 3140 +1388
Branches 444 781 +337
==========================================
+ Hits 1583 2773 +1190
- Misses 149 328 +179
- Partials 20 39 +19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
# Conflicts: # examples/react/next-server-actions-zod/package.json # examples/react/next-server-actions/package.json # examples/react/remix/package.json # pnpm-lock.yaml
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
docs/framework/preact/guides/form-groups.md (1)
134-143: ⚡ Quick winAdd language identifier to the code fence.
The code block starting at line 134 should specify a language (e.g.,
typescriptorts) for proper syntax highlighting.📝 Proposed fix
-> ``` +> ```typescript > const step1Schema = z.object({ > name: z.string().min(2) > })🤖 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/preact/guides/form-groups.md` around lines 134 - 143, The fenced code block containing the schema definitions (const step1Schema, step2Schema, and const schema using z.object) should include a language identifier for syntax highlighting; update the opening fence from ``` to ```typescript (or ```ts) so the block with step1Schema and schema is marked as TypeScript.examples/lit/multi-step-wizard/package.json (1)
15-15: ⚡ Quick winRemove the “invalid Zod version” / installation-failure concern for
^3.25.76.
zod@3.25.76is a published release on npm/GitHub, so^3.25.76should resolve successfully to a valid Zod 3.x version. If the intent is to use the newest Zod, consider upgrading to Zod 4.x (the npm default “latest” major is now 4.x).🤖 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 `@examples/lit/multi-step-wizard/package.json` at line 15, The review flags an incorrect "invalid Zod version" concern for the dependency "zod": "^3.25.76" in package.json; remove that concern by either leaving the existing dependency as-is (since ^3.25.76 is a valid published 3.x release) or, if you intended to use Zod v4, update the package.json entry "zod": "^3.25.76" to a 4.x spec (for example "zod": "^4.0.0") and adjust any PR description/comment that claims the version is invalid to reflect the correct status.
🤖 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 `@docs/framework/preact/guides/form-groups.md`:
- Line 86: The sentence "Form groups have a distinct validation proceedure that
we think makes sense for sub-forms:" contains a typo; update the text to "Form
groups have a distinct validation procedure that we think makes sense for
sub-forms:" by replacing "proceedure" with "procedure" in the
docs/framework/preact/guides/form-groups.md content where that exact sentence
appears.
In `@docs/framework/react/guides/form-groups.md`:
- Line 86: Fix the typos in the documentation text: replace “proceedure” with
“procedure” and change “is ran” to “is run” wherever they appear (e.g., the
sentence starting "Form groups have a distinct validation proceedure..." and the
other instance noted at the second occurrence); update both occurrences to
correct spelling and grammar so the text reads "procedure" and "is run".
- Line 134: The fenced code block closing with the triple backticks currently
lacks a language tag; update that code fence to declare the language (e.g., add
"ts") so the block becomes a TypeScript fenced code block — locate the fenced
block around the closing ``` in the docs/framework/react/guides/form-groups.md
content and change the opening fence to ```ts to improve markdown linting and
readability.
In `@docs/framework/solid/guides/form-groups.md`:
- Line 89: Update the two grammar typos in the documentation: change the phrase
"validation proceedure" to "validation procedure" and change "is ran" to "is
run" wherever they occur (e.g., the sentence starting "Form groups have a
distinct validation proceedure..." and the line containing "is ran"); ensure
both occurrences are corrected to the proper spelling/tense.
- Around line 135-144: Add a language identifier to the fenced code block that
contains the definitions for step1Schema and schema by changing the opening ```
to ```tsx (or another appropriate language) so the block (the snippet defining
step1Schema, step2Schema and schema) gets proper syntax highlighting and
satisfies the MD040 lint rule.
In `@docs/framework/svelte/guides/form-groups.md`:
- Line 187: Change the grammatically incorrect phrase "is ran" to "is run" in
the sentence that references group.submissionAttempts (the text "It will treat
`group.submissionAttempts` as the way to change what validator is ran
before/after submit."); update that sentence to read something like "It will
treat `group.submissionAttempts` as the way to change what validator is run
before/after submit."
In `@docs/framework/vue/guides/form-groups.md`:
- Line 101: Fix two wording issues: correct the misspelling "proceedure" to
"procedure" and replace the grammatically incorrect "is ran" with "is run".
Locate the occurrences of the phrases (e.g., the sentence starting "Form groups
have a distinct validation proceedure..." and the other occurrence flagged) and
update them in the guide content and the second instance mentioned; ensure
surrounding punctuation/capitalization remains consistent.
In `@examples/angular/multi-step-wizard/src/index.html`:
- Line 5: Update the HTML document title element (the <title> tag) to a more
descriptive string that reflects the page content; replace the current "Simple"
title with something like "TanStack Form Angular Multi-Step Wizard" so the
browser tab and metadata clearly indicate this is the multi-step wizard example.
In `@examples/react/multi-step-wizard/index.html`:
- Line 9: Update the <title> element text so it reflects the correct example:
replace "TanStack Form React Simple Example App" with "TanStack Form React
Multi-Step Wizard" in the <title> tag to match the multi-step-wizard example.
In `@examples/react/multi-step-wizard/src/features/wizard/step2-subform.tsx`:
- Line 33: The Back button inside the form currently uses a bare <button> which
defaults to type="submit" and can trigger form submission; update the Back
button element (the one calling setStep(step - 1)) to explicitly set
type="button" so clicking it won't submit or validate the form.
---
Nitpick comments:
In `@docs/framework/preact/guides/form-groups.md`:
- Around line 134-143: The fenced code block containing the schema definitions
(const step1Schema, step2Schema, and const schema using z.object) should include
a language identifier for syntax highlighting; update the opening fence from ```
to ```typescript (or ```ts) so the block with step1Schema and schema is marked
as TypeScript.
In `@examples/lit/multi-step-wizard/package.json`:
- Line 15: The review flags an incorrect "invalid Zod version" concern for the
dependency "zod": "^3.25.76" in package.json; remove that concern by either
leaving the existing dependency as-is (since ^3.25.76 is a valid published 3.x
release) or, if you intended to use Zod v4, update the package.json entry "zod":
"^3.25.76" to a 4.x spec (for example "zod": "^4.0.0") and adjust any PR
description/comment that claims the version is invalid to reflect the correct
status.
🪄 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
Run ID: eb598f67-52e2-4a28-9bde-4bef92098edb
⛔ Files ignored due to path filters (3)
docs/assets/stepper.pngis excluded by!**/*.pngexamples/angular/multi-step-wizard/src/favicon.icois excluded by!**/*.icoexamples/react/multi-step-wizard/public/emblem-light.svgis excluded by!**/*.svg
📒 Files selected for processing (68)
docs/config.jsondocs/framework/angular/guides/form-groups.mddocs/framework/lit/guides/form-groups.mddocs/framework/preact/guides/form-groups.mddocs/framework/react/guides/form-groups.mddocs/framework/solid/guides/form-groups.mddocs/framework/svelte/guides/form-groups.mddocs/framework/vue/guides/form-groups.mdexamples/angular/multi-step-wizard/.editorconfigexamples/angular/multi-step-wizard/.gitignoreexamples/angular/multi-step-wizard/README.mdexamples/angular/multi-step-wizard/angular.jsonexamples/angular/multi-step-wizard/package.jsonexamples/angular/multi-step-wizard/src/app/app.component.tsexamples/angular/multi-step-wizard/src/app/shared-form.tsexamples/angular/multi-step-wizard/src/app/step1.component.tsexamples/angular/multi-step-wizard/src/app/step2.component.tsexamples/angular/multi-step-wizard/src/app/text-field.component.tsexamples/angular/multi-step-wizard/src/index.htmlexamples/angular/multi-step-wizard/src/main.tsexamples/angular/multi-step-wizard/tsconfig.app.jsonexamples/angular/multi-step-wizard/tsconfig.jsonexamples/angular/multi-step-wizard/tsconfig.spec.jsonexamples/lit/multi-step-wizard/.eslintrc.cjsexamples/lit/multi-step-wizard/.gitignoreexamples/lit/multi-step-wizard/README.mdexamples/lit/multi-step-wizard/index.htmlexamples/lit/multi-step-wizard/package.jsonexamples/lit/multi-step-wizard/src/components/text-field.tsexamples/lit/multi-step-wizard/src/features/wizard/page.tsexamples/lit/multi-step-wizard/src/features/wizard/shared-form.tsexamples/lit/multi-step-wizard/src/features/wizard/step1-subform.tsexamples/lit/multi-step-wizard/src/features/wizard/step2-subform.tsexamples/lit/multi-step-wizard/src/index.tsexamples/lit/multi-step-wizard/tsconfig.jsonexamples/preact/multi-step-wizard/README.mdexamples/preact/multi-step-wizard/index.htmlexamples/preact/multi-step-wizard/package.jsonexamples/preact/multi-step-wizard/src/App.tsxexamples/preact/multi-step-wizard/src/components/text-fields.tsxexamples/preact/multi-step-wizard/src/features/wizard/page.tsxexamples/preact/multi-step-wizard/src/features/wizard/shared-form.tsxexamples/preact/multi-step-wizard/src/features/wizard/step1-subform.tsxexamples/preact/multi-step-wizard/src/features/wizard/step2-subform.tsxexamples/preact/multi-step-wizard/src/hooks/form-context.tsxexamples/preact/multi-step-wizard/src/hooks/form.tsxexamples/preact/multi-step-wizard/src/index.tsxexamples/preact/multi-step-wizard/tsconfig.jsonexamples/preact/multi-step-wizard/vite.config.tsexamples/react/multi-step-wizard/.eslintrc.cjsexamples/react/multi-step-wizard/.gitignoreexamples/react/multi-step-wizard/README.mdexamples/react/multi-step-wizard/index.htmlexamples/react/multi-step-wizard/package.jsonexamples/react/multi-step-wizard/src/App.tsxexamples/react/multi-step-wizard/src/components/text-fields.tsxexamples/react/multi-step-wizard/src/features/wizard/page.tsxexamples/react/multi-step-wizard/src/features/wizard/shared-form.tsxexamples/react/multi-step-wizard/src/features/wizard/step1-subform.tsxexamples/react/multi-step-wizard/src/features/wizard/step2-subform.tsxexamples/react/multi-step-wizard/src/hooks/form-context.tsxexamples/react/multi-step-wizard/src/hooks/form.tsxexamples/react/multi-step-wizard/src/index.tsxexamples/react/multi-step-wizard/tsconfig.jsonexamples/react/next-server-actions-zod/package.jsonexamples/react/next-server-actions/package.jsonexamples/react/remix/package.jsonexamples/react/tanstack-start/package.json
✅ Files skipped from review due to trivial changes (27)
- examples/lit/multi-step-wizard/README.md
- examples/preact/multi-step-wizard/README.md
- examples/react/multi-step-wizard/README.md
- examples/angular/multi-step-wizard/src/main.ts
- examples/preact/multi-step-wizard/src/index.tsx
- examples/react/multi-step-wizard/.eslintrc.cjs
- examples/preact/multi-step-wizard/src/App.tsx
- examples/angular/multi-step-wizard/tsconfig.spec.json
- examples/react/next-server-actions-zod/package.json
- examples/react/remix/package.json
- examples/preact/multi-step-wizard/index.html
- examples/angular/multi-step-wizard/.editorconfig
- examples/react/multi-step-wizard/.gitignore
- examples/lit/multi-step-wizard/.eslintrc.cjs
- examples/react/multi-step-wizard/src/hooks/form-context.tsx
- examples/react/next-server-actions/package.json
- examples/react/multi-step-wizard/package.json
- examples/preact/multi-step-wizard/tsconfig.json
- examples/lit/multi-step-wizard/src/index.ts
- examples/react/multi-step-wizard/src/index.tsx
- examples/angular/multi-step-wizard/README.md
- examples/angular/multi-step-wizard/.gitignore
- examples/lit/multi-step-wizard/.gitignore
- examples/angular/multi-step-wizard/package.json
- examples/angular/multi-step-wizard/src/app/shared-form.ts
- examples/angular/multi-step-wizard/tsconfig.app.json
- examples/lit/multi-step-wizard/src/features/wizard/shared-form.ts
|
|
||
| ## Form Group Validation | ||
|
|
||
| Form groups have a distinct validation proceedure that we think makes sense for sub-forms: |
There was a problem hiding this comment.
Fix the typo.
"proceedure" should be "procedure".
📝 Proposed fix
-Form groups have a distinct validation proceedure that we think makes sense for sub-forms:
+Form groups have a distinct validation procedure that we think makes sense for sub-forms:📝 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.
Suggested change
| Form groups have a distinct validation proceedure that we think makes sense for sub-forms: | |
| Form groups have a distinct validation procedure that we think makes sense for sub-forms: |
🧰 Tools
🪛 LanguageTool
[grammar] ~86-~86: Ensure spelling is correct
Context: ... Form groups have a distinct validation proceedure that we think makes sense for sub-forms...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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/preact/guides/form-groups.md` at line 86, The sentence "Form
groups have a distinct validation proceedure that we think makes sense for
sub-forms:" contains a typo; update the text to "Form groups have a distinct
validation procedure that we think makes sense for sub-forms:" by replacing
"proceedure" with "procedure" in the docs/framework/preact/guides/form-groups.md
content where that exact sentence appears.
|
|
||
| ## Form Group Validation | ||
|
|
||
| Form groups have a distinct validation proceedure that we think makes sense for sub-forms: |
There was a problem hiding this comment.
Fix spelling/grammar in validation wording.
Please change “proceedure” → “procedure” and “is ran” → “is run”.
Suggested edits
-Form groups have a distinct validation proceedure that we think makes sense for sub-forms:
+Form groups have a distinct validation procedure that we think makes sense for sub-forms:
...
-It will treat `group.submissionAttempts` as the way to change what validator is ran before/after submit.
+It will treat `group.submissionAttempts` as the way to change what validator is run before/after submit.Also applies to: 168-168
🧰 Tools
🪛 LanguageTool
[grammar] ~86-~86: Ensure spelling is correct
Context: ... Form groups have a distinct validation proceedure that we think makes sense for sub-forms...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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/react/guides/form-groups.md` at line 86, Fix the typos in the
documentation text: replace “proceedure” with “procedure” and change “is ran” to
“is run” wherever they appear (e.g., the sentence starting "Form groups have a
distinct validation proceedure..." and the other instance noted at the second
occurrence); update both occurrences to correct spelling and grammar so the text
reads "procedure" and "is run".
|
|
||
| > The reason we don't use the full path names for fields is so that you can compose your schemas like so: | ||
| > | ||
| > ``` |
There was a problem hiding this comment.
Add a language tag to the fenced code block.
The quoted code fence should declare a language for markdown lint/readability.
Suggested edit
-> ```
+> ```ts📝 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.
Suggested change
| > ``` | |
| > |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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/react/guides/form-groups.md` at line 134, The fenced code
block closing with the triple backticks currently lacks a language tag; update
that code fence to declare the language (e.g., add "ts") so the block becomes a
TypeScript fenced code block — locate the fenced block around the closing ``` in
the docs/framework/react/guides/form-groups.md content and change the opening
fence to ```ts to improve markdown linting and readability.
|
|
||
| ## Form Group Validation | ||
|
|
||
| Form groups have a distinct validation proceedure that we think makes sense for sub-forms: |
There was a problem hiding this comment.
Fix documentation grammar typos in validation text.
There are user-facing grammar mistakes:
- Line 89:
proceedure→procedure - Line 169:
is ran→is run
✍️ Suggested patch
-Form groups have a distinct validation proceedure that we think makes sense for sub-forms:
+Form groups have a distinct validation procedure that we think makes sense for sub-forms:-It will treat `group().submissionAttempts` as the way to change what validator is ran before/after submit.
+It will treat `group().submissionAttempts` as the way to change which validator is run before/after submit.Also applies to: 169-169
🧰 Tools
🪛 LanguageTool
[grammar] ~89-~89: Ensure spelling is correct
Context: ... Form groups have a distinct validation proceedure that we think makes sense for sub-forms...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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/solid/guides/form-groups.md` at line 89, Update the two
grammar typos in the documentation: change the phrase "validation proceedure" to
"validation procedure" and change "is ran" to "is run" wherever they occur
(e.g., the sentence starting "Form groups have a distinct validation
proceedure..." and the line containing "is ran"); ensure both occurrences are
corrected to the proper spelling/tense.
Comment on lines
+135
to
+144
| > ``` | ||
| > const step1Schema = z.object({ | ||
| > name: z.string().min(2) | ||
| > }) | ||
| > | ||
| > const schema = z.object({ | ||
| > step1: step1Schema, | ||
| > step2: step2Schema | ||
| > }) | ||
| > ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
The fenced block is missing a language tag (MD040), which hurts linting and syntax highlighting.
🧩 Suggested patch
-> ```
+> ```tsx
> const step1Schema = z.object({
> name: z.string().min(2)
> })
>
> const schema = z.object({
> step1: step1Schema,
> step2: step2Schema
> })
> ```🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 135-135: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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/solid/guides/form-groups.md` around lines 135 - 144, Add a
language identifier to the fenced code block that contains the definitions for
step1Schema and schema by changing the opening ``` to ```tsx (or another
appropriate language) so the block (the snippet defining step1Schema,
step2Schema and schema) gets proper syntax highlighting and satisfies the MD040
lint rule.
| <form.FormGroup name="step1" validators={{ onDynamic: step1Schema }} /> | ||
| ``` | ||
|
|
||
| It will treat `group.submissionAttempts` as the way to change what validator is ran before/after submit. |
There was a problem hiding this comment.
Fix grammar in the validator sentence.
“is ran” should be “is run” to avoid confusing wording in the guide.
Suggested edit
-It will treat `group.submissionAttempts` as the way to change what validator is ran before/after submit.
+It will treat `group.submissionAttempts` as the way to change what validator is run before/after submit.📝 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.
Suggested change
| It will treat `group.submissionAttempts` as the way to change what validator is ran before/after submit. | |
| It will treat `group.submissionAttempts` as the way to change what validator is run before/after submit. |
🤖 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-groups.md` at line 187, Change the
grammatically incorrect phrase "is ran" to "is run" in the sentence that
references group.submissionAttempts (the text "It will treat
`group.submissionAttempts` as the way to change what validator is ran
before/after submit."); update that sentence to read something like "It will
treat `group.submissionAttempts` as the way to change what validator is run
before/after submit."
|
|
||
| ## Form Group Validation | ||
|
|
||
| Form groups have a distinct validation proceedure that we think makes sense for sub-forms: |
There was a problem hiding this comment.
Correct two wording issues in the guide.
There’s a spelling typo (“proceedure”) and a grammar issue (“is ran”).
Suggested edits
-Form groups have a distinct validation proceedure that we think makes sense for sub-forms:
+Form groups have a distinct validation procedure that we think makes sense for sub-forms:
...
-It will treat `formGroup.submissionAttempts` as the way to change what validator is ran before/after submit.
+It will treat `formGroup.submissionAttempts` as the way to change what validator is run before/after submit.Also applies to: 191-191
🤖 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/vue/guides/form-groups.md` at line 101, Fix two wording
issues: correct the misspelling "proceedure" to "procedure" and replace the
grammatically incorrect "is ran" with "is run". Locate the occurrences of the
phrases (e.g., the sentence starting "Form groups have a distinct validation
proceedure..." and the other occurrence flagged) and update them in the guide
content and the second instance mentioned; ensure surrounding
punctuation/capitalization remains consistent.
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>Simple</title> |
There was a problem hiding this comment.
Use a more descriptive title.
The title "Simple" doesn't convey that this is a multi-step wizard example. Consider "TanStack Form Angular Multi-Step Wizard" for clarity.
📝 Proposed fix
- <title>Simple</title>
+ <title>TanStack Form Angular Multi-Step Wizard</title>📝 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.
Suggested change
| <title>Simple</title> | |
| <title>TanStack Form Angular Multi-Step Wizard</title> |
🤖 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 `@examples/angular/multi-step-wizard/src/index.html` at line 5, Update the HTML
document title element (the <title> tag) to a more descriptive string that
reflects the page content; replace the current "Simple" title with something
like "TanStack Form Angular Multi-Step Wizard" so the browser tab and metadata
clearly indicate this is the multi-step wizard example.
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
| <meta name="theme-color" content="#000000" /> | ||
|
|
||
| <title>TanStack Form React Simple Example App</title> |
There was a problem hiding this comment.
Update the title to match the example.
The title references "Simple Example App" but this is the multi-step-wizard example. Consider changing it to "TanStack Form React Multi-Step Wizard" for clarity.
📝 Proposed fix
- <title>TanStack Form React Simple Example App</title>
+ <title>TanStack Form React Multi-Step Wizard</title>📝 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.
Suggested change
| <title>TanStack Form React Simple Example App</title> | |
| <title>TanStack Form React Multi-Step Wizard</title> |
🤖 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 `@examples/react/multi-step-wizard/index.html` at line 9, Update the <title>
element text so it reflects the correct example: replace "TanStack Form React
Simple Example App" with "TanStack Form React Multi-Step Wizard" in the <title>
tag to match the multi-step-wizard example.
| {(field) => <field.TextField label="Step 2 Name" />} | ||
| </form.AppField> | ||
|
|
||
| <button onClick={() => setStep(step - 1)}>Back</button> |
There was a problem hiding this comment.
Set the Back button type to button to prevent accidental submit.
Inside a <form>, a bare <button> submits by default, so Back can unintentionally trigger submit/validation.
Proposed fix
- <button onClick={() => setStep(step - 1)}>Back</button>
+ <button type="button" onClick={() => setStep(step - 1)}>
+ Back
+ </button>📝 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.
Suggested change
| <button onClick={() => setStep(step - 1)}>Back</button> | |
| <button type="button" onClick={() => setStep(step - 1)}> | |
| Back | |
| </button> |
🤖 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 `@examples/react/multi-step-wizard/src/features/wizard/step2-subform.tsx` at
line 33, The Back button inside the form currently uses a bare <button> which
defaults to type="submit" and can trigger form submission; update the Back
button element (the one calling setStep(step - 1)) to explicitly set
type="button" so clicking it won't submit or validate the form.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/form-core/src/FieldApi.ts (1)
1590-1594:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
isFieldInGroupwhen collecting enclosing groups.Line 1593 still uses a raw prefix match, so
step10.namewill be treated as part of astep1group and inherit unrelated group validation/submission state. This should use the same boundary-aware helper as the filtered form-validation path.Suggested fix
const encompassingGroups = opts?.skipGroupValidation ? [] : Array.from(this.form.formGroupApis).filter((group) => - this.name.startsWith(group.name), + isFieldInGroup(group.name as string, this.name as string), )🤖 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-core/src/FieldApi.ts` around lines 1590 - 1594, The current collection of encompassingGroups uses a raw prefix match (this.name.startsWith(group.name)) which misclassifies groups like "step10" as part of "step1"; update the logic in the block that defines encompassingGroups (respecting opts?.skipGroupValidation) to call the boundary-aware helper isFieldInGroup(this.name, group.name) for each group in this.form.formGroupApis instead of startsWith, so group membership matches the filtered form-validation path exactly.packages/form-core/src/FormGroupApi.ts (2)
1432-1468:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInvoke the group lifecycle hooks you expose.
FormGroupValidators.onMount,FormGroupListeners.onMount, andFormGroupListeners.onUnmountare part of the public API, butmount()only seeds state and returns cleanup. UnlikeFieldApi, those hooks are currently dead for groups.🤖 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-core/src/FormGroupApi.ts` around lines 1432 - 1468, mount currently only seeds state and never calls the public lifecycle hooks for groups; invoke the exposed hooks by calling FormGroupValidators.onMount and FormGroupListeners.onMount during mount (e.g., this.options.validators?.onMount?.(this) and this.options.listeners?.onMount?.(this)), and call FormGroupListeners.onUnmount in the returned cleanup (e.g., this.options.listeners?.onUnmount?.(this)); mirror FieldApi.mount's pattern so group hooks run on mount and unmount while keeping the existing state-seeding and teardown logic.
1432-1468:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRegister the mounted group in
fieldInfo.
validateAsync()drops group-owned async results at Line 1984 becausemount()never setsthis.getInfo().instance = this. As written, every async group validator resolves through the stale-instance guard and never updates group or distributed child errors.Suggested fix
mount = () => { this.update(this.options as never) + const info = this.getInfo() + info.instance = this as never this.form.formGroupApis.add(this) @@ return () => { this.form.formGroupApis.delete(this) + const info = this.getInfo() + if (info.instance !== this) return // Reset this group's submission lifecycle state on the form. Mirrors // `FieldApi.mount`'s teardown which resets `fieldMetaBase` for the // unmounting field while preserving the entry on the parent store. this.form.baseStore.setState((prev) => ({ @@ [this.name as never]: getDefaultFormGroupState({}), }, })) + + info.instance = null } }🤖 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-core/src/FormGroupApi.ts` around lines 1432 - 1468, FormGroupApi.mount currently never registers the instance on its fieldInfo, so async validators in validateAsync (which check getInfo().instance) treat the instance as stale and drop results; update mount (in FormGroupApi.mount) to assign this.getInfo().instance = this when mounting (before async validation can run, e.g., right after this.form.formGroupApis.add(this) or at the start of mount) and clear it on unmount (set this.getInfo().instance = undefined or null) inside the returned teardown so group async results can pass the stale-instance guard and update group/child errors as intended.
♻️ Duplicate comments (1)
packages/form-core/src/FormGroupApi.ts (1)
1531-1545:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the boundary-aware helper in
getRelatedFields().Line 1539 still uses
startsWith, so a group namedstep1will also pull instep10.*fields during group validation and submission. This is the same false-positive class thatisFieldInGroupwas added to fix.Suggested fix
for (const field of fields) { if (!field.instance) continue // TODO: How to handle FormGroups? if (!(field.instance instanceof FieldApi)) continue - if (field.instance.name.startsWith(this.name)) { + if (isFieldInGroup(this.name as string, field.instance.name as string)) { relatedFields.push(field.instance) } }🤖 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-core/src/FormGroupApi.ts` around lines 1531 - 1545, getRelatedFields currently uses name.startsWith(this.name) which falsely includes names like "step10" when group is "step1"; replace that boundary-unsafe check with the boundary-aware helper isFieldInGroup to determine group membership. In the getRelatedFields method (in FormGroupApi) swap the startsWith check (field.instance.name.startsWith(this.name)) for a call to isFieldInGroup(this.name, field.instance.name) (or the appropriate import/name for the helper), keeping the other guards (instance type check, null checks) the same so only true group members are returned.
🤖 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.
Outside diff comments:
In `@packages/form-core/src/FieldApi.ts`:
- Around line 1590-1594: The current collection of encompassingGroups uses a raw
prefix match (this.name.startsWith(group.name)) which misclassifies groups like
"step10" as part of "step1"; update the logic in the block that defines
encompassingGroups (respecting opts?.skipGroupValidation) to call the
boundary-aware helper isFieldInGroup(this.name, group.name) for each group in
this.form.formGroupApis instead of startsWith, so group membership matches the
filtered form-validation path exactly.
In `@packages/form-core/src/FormGroupApi.ts`:
- Around line 1432-1468: mount currently only seeds state and never calls the
public lifecycle hooks for groups; invoke the exposed hooks by calling
FormGroupValidators.onMount and FormGroupListeners.onMount during mount (e.g.,
this.options.validators?.onMount?.(this) and
this.options.listeners?.onMount?.(this)), and call FormGroupListeners.onUnmount
in the returned cleanup (e.g., this.options.listeners?.onUnmount?.(this));
mirror FieldApi.mount's pattern so group hooks run on mount and unmount while
keeping the existing state-seeding and teardown logic.
- Around line 1432-1468: FormGroupApi.mount currently never registers the
instance on its fieldInfo, so async validators in validateAsync (which check
getInfo().instance) treat the instance as stale and drop results; update mount
(in FormGroupApi.mount) to assign this.getInfo().instance = this when mounting
(before async validation can run, e.g., right after
this.form.formGroupApis.add(this) or at the start of mount) and clear it on
unmount (set this.getInfo().instance = undefined or null) inside the returned
teardown so group async results can pass the stale-instance guard and update
group/child errors as intended.
---
Duplicate comments:
In `@packages/form-core/src/FormGroupApi.ts`:
- Around line 1531-1545: getRelatedFields currently uses
name.startsWith(this.name) which falsely includes names like "step10" when group
is "step1"; replace that boundary-unsafe check with the boundary-aware helper
isFieldInGroup to determine group membership. In the getRelatedFields method (in
FormGroupApi) swap the startsWith check
(field.instance.name.startsWith(this.name)) for a call to
isFieldInGroup(this.name, field.instance.name) (or the appropriate import/name
for the helper), keeping the other guards (instance type check, null checks) the
same so only true group members are returned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5f51631-e6c2-49f6-b22c-d7daa31375f0
📒 Files selected for processing (4)
packages/form-core/src/FieldApi.tspackages/form-core/src/FormApi.tspackages/form-core/src/FormGroupApi.tspackages/form-core/src/utils.ts
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
4 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.
This PR implements #419 as outlined in the issue.
Core TODOs
corepackage integration testscorepackage type testsFormGroupgetFormGroupStoreor something like it so users can access group validity externallyFramework support
reactadapter base codereactadapter integration testsreactadapter type testsreactadapter examplesreactadapter docspreactadapter base codepreactadapter integration testspreactadapter type testspreactadapter examplespreactadapter docsangularadapter base codeangularadapter integration testsangularadapter examplesangularadapter docslitadapter base codelitadapter integration testslitadapter exampleslitadapter docssolidadapter base codesolidadapter integration testssolidadapter type testssolidadapter examplessolidadapter docssvelteadapter base codesvelteadapter integration testssvelteadapter examplessvelteadapter docsvueadapter base codevueadapter integration testsvueadapter examplesvueadapter docsSummary by CodeRabbit
New Features
Documentation
Tests