fix(settings): hoist tab-visibility useEffect above isLoading early return

Surfaced while exercising Part D (WhatsApp) end-to-end. Navigating to
Settings → WhatsApp triggered React error #310 ("Rendered more hooks
than during the previous render"). Root cause is pre-existing: the
SettingsPage redirect-to-visible-tab `useEffect` lived AFTER the
`if (isLoading) return <Loading />` early return, so on the
isLoading=true→false transition the hook count grew by one and React's
rules-of-hooks invariant blew up.

Move the effect above the early return so the hook count is stable
across renders. While here, switch the gating logic from "is the key in
the currently-visible nav list" (which the bundle couldn't reference
yet because the nav array is built lower down) to a small lookup keyed
by activeTab → matching dependency flag. That's an equivalent decision
for the four tabs we already gated (crm, contracts, reminderTemplates,
accounting) plus the new whatsapp tab.

Add `flagsLoading` from the FeatureFlags context to the deps so the
snap-back only fires once the server's actual flag values have arrived.
Without this, the initial render with the placeholder DEFAULT_FLAGS
would falsely snap away from any tab whose flag is "on" on the server
but absent from the placeholder.

Also add `whatsapp: false` to `DEFAULT_FLAGS` in FeatureFlagsContext
(was missing — TypeScript should have caught the Record<FeatureKey,
boolean> violation but the build pipeline didn't surface it). Without
this, `flags.whatsapp` is undefined on the placeholder, which had
secondary effects on tab visibility and the snap-back logic.

Verified via Chrome DevTools: Settings → WhatsApp now loads cleanly
with all 5 form fields, the saved config values prefilled, the Save
button, and the Send-test card.
This commit is contained in:
Paul Nothaft
2026-06-18 23:55:47 +02:00
parent fabd67aecd
commit 49bfb45332
2 changed files with 32 additions and 13 deletions
@@ -60,6 +60,8 @@ export const DEFAULT_FLAGS: FeatureFlags = {
// the Project Overview cockpit. Off by default — admin opts in under
// Settings → Features once they want the CRM → Overview area.
projects: false,
// WhatsApp Business API delivery channel (migration 136, #640D).
whatsapp: false,
};
export const FEATURE_FLAGS_QUERY_KEY = ['feature-flags'] as const;
+30 -13
View File
@@ -115,7 +115,7 @@ function isValidTab(value: string | null): value is TabType {
export const SettingsPage: React.FC = () => {
const { t } = useTranslation();
const [searchParams, setSearchParams] = useSearchParams();
const { flags } = useFeatureFlags();
const { flags, isLoading: flagsLoading } = useFeatureFlags();
// Read ?tab=… on mount; default to Features per the redesign.
const initialTab: TabType = isValidTab(searchParams.get('tab'))
@@ -183,6 +183,33 @@ export const SettingsPage: React.FC = () => {
saveSeoMutation,
} = useSettingsState();
// If the active tab refers to an item that's now hidden (e.g. admin
// landed on ?tab=reminderTemplates after disabling reminderEmails),
// snap to the first key that the dependency-rule flags allow. Effect
// re-fires when flags toggle live. MUST stay above the isLoading early
// return so React's rules-of-hooks count stays consistent across renders
// (was previously after the early return — that's a hooks violation that
// surfaced as React error #310 once settled long enough for `isLoading`
// to transition true→false in the same mount, #640D pre-existing-bug fix).
useEffect(() => {
// Wait for the server's actual flag values before deciding whether the
// current tab is allowed — during initial load `flags` is the defaults
// placeholder which would falsely snap-back away from a tab the server
// has actually enabled.
if (flagsLoading) return;
const gatedOff: Record<string, boolean> = {
crm: !(flags.quotes || flags.bills || flags.contracts),
contracts: !flags.contracts,
reminderTemplates: !flags.reminderEmails,
accounting: !flags.accounting,
whatsapp: !flags.whatsapp,
};
if (gatedOff[activeTab]) {
setActiveTab('features');
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [flagsLoading, flags.quotes, flags.bills, flags.contracts, flags.reminderEmails, flags.accounting, flags.whatsapp, activeTab]);
if (isLoading) {
return (
<div className="flex items-center justify-center min-h-[400px]">
@@ -275,18 +302,8 @@ export const SettingsPage: React.FC = () => {
const allItems = navGroups.flatMap((g) => g.items);
const activeItem = allItems.find((i) => i.key === activeTab) ?? allItems[0];
// If the active tab refers to an item that's now hidden (e.g. admin
// landed on ?tab=reminderTemplates after disabling reminderEmails),
// snap to the first visible item so the content area doesn't render
// a hidden tab's UI. Effect re-fires when flags toggle live.
useEffect(() => {
const visibleKeys = allItems.map((i) => i.key);
if (!visibleKeys.includes(activeTab) && visibleKeys.length > 0) {
setActiveTab(visibleKeys[0]);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [flags.quotes, flags.bills, flags.contracts, flags.reminderEmails, flags.accounting, activeTab]);
// (Visibility snap-back is handled in the useEffect above, which sits
// before the isLoading early return to keep hook ordering stable.)
// For tabs that mount existing top-level pages OR bring their own
// header (FeaturesTab has its own icon+title+description block), skip