fix: prevent formId reset render loop - #2408
maricastroc 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 (5)
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 React and Preact ChangesForm ID fallback
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Removing an explicit formId restores the generated identifier, including through useAppForm. Both adapter tests cover the transitions, and no material merge risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The fix stops the render loop and restores the generated form ID. A form that changes IDs may still leave developer-tool actions attached to an older form instance. No new authorization boundary or attacker-accessible path was established. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. (1 skipped: 1 unsupported.)
✨ 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 |
🎯 Changes
Fixes #2407.
useForm(and thereforeuseAppForm) re-creates itsFormApiwhenformIdchanges. It comparedprevFormIdwith the rawopts.formId, but stored the resolved id (opts.formId ?? fallbackFormId). AfterformIdchanged from a string toundefined,prevFormIdheld the generated fallback id, so the check stayed true on every render and the hook kept setting state during render ("Too many re-renders" in React, an unbounded render loop in Preact).prevFormIdnow stores the rawopts.formId, matching how it is initialized and howuseField/useFormGrouptrack their options. ChangingformIdback toundefinedrestores the generated fallback id.The same fix is applied to
@tanstack/react-formand@tanstack/preact-form, which share this code. Each package gets a regression test forundefined → 'test' → undefined → 'test'; the Preact test caps render count so an unfixed build fails instead of hanging.✅ Checklist
pnpm test:pr, or these tests do not apply to this pull request.🚀 Release Impact
Summary by CodeRabbit
formIdtoundefinedafter using an explicit ID now correctly restores the generated ID in React and Preact forms.