Skip to content

fix: prevent formId reset render loop - #2408

Open
maricastroc wants to merge 1 commit into
TanStack:mainfrom
maricastroc:fix/useform-formid-undefined-loop
Open

maricastroc wants to merge 1 commit into
TanStack:mainfrom
maricastroc:fix/useform-formid-undefined-loop

Conversation

@maricastroc

@maricastroc maricastroc commented Sep 26, 2026 •

Copy link
Copy Markdown

🎯 Changes

Fixes #2407.

useForm (and therefore useAppForm) re-creates its FormApi when formId changes. It compared prevFormId with the raw opts.formId, but stored the resolved id (opts.formId ?? fallbackFormId). After formId changed from a string to undefined, prevFormId held 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).

prevFormId now stores the raw opts.formId, matching how it is initialized and how useField/useFormGroup track their options. Changing formId back to undefined restores the generated fallback id.

The same fix is applied to @tanstack/react-form and @tanstack/preact-form, which share this code. Each package gets a regression test for undefined → 'test' → undefined → 'test'; the Preact test caps render count so an unfixed build fails instead of hanging.

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested code changes locally with pnpm test:pr, or these tests do not apply to this pull request.
  • I fully understand the code in this pull request, including any code generated with AI assistance.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • Bug Fixes
    • Restoring formId to undefined after using an explicit ID now correctly restores the generated ID in React and Preact forms.
    • Switching between explicit and generated IDs no longer causes a render loop.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Repository: TanStack/form/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 2e9d9ad2-746c-44a6-b363-75152d0336e3

📥 Commits

Reviewing files that changed from the base of the PR and between 555509c and 7f1262f.

📒 Files selected for processing (5)
  • .changeset/quiet-ids-return.md
  • packages/preact-form/src/useForm.tsx
  • packages/preact-form/tests/useForm.test.tsx
  • packages/react-form/src/useForm.tsx
  • packages/react-form/tests/useForm.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The React and Preact useForm hooks now track the provided formId, so changing it to undefined restores the generated ID. Regression tests cover these transitions in both adapters.

Changes

Form ID fallback

Layer / File(s) Summary
Form ID tracking and regression coverage
packages/react-form/src/useForm.tsx, packages/preact-form/src/useForm.tsx, packages/react-form/tests/useForm.test.tsx, packages/preact-form/tests/useForm.test.tsx, .changeset/quiet-ids-return.md
Both hooks record the provided ID instead of the resolved ID. Tests verify that the generated ID returns when formId becomes undefined. A changeset adds patch entries for both packages.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 7f126

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 Review

Security architecture risk: 🔵 Low · up to 7f126

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

  • Low · architecture · inferred: After an ID change, the displayed FormApi can differ from the instance whose developer-tool listeners remain mounted. Returning to the generated ID may let ID-matched requests act on the older instance.
Security review details

Security Blast Radius

  • inferred — The identified stale-owner possibility is in form instances and their developer-tool event handling in both frontend adapters. Available evidence does not establish a tenant, service, credential, or attacker-accessible boundary crossing.

Trust Boundaries and Controls

  • inferred — The inspected change does not add an authorization decision or a new developer-tool request handler. Whether an untrusted actor can reach the existing event client is not established by the available repository evidence.

Resilience and Maintainability Implications

  • inferred — If the original mount remains active after an ID change, a developer-tool request may address stale state even though the form getter reports the replacement instance's ID. The empty-dependency mount behavior predates this fix.

Hardening Proposals

  • proposed — Verify mount and cleanup ownership across committed ID changes, including a return to the generated ID and developer-tool reset and force-submit requests; bind lifecycle cleanup to the active FormApi if verification confirms stale listeners.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main fix: preventing a formId reset render loop.
Description check ✅ Passed The description explains the root cause, affected packages, behavior change, regression tests, checklist completion, and changeset release impact.
Linked Issues check ✅ Passed The PR addresses issue #2407. In both @tanstack/react-form and @tanstack/preact-form, prevFormId now stores the raw opts?.formId. The resolved formId still uses `opts?.formId ?? fallbackForm…
Out of Scope Changes check ✅ Passed All changed files support issue #2407. The source changes fix useForm state tracking, the React and Preact tests verify the regression, and the changeset documents the patch for both affected packag…
Full details: Docstring Coverage

Explanation

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.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

useAppForm causes infinite re-render loop when formId changes to undefined

1 participant