Skip to content

fix: seed enterprise sso form with defined defaults to avoid controlled warnings (#2095)#2120

Open
larryro wants to merge 2 commits into
mainfrom
tale/xs76fw5vjnm1qmb63ekjbt829s8998fr
Open

fix: seed enterprise sso form with defined defaults to avoid controlled warnings (#2095)#2120
larryro wants to merge 2 commits into
mainfrom
tale/xs76fw5vjnm1qmb63ekjbt829s8998fr

Conversation

@larryro

@larryro larryro commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #2095 — the Enterprise SSO settings form logged five React "changing from uncontrolled to controlled" warnings on load (2× Select, 3× Switch).

Root cause

EnterpriseSsoForm's data useMemo returns undefined while the connection config is loading, and useFormEditor passed that straight to RHF's defaultValues. The controlled Select/Switch inputs therefore mounted with value/checked = undefined (uncontrolled), then flipped to controlled once data resolved and the form reset.

Fix

  • useFormEditor gains an optional defaultValues argument used purely for RHF's initial defaultValues (read once at mount). data stays the loading sentinel, so isLoading / dirty-suppression / the remote-update reset logic are unchanged.
  • EnterpriseSsoForm passes a fully-defined EMPTY_FORM_DATA (empty strings for text/Selects, booleans for Switches) so every control is controlled from the first render.

Verification

  • bunx tsc --noEmit (platform): clean.
  • bunx oxlint --type-aware on touched files: 0 warnings, 0 errors.
  • oxfmt --check: clean.
  • Unit tests (vitest.ui.config.ts): use-form-editor 10/10 (added a regression test asserting defined defaults while loading) and existing enterprise-sso suite 8/8.

@larryro

larryro commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Desk Review — #2095 (uncontrolled→controlled warnings on Enterprise SSO form)

Verdict: READY TO MERGE.

CI: All checks green (gh pr checks — Build platform, type-aware Lint, Knip, Format, Browser, UI, all 16 Playwright platform shards, etc.; "Docs container test" skipped as expected). No red, no pending.

Tests run locally (once): bunx vitest --run --config vitest.ui.config.ts app/components/ui/editor/use-form-editor.test.tsx10 passed.

What the fix does

  • use-form-editor.ts: adds an optional defaultValues arg and changes defaultValues: data as …defaultValues: (data ?? defaultValues) as …. RHF reads defaultValues once at mount, so while data is undefined the form is seeded with defined values; the existing useEffect still form.reset(data)s once data arrives.
  • enterprise-sso-form.tsx: adds EMPTY_FORM_DATA: SsoFormData (every field defined) and passes it as defaultValues.

Why it's correct (verified across all branches)

  • Resolves all 5 cited warnings. Each Select/Switch field now mounts controlled: protocol'entra-id' (Select), defaultRole'member' (Select), pkcetrue, autoRolefalse, autoTeamfalse (Switches). No controlled input can mount undefined.
  • Production path is real: enterprise-sso-settings.tsx passes config=undefined during load straight into the form, so the targeted loading window is genuinely hit.
  • No collateral on the shared hook. defaultValues is optional; for the ~20 other useFormEditor callers data ?? undefined is byte-for-byte the prior behavior. isLoading (data === undefined), the isDirty suppression-while-loading, hasInitializedRef/lastDataRef, and the Convex baseline-drift / remote-update logic are all untouched — the seed is inert after the first form.reset(data).
  • Type-safe & DRY-safe: EMPTY_FORM_DATA: SsoFormData forces the compiler to cover every field if the interface grows; type-aware Lint is green.
  • Genuine regression guard: reverting the one-line change makes the new test fail (getValues returns undefined) — confirmed empirically.

Non-blocking observations (optional, do not gate merge)

  1. The new test asserts defined values rather than spying on console.error/warn for the actual "uncontrolled to controlled" string — a vi.spyOn(console,'error') assertion would tie the test more directly to the symptom.
  2. Coverage is hook-level only; no component-level test renders EnterpriseSsoForm through the config=undefined→loaded transition. Acceptable given the hook test pins the mechanism and Browser/Playwright CI is green.

The related "protocol-switch validation gap [08]" is correctly out of scope here.

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.

Bug: Enterprise SSO settings form logs React uncontrolled→controlled warnings (Select + Switch initialized undefined)

1 participant