fix(react-form): accept extendForm forms in base withFieldGroup components - #2412
dasjideepak wants to merge 1 commit into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: TanStack/form/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change declares ChangesForm component compatibility
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to This is a type-level change that lets forms from extendForm be passed to base withFieldGroup components. Base forms passed to extended groups are still rejected. No runtime behavior changes, and no merge-blocking risk was found. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The reviewed change widens compile-time compatibility without changing runtime execution or privileges. The added type test preserves rejection of forms that lack components required by an extended field group. No introduced or worsened security risk was identified in this change. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Closes #2411
🎯 Changes
A form from
extendFormcould not be passed to a field group made with the base hook'swithFieldGroup, even though it only adds components.Why:
FieldComponent,LensFieldComponentandFormGroupComponentmarkedExtendedApi(the component map) asin out, so it had to match exactly.But
ExtendedApiis only used in the type of the field passed tochildren. A form with more components giveschildrena field with more properties, which is always safe. Sooutis the right variance.The fix:
in out ExtendedApi→out ExtendedApiin those 3 types.Test: a new case in
createFormHook.test-d.tsx:extendForm(extra field and form components) is accepted by a basewithFieldGroupgroup.ExtendedField.Before (
main): the new test fails with the error from the issue.After (this PR): 0 type errors, and TypeScript 5.4 to 5.9 all pass.
✅ Checklist
pnpm test:pr, or these tests do not apply to this pull request.🚀 Release Impact
Summary by CodeRabbit
New Features
Bug Fixes