[v2] Refactor: Move Form Groups to trie - #2326
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughForm groups now attach to field-trie nodes instead of a form registry. Group state, validation, errors, reset behavior, traversal, and devtools lookup use attached nodes and containing-group resolution. ChangesForm group storage and validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FieldApi
participant FieldTrie
participant FormGroupApi
participant Validation
participant Devtools
FieldApi->>FieldTrie: Attach form group to field node
FieldApi->>FieldTrie: Resolve containing group
FormGroupApi->>Validation: Run group validators
Validation-->>FormGroupApi: Return group and field errors
FormGroupApi->>FieldTrie: Store metadata and routed errors
FieldApi->>Devtools: Emit field update
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
|
View your CI Pipeline Execution ↗ for commit 235441a
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.changeset/tasty-humans-joke.md (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
@tanstack/form-devtoolsto the changeset.This PR changes devtools source files (
packages/form-devtools/src/bridge/fields/list.ts,detailSnapshot.ts,validatorsWithoutTriggers.ts,debug/schemaErrorOnUnmountedField.ts). Without an entry, that package is not released with the core change.📦 Proposed changeset front matter
--- '`@tanstack/form-core`': patch +'`@tanstack/form-devtools`': patch ---🤖 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 @.changeset/tasty-humans-joke.md at line 2, Update the changeset front matter to include `@tanstack/form-devtools` alongside `@tanstack/form-core` with the appropriate release bump, so the devtools source changes are included in the release.packages/form-core/tests/FormGroupApi/FormGroupApi.spec.ts (1)
104-224: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the deferred-validator scaffold.
The three tests repeat the same
resolversarray,vi.fnpromise validator, form, and group construction. A small helper that returns{ form, group, validator, resolvers }would remove the duplication and keep each test focused on its lifecycle assertions.🤖 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/tests/FormGroupApi/FormGroupApi.spec.ts` around lines 104 - 224, Extract the repeated deferred-validator setup from the three tests into a shared helper returning form, group, validator, and resolvers. Reuse that helper in each test while preserving the existing guestDetails configuration and lifecycle assertions.
🤖 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 @.changeset/tasty-humans-joke.md:
- Line 5: Update the changelog entry text to use the correct plural possessive
for “Groups” and specify what is being accessed, preserving the intended meaning
of the Form Groups storage migration.
In `@packages/form-core/src/FieldApi/fieldState.lib.ts`:
- Line 32: Preserve per-group error ownership across the listed sites: in
packages/form-core/src/FieldApi/fieldState.lib.ts lines 32-32, replace the
single shared _formGroupValidatorErrors value with metadata that retains
separate buckets keyed by containing group; in
packages/form-core/src/FormGroupApi/FormGroupApi.lib.ts lines 338-377, update
and clear only the current group’s bucket; and in
packages/form-core/src/FormApi/formState.lib.ts lines 132-145, aggregate all
active group-error buckets when computing field errors so nested and outer group
errors remain visible to the correct owner.
In `@packages/form-core/src/FieldApi/fieldTree.lib.ts`:
- Around line 331-334: Update the killField lifecycle so deleting a field that
backs an InternalFormGroupApi does not leave the live group pointing at a
detached node; either reject deletion, clean up the group, or reattach it to the
replacement live trie node. Preserve group attachment and validation for later
field creation under the same path, and keep _groupField and _formGroup
consistent.
In `@packages/form-core/src/FormGroupApi/FormGroupApi.lib.ts`:
- Around line 235-242: Update FormGroupApi._cancelValidation so it no longer
resets the shared _groupField validation count. Track validation runs owned by
the group, including those started through _runFieldValidations, and on
cancellation remove only the group’s contributions while preserving
backing-field validator counts and isValidating state.
---
Nitpick comments:
In @.changeset/tasty-humans-joke.md:
- Line 2: Update the changeset front matter to include `@tanstack/form-devtools`
alongside `@tanstack/form-core` with the appropriate release bump, so the devtools
source changes are included in the release.
In `@packages/form-core/tests/FormGroupApi/FormGroupApi.spec.ts`:
- Around line 104-224: Extract the repeated deferred-validator setup from the
three tests into a shared helper returning form, group, validator, and
resolvers. Reuse that helper in each test while preserving the existing
guestDetails configuration and lifecycle assertions.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d3793037-a07c-4684-bd99-716b352a569c
📒 Files selected for processing (20)
.changeset/tasty-humans-joke.mdpackages/form-core/src/FieldApi/FieldApi.lib.tspackages/form-core/src/FieldApi/fieldState.lib.tspackages/form-core/src/FieldApi/fieldTraversal.lib.tspackages/form-core/src/FieldApi/fieldTree.lib.tspackages/form-core/src/FormApi/FormApi.lib.tspackages/form-core/src/FormApi/FormApi.public.tspackages/form-core/src/FormApi/formState.lib.tspackages/form-core/src/FormGroupApi/FormGroupApi.lib.tspackages/form-core/src/validation.lib.tspackages/form-core/src/validation.public.tspackages/form-core/tests/FieldApi/fieldTraversal.spec.tspackages/form-core/tests/FieldApi/meta.spec.tspackages/form-core/tests/FormGroupApi/FormGroupApi.spec.tspackages/form-devtools/src/bridge/fields/debug/schemaErrorOnUnmountedField.tspackages/form-devtools/src/bridge/fields/detailSnapshot.tspackages/form-devtools/src/bridge/fields/fieldDebug/validatorsWithoutTriggers.tspackages/form-devtools/src/bridge/fields/list.tspackages/form-devtools/tests/fieldDebugCases.test.tspackages/form-devtools/tests/fieldDetailsBridge.test.ts
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #2326 +/- ##
========================================
Coverage ? 95.17%
========================================
Files ? 64
Lines ? 3482
Branches ? 831
========================================
Hits ? 3314
Misses ? 160
Partials ? 8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-core/src/FieldApi/fieldTree.lib.ts`:
- Around line 272-274: Update the replacement cleanup loop around formGroups and
group._cancelValidation() to remove killed fields from _routedErrorFields,
filtering out any fields no longer live while preserving active routed error
fields. Add a regression test covering replacement of a routed field and
verifying the killed field is removed immediately.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 37a12217-15bf-448a-b552-f7799e58d31e
📒 Files selected for processing (5)
packages/form-core/src/FieldApi/FieldApi.lib.tspackages/form-core/src/FieldApi/fieldTree.lib.tspackages/form-core/src/FormApi/FormApi.lib.tspackages/form-core/src/FormGroupApi/FormGroupApi.lib.tspackages/form-core/tests/FormGroupApi/FormGroupApi.spec.ts
💤 Files with no reviewable changes (1)
- packages/form-core/src/FormApi/FormApi.lib.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/form-core/tests/FormGroupApi/FormGroupApi.spec.ts
- packages/form-core/src/FormGroupApi/FormGroupApi.lib.ts
Summary by CodeRabbit
Bug Fixes
Refactor