Fix "Maximum update depth exceeded" in auth flow components (#3697)#17
Fix "Maximum update depth exceeded" in auth flow components (#3697)#17webdevelopersrinu wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe auth presentation components now memoize their derived ChangesMemoize formFields across auth components
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/react/src/components/presentation/auth/Recovery/BaseRecovery.tsx (1)
232-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStatic analysis flags unsafe-any on this memo (pre-existing typing gap).
ESLint reports unsafe assignment/member-access/call on
currentFlow?.data?.components. This stems fromcurrentFlow's loosely-typed.datashape rather than the newuseMemowrapper itself, but it's worth tightening the type ofEmbeddedRecoveryFlowResponse.data(or narrowing with a type guard) to eliminate theanyleakage, especially since the PR verification notes claim no new lint issues.🤖 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/react/src/components/presentation/auth/Recovery/BaseRecovery.tsx` around lines 232 - 235, The unsafe-any warning comes from the loosely typed recovery flow payload used by BaseRecovery’s useMemo, not from the memo itself. Tighten the type of EmbeddedRecoveryFlowResponse.data so currentFlow.data.components is strongly typed, or add an explicit type guard/narrowing before calling extractFormFields in BaseRecovery, and ensure the currentFlow access path no longer leaks any into the memo.Source: Linters/SAST tools
packages/react/src/components/presentation/auth/SignUp/BaseSignUp.tsx (2)
461-465: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStatic analysis unsafe-any flags on the touched lines.
Same as
BaseRecovery, ESLint flags unsafe assignment/member-access due tocurrentFlow's loosely-typed.data. Pre-existing typing gap, not newly introduced by this refactor's logic, but worth tightening the response type to remove theanycasts long-term.🤖 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/react/src/components/presentation/auth/SignUp/BaseSignUp.tsx` around lines 461 - 465, The memoized form field extraction in BaseSignUp is still relying on unsafe any casts against currentFlow.data, which triggers static-analysis unsafe-assignment/member-access flags. Tighten the typing of the flow response/data shape used in the useMemo block and extractFormFields call so the components property can be accessed without any assertions, following the same approach used in BaseRecovery.Source: Linters/SAST tools
458-465: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMemoization logic is correct; minor style nit on the non-null assertion.
currentFlowis state, so theuseMemodependency is stable across renders like inBaseRecovery. The expression itself is a bit convoluted —currentFlow!.data as anyafter already null-checking viacurrentFlow?.data as any.BaseRecovery's equivalent (currentFlow?.data?.components ? extractFormFields(currentFlow.data.components) : []) is cleaner and avoids the non-null assertion; consider aligning both implementations for consistency.♻️ Suggested cleanup
const formFields: FormField[] = useMemo( - () => - (currentFlow?.data as any)?.components ? extractFormFields((currentFlow!.data as any).components) : [], + () => (currentFlow?.data as any)?.components ? extractFormFields((currentFlow?.data as any).components) : [], [currentFlow, extractFormFields], );🤖 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/react/src/components/presentation/auth/SignUp/BaseSignUp.tsx` around lines 458 - 465, The memoization logic in `BaseSignUp` is fine, but the `formFields` derivation uses a redundant non-null assertion (`currentFlow!.data as any`) after already guarding with optional chaining. Update the `useMemo` expression in `BaseSignUp` to match the cleaner pattern used in `BaseRecovery`, keeping the same behavior while removing the unnecessary assertion and improving consistency.
🤖 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/react/src/components/presentation/auth/SignIn/BaseSignIn.tsx`:
- Around line 354-362: The `formFields` memo in `BaseSignIn` still depends on an
inline empty array when `components` is missing, which creates a new reference
on every render and breaks the stability fix. Hoist a shared empty-array
sentinel (for example a module-level constant) and use it in the `components`
fallback so `useMemo` can preserve identity across renders. Update the
`BaseSignIn` logic where `components`, `extractFormFields`, and `formFields` are
derived.
---
Nitpick comments:
In `@packages/react/src/components/presentation/auth/Recovery/BaseRecovery.tsx`:
- Around line 232-235: The unsafe-any warning comes from the loosely typed
recovery flow payload used by BaseRecovery’s useMemo, not from the memo itself.
Tighten the type of EmbeddedRecoveryFlowResponse.data so
currentFlow.data.components is strongly typed, or add an explicit type
guard/narrowing before calling extractFormFields in BaseRecovery, and ensure the
currentFlow access path no longer leaks any into the memo.
In `@packages/react/src/components/presentation/auth/SignUp/BaseSignUp.tsx`:
- Around line 461-465: The memoized form field extraction in BaseSignUp is still
relying on unsafe any casts against currentFlow.data, which triggers
static-analysis unsafe-assignment/member-access flags. Tighten the typing of the
flow response/data shape used in the useMemo block and extractFormFields call so
the components property can be accessed without any assertions, following the
same approach used in BaseRecovery.
- Around line 458-465: The memoization logic in `BaseSignUp` is fine, but the
`formFields` derivation uses a redundant non-null assertion (`currentFlow!.data
as any`) after already guarding with optional chaining. Update the `useMemo`
expression in `BaseSignUp` to match the cleaner pattern used in `BaseRecovery`,
keeping the same behavior while removing the unnecessary assertion and improving
consistency.
🪄 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 Plus
Run ID: 133c9187-2306-40a7-863c-4d9a56c0819b
📒 Files selected for processing (3)
packages/react/src/components/presentation/auth/Recovery/BaseRecovery.tsxpackages/react/src/components/presentation/auth/SignIn/BaseSignIn.tsxpackages/react/src/components/presentation/auth/SignUp/BaseSignUp.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/src/components/presentation/auth/SignIn/BaseSignIn.tsx (1)
349-352: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant truthiness check now that
componentsdefaults toEMPTY_COMPONENTS.Since
componentsalways resolves to at leastEMPTY_COMPONENTS(neverundefined/null), thecomponents ? ... : []ternary's false branch is unreachable dead code.♻️ Simplify
const formFields: FormField[] = useMemo( - () => (components ? extractFormFields(components) : []), + () => extractFormFields(components), [components, extractFormFields], );🤖 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/react/src/components/presentation/auth/SignIn/BaseSignIn.tsx` around lines 349 - 352, In BaseSignIn’s useMemo for formFields, remove the redundant `components ? ... : []` guard because `components` always falls back to `EMPTY_COMPONENTS`; simplify the expression to always derive fields from `extractFormFields(components)`. Keep the dependency list unchanged and ensure the `formFields` initialization in `BaseSignIn` still uses the existing `components` and `extractFormFields` references.
🤖 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.
Nitpick comments:
In `@packages/react/src/components/presentation/auth/SignIn/BaseSignIn.tsx`:
- Around line 349-352: In BaseSignIn’s useMemo for formFields, remove the
redundant `components ? ... : []` guard because `components` always falls back
to `EMPTY_COMPONENTS`; simplify the expression to always derive fields from
`extractFormFields(components)`. Keep the dependency list unchanged and ensure
the `formFields` initialization in `BaseSignIn` still uses the existing
`components` and `extractFormFields` references.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 506e4aae-3422-4275-a044-5cdb99772d83
📒 Files selected for processing (2)
packages/react/src/components/presentation/auth/SignIn/BaseSignIn.tsxpackages/react/src/components/presentation/auth/SignUp/BaseSignUp.tsx
|
Hey @webdevelopersrinu! Thanks a lot for the PR. Really appreciate the effor. Could you kindly share a couple of screenshots to validate the fix.
Cheers! 🙌 |
2 participants
Summary
Fixes thunder-id/thunderid#3697 —
@thunderid/reactthrows a continuous "Maximum update depth exceeded" error during flow rendering.Root cause
In
BaseSignIn,formFieldswas rebuilt as a new array on every render and passed touseForm({ fields }). That new identity cascades through the hook:fields→getFieldConfig([fields]) →validateField→setTouchedso
setTouchedgets a new identity every render.setTouchedis a dependency of theuseEffectthat projects server errors, and that effect callsclearFormErrors()unconditionally (which doessetFormErrors({}), a fresh object). The result is an infinite loop:render → effect fires →
clearFormErrors()→ re-render → newformFields→ newsetTouched→ effect fires again → …Fix
Memoize the derived field list so its identity is stable across renders:
The same unmemoized pattern existed in
BaseSignUpandBaseRecovery, so all three are fixed consistently.Changes
BaseSignIn.tsx— memoizeformFieldsBaseSignUp.tsx— memoizeformFieldsBaseRecovery.tsx— memoizeformFieldsVerification
tsctypecheck passes cleanreact-hooks/exhaustive-depswarningsNotes
useMemoreturns the identical array contents as before — only the reference identity is now stable — so there is no behavioral change beyond eliminating the render loop.Summary by CodeRabbit