letsbe/pn-new-crm
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(compiler): React Compiler safety triage — 5 categories cleared

Browse Source 
Cleared 4 rule buckets (37 violations, including 5 real bugs) and
silenced 1 informational bucket from the Next 16 / react-hooks v7
upgrade. Cleared rules promoted from `warn` back to `error` so new
regressions block CI.

Real bug fixes:
- `interest-contact-log-tab.tsx`: `useMemo` used for side effects
  (5 setState calls inside a memo body); converted to `useEffect`.
- `PieChart.tsx`: cumulative `let angle` mutation in a render-phase
  `map`; converted to `reduce` so the slice array is built without
  re-assignment.
- `documents-hub.tsx`: `useMemo(() => ({ count: 0 }))` used as a
  mutable drag counter; converted to `useRef`.
- `notes-list.tsx`: `Date.now()` read during render for note-edit
  countdown (impure) → pinned to a `now` state ticked every 30s.
- `onboarding-checklist.tsx` / `user-profile.tsx` /
  `user-settings.tsx`: `useEffect(() => void load(), [])` with the
  `load` function declared AFTER the effect — relied on hoisting,
  trips Compiler's "access before declared" rule. Declared inside
  the effect.

Pattern fixes (intentional cache-via-ref → state or layout-effect):
- 6 `ref.current = x` writes during render moved into layout
  effects (`use-realtime-invalidation`, `settings-form-card`,
  `inbox`).
- 3 `ref.current` reads during render (search totals cache,
  scanner file ref) rewritten to backed-by-state.
- `use-is-mobile.ts` rewritten on `useSyncExternalStore` to avoid
  the SSR-then-rehydrate setState dance.
- `use-notifications.ts` rewritten to write socket pushes directly
  into the React Query cache via `setQueryData`, removing a local
  state mirror.

Rule config (`eslint.config.mjs`):
- `react-hooks/purity` → error (was warn, cleared)
- `react-hooks/set-state-in-render` → error (was warn, cleared)
- `react-hooks/immutability` → error (was warn, cleared)
- `react-hooks/refs` → error (was warn, cleared)
- `react-hooks/incompatible-library` → off (informational only)
- `react-hooks/set-state-in-effect` → warn (51 remaining, all the
  useEffect→fetch→setState data-fetch pattern; migration to
  useQuery tracked in BACKLOG)

Verified: tsc clean, eslint 0 errors / 69 warnings (down from 105),
vitest 1315/1315, next build green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Matt
2026-05-12 23:14:16 +02:00
parent ba1db2afea
commit 4329db7fc3
17 changed files with 208 additions and 166 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
src/components/admin/onboarding-checklist.tsx
View File

@@ -113,45 +113,44 @@ export function OnboardingChecklist() {
const [saving, setSaving] = useState<string | null>(null);
useEffect(() => {
async function load() {
setLoading(true);
try {
const settings = await apiFetch<SettingsResp>('/api/v1/admin/settings');
const all = [...settings.data.portSettings, ...settings.data.globalSettings];
const byKey = new Map(all.map((r) => [r.key, r.value]));
const checks: Record<string, boolean> = {};
const listChecks = await Promise.all(
STEPS.map(async (s) => {
if (s.autoCheckSettingKey) {
const v = byKey.get(s.autoCheckSettingKey);
return [s.id, v !== undefined && v !== null && v !== '' && v !== false] as const;
}
if (s.autoCheckListEndpoint) {
try {
const res = await apiFetch<{ data: unknown[] }>(s.autoCheckListEndpoint);
return [s.id, Array.isArray(res.data) && res.data.length > 0] as const;
} catch {
return [s.id, false] as const;
}
}
return [s.id, false] as const;
}),
);
for (const [id, done] of listChecks) checks[id] = done;
setAutoChecks(checks);
// Pull the manual-checkbox state from system_settings.
const manual = (byKey.get('onboarding_manual_status') ?? {}) as Record<string, boolean>;
setManualChecks(manual);
} finally {
setLoading(false);
}
}
void load();
}, []);
async function load() {
setLoading(true);
try {
const settings = await apiFetch<SettingsResp>('/api/v1/admin/settings');
const all = [...settings.data.portSettings, ...settings.data.globalSettings];
const byKey = new Map(all.map((r) => [r.key, r.value]));
const checks: Record<string, boolean> = {};
const listChecks = await Promise.all(
STEPS.map(async (s) => {
if (s.autoCheckSettingKey) {
const v = byKey.get(s.autoCheckSettingKey);
return [s.id, v !== undefined && v !== null && v !== '' && v !== false] as const;
}
if (s.autoCheckListEndpoint) {
try {
const res = await apiFetch<{ data: unknown[] }>(s.autoCheckListEndpoint);
return [s.id, Array.isArray(res.data) && res.data.length > 0] as const;
} catch {
return [s.id, false] as const;
}
}
return [s.id, false] as const;
}),
);
for (const [id, done] of listChecks) checks[id] = done;
setAutoChecks(checks);
// Pull the manual-checkbox state from system_settings.
const manual = (byKey.get('onboarding_manual_status') ?? {}) as Record<string, boolean>;
setManualChecks(manual);
} finally {
setLoading(false);
}
}
async function toggleManual(id: string) {
const next = { ...manualChecks, [id]: !manualChecks[id] };
setManualChecks(next);

7
src/components/admin/shared/settings-form-card.tsx
View File

@@ -82,9 +82,12 @@ export function SettingsFormCard({ title, description, fields, extra }: Settings
// Parent components often pass `FIELDS.slice(0, 5)` directly, so the prop
// reference changes on every render. Capture it in a ref so the fetch
// callback can read the current list without being re-created and looping
// through useEffect forever.
// through useEffect forever. Update inside an effect — writing to ref
// .current during render trips the React Compiler purity rules.
const fieldsRef = useRef(fields);
fieldsRef.current = fields;
useEffect(() => {
fieldsRef.current = fields;
});
const fetchValues = useCallback(async () => {
setLoading(true);

48
src/components/documents/documents-hub.tsx
View File

@@ -1,6 +1,6 @@
'use client';
import { useCallback, useEffect, useMemo, useState } from 'react';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import Link from 'next/link';
import { useQueryClient } from '@tanstack/react-query';
import { ChevronDown, ChevronRight, FileText, Plus, Upload } from 'lucide-react';
@@ -478,33 +478,29 @@ interface FolderDropZoneProps {
function FolderDropZone({ folderId, entityType, entityId, children }: FolderDropZoneProps) {
const [dragActive, setDragActive] = useState(false);
const [uploading, setUploading] = useState(false);
const dragCounter = useMemo(() => ({ count: 0 }), []);
// useRef for mutable per-drag counters — useMemo's value is supposed
// to be immutable; React Compiler flags writes as a bug class.
const dragCounter = useRef(0);
const queryClient = useQueryClient();
const portId = useUIStore((s) => s.currentPortId);
const onDragEnter = useCallback(
(e: React.DragEvent<HTMLDivElement>) => {
// Only react to drags that carry files. Avoids fighting with
// text-selection / element drags inside the listing.
if (!Array.from(e.dataTransfer.types).includes('Files')) return;
e.preventDefault();
dragCounter.count += 1;
if (dragCounter.count === 1) setDragActive(true);
},
[dragCounter],
);
const onDragEnter = useCallback((e: React.DragEvent<HTMLDivElement>) => {
// Only react to drags that carry files. Avoids fighting with
// text-selection / element drags inside the listing.
if (!Array.from(e.dataTransfer.types).includes('Files')) return;
e.preventDefault();
dragCounter.current += 1;
if (dragCounter.current === 1) setDragActive(true);
}, []);
const onDragLeave = useCallback(
(e: React.DragEvent<HTMLDivElement>) => {
if (!Array.from(e.dataTransfer.types).includes('Files')) return;
dragCounter.count -= 1;
if (dragCounter.count <= 0) {
dragCounter.count = 0;
setDragActive(false);
}
},
[dragCounter],
);
const onDragLeave = useCallback((e: React.DragEvent<HTMLDivElement>) => {
if (!Array.from(e.dataTransfer.types).includes('Files')) return;
dragCounter.current -= 1;
if (dragCounter.current <= 0) {
dragCounter.current = 0;
setDragActive(false);
}
}, []);
const onDragOver = useCallback((e: React.DragEvent<HTMLDivElement>) => {
if (!Array.from(e.dataTransfer.types).includes('Files')) return;
@@ -516,7 +512,7 @@ function FolderDropZone({ folderId, entityType, entityId, children }: FolderDrop
async (e: React.DragEvent<HTMLDivElement>) => {
if (!Array.from(e.dataTransfer.types).includes('Files')) return;
e.preventDefault();
dragCounter.count = 0;
dragCounter.current = 0;
setDragActive(false);
const files = Array.from(e.dataTransfer.files);
if (files.length === 0) return;
@@ -549,7 +545,7 @@ function FolderDropZone({ folderId, entityType, entityId, children }: FolderDrop
setUploading(false);
}
},
[dragCounter, folderId, entityType, entityId, portId, queryClient],
[folderId, entityType, entityId, portId, queryClient],
);
return (

7
src/components/interests/interest-contact-log-tab.tsx
View File

@@ -1,6 +1,6 @@
'use client';
import { useMemo, useState } from 'react';
import { useEffect, useMemo, useState } from 'react';
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
import {
Bell,
@@ -285,8 +285,9 @@ function ComposeDialog({
);
// Re-sync local state when the existing entry changes (e.g. opening
// the edit dialog for a different row).
useMemo(() => {
// the edit dialog for a different row). useEffect, not useMemo —
// setState in render is a Compiler red flag.
useEffect(() => {
if (open) {
setOccurredAt(
existing ? localIsoString(existing.occurredAt) : localIsoString(new Date().toISOString()),

4
src/components/layout/inbox.tsx
View File

@@ -106,7 +106,9 @@ export function Inbox() {
// user any time a new socket event re-derived the initial tab while
// they were actively reading the other one.
const initialTabRef = useRef(initialTab);
initialTabRef.current = initialTab;
useEffect(() => {
initialTabRef.current = initialTab;
});
useEffect(() => {
if (open) setActiveTab(initialTabRef.current);
}, [open]);

9
src/components/scan/scan-shell.tsx
View File

@@ -328,6 +328,11 @@ export function ScanShell() {
const fileRef = useRef<HTMLInputElement>(null);
const [state, setState] = useState<ScanState>({ kind: 'idle' });
const [imagePreview, setImagePreview] = useState<string | null>(null);
// Track the (possibly compressed) File alongside its preview URL.
// Reading fileRef.current.files[0] during render trips the React
// Compiler's ref-purity rule + would point at the *raw* uploaded
// bytes (pre-compression), not the resized bytes we OCR'd.
const [currentFile, setCurrentFile] = useState<File | null>(null);
// Revoke blob URL on unmount.
useEffect(() => {
@@ -345,6 +350,7 @@ export function ScanShell() {
const file = await compressReceiptIfHeavy(rawFile);
if (imagePreview) URL.revokeObjectURL(imagePreview);
setImagePreview(URL.createObjectURL(file));
setCurrentFile(file);
setState({ kind: 'processing', engine: 'tesseract' });
// Always run Tesseract first - it's free, on-device, and gives us a
@@ -482,6 +488,7 @@ export function ScanShell() {
URL.revokeObjectURL(imagePreview);
setImagePreview(null);
}
setCurrentFile(null);
setState({ kind: 'idle' });
if (fileRef.current) fileRef.current.value = '';
}
@@ -563,7 +570,7 @@ export function ScanShell() {
<VerifyForm
parsed={state.parsed}
imagePreview={imagePreview}
imageFile={fileRef.current?.files?.[0] as File}
imageFile={currentFile as File}
source={state.source}
reason={state.reason}
providerError={state.providerError}

9
src/components/search/command-search.tsx
View File

@@ -123,15 +123,16 @@ export function CommandSearch() {
// populated when the user narrows to a single bucket. Without this, the
// narrowed query only returns counts for the active bucket and every
// other chip would vanish — making it impossible to swap between
// filters without clearing back to "All" first.
const lastAllTotalsRef = useRef<SearchResults['totals'] | null>(null);
// filters without clearing back to "All" first. Backed by state (not
// a ref) so render reads are pure — React Compiler-safe.
const [lastAllTotals, setLastAllTotals] = useState<SearchResults['totals'] | null>(null);
useEffect(() => {
if (activeBucket === 'all' && results?.totals) {
lastAllTotalsRef.current = results.totals;
setLastAllTotals(results.totals);
}
}, [activeBucket, results]);
const chipTotals: SearchResults['totals'] | undefined =
activeBucket === 'all' ? results?.totals : (lastAllTotalsRef.current ?? results?.totals);
activeBucket === 'all' ? results?.totals : (lastAllTotals ?? results?.totals);
const showDropdown = focused;

9
src/components/search/mobile-search-overlay.tsx
View File

@@ -93,15 +93,16 @@ export function MobileSearchOverlay({ open, onOpenChange }: MobileSearchOverlayP
// Persist counts from the last "all" query so chip counts stay visible
// when the user narrows to a single bucket. Narrowed queries only
// return counts for the active bucket, which would otherwise wipe the
// counts off every other chip the moment the user taps one.
const lastAllTotalsRef = useRef<SearchResults['totals'] | null>(null);
// counts off every other chip the moment the user taps one. Backed
// by state, not a ref, so render reads are pure (Compiler-safe).
const [lastAllTotals, setLastAllTotals] = useState<SearchResults['totals'] | null>(null);
useEffect(() => {
if (activeBucket === 'all' && results?.totals) {
lastAllTotalsRef.current = results.totals;
setLastAllTotals(results.totals);
}
}, [activeBucket, results]);
const chipTotals: SearchResults['totals'] | undefined =
activeBucket === 'all' ? results?.totals : (lastAllTotalsRef.current ?? results?.totals);
activeBucket === 'all' ? results?.totals : (lastAllTotals ?? results?.totals);
// Auto-focus is delegated to Vaul's `autoFocus` + the input's
// `autoFocus` attribute (synchronous in-gesture, which iOS Safari

11
src/components/settings/user-profile.tsx
View File

@@ -35,15 +35,14 @@ export function UserProfile() {
} | null>(null);
useEffect(() => {
async function load() {
const res = await apiFetch<{ data: { user?: MeUser } }>('/api/v1/me');
setMe(res.data.user ?? null);
setDisplayName(res.data.user?.name ?? '');
}
void load();
}, []);
async function load() {
const res = await apiFetch<{ data: { user?: MeUser } }>('/api/v1/me');
setMe(res.data.user ?? null);
setDisplayName(res.data.user?.name ?? '');
}
async function saveProfile() {
setSavingProfile(true);
setProfileMessage(null);

43
src/components/settings/user-settings.tsx
View File

@@ -67,29 +67,28 @@ export function UserSettings() {
}, []);
useEffect(() => {
async function loadProfile() {
const res = await apiFetch<{ data: MeResponse }>('/api/v1/me', { method: 'GET' });
setFirstName(res.data.profile?.firstName ?? '');
setLastName(res.data.profile?.lastName ?? '');
// Display name is the override; fall back to user.name if profile
// doesn't carry one (e.g. legacy rows pre-Wave 10).
setDisplayName(res.data.profile?.displayName ?? res.data.user?.name ?? '');
setEmail(res.data.user?.email ?? '');
setOriginalEmail(res.data.user?.email ?? '');
setUsername(res.data.profile?.username ?? '');
setOriginalUsername(res.data.profile?.username ?? '');
setCountry(res.data.preferences?.country ?? null);
// Fall back to the browser-detected zone when no value has been
// saved yet — first-time users land on a sensible default rather
// than an empty picker. Doesn't overwrite an explicit choice.
setTimezone(res.data.preferences?.timezone ?? detectedTz ?? null);
const fid = res.data.profile?.avatarFileId ?? null;
setAvatarFileId(fid);
setAvatarUrl(fid ? `/api/v1/files/${fid}/preview` : null);
}
void loadProfile();
}, []);
async function loadProfile() {
const res = await apiFetch<{ data: MeResponse }>('/api/v1/me', { method: 'GET' });
setFirstName(res.data.profile?.firstName ?? '');
setLastName(res.data.profile?.lastName ?? '');
// Display name is the override; fall back to user.name if profile
// doesn't carry one (e.g. legacy rows pre-Wave 10).
setDisplayName(res.data.profile?.displayName ?? res.data.user?.name ?? '');
setEmail(res.data.user?.email ?? '');
setOriginalEmail(res.data.user?.email ?? '');
setUsername(res.data.profile?.username ?? '');
setOriginalUsername(res.data.profile?.username ?? '');
setCountry(res.data.preferences?.country ?? null);
// Fall back to the browser-detected zone when no value has been saved
// yet — first-time users land on a sensible default rather than an
// empty picker. Doesn't overwrite an explicit choice.
setTimezone(res.data.preferences?.timezone ?? detectedTz ?? null);
const fid = res.data.profile?.avatarFileId ?? null;
setAvatarFileId(fid);
setAvatarUrl(fid ? `/api/v1/files/${fid}/preview` : null);
}
}, [detectedTz]);
// When the user picks a country and no timezone is set, suggest the
// primary zone for that country. Doesn't fight an explicit timezone

15
src/components/shared/notes-list.tsx
View File

@@ -1,6 +1,6 @@
'use client';
import { useState } from 'react';
import { useEffect, useState } from 'react';
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
import { formatDistanceToNow } from 'date-fns';
import { Lock, Pencil, Trash2, Send, Loader2 } from 'lucide-react';
@@ -128,6 +128,15 @@ export function NotesList({ entityType, entityId, currentUserId, aggregate }: No
const [editingId, setEditingId] = useState<string | null>(null);
const [editContent, setEditContent] = useState('');
const [groupBySource, setGroupBySource] = useState(false);
// Wall-clock 'now' ticked every 30s so the per-note "Xm left to edit"
// countdown decrements on screen. Reading `Date.now()` directly inside
// render is impure (different value every call); pinning to a state
// value means React Compiler can memoize cleanly.
const [now, setNow] = useState(() => Date.now());
useEffect(() => {
const id = setInterval(() => setNow(Date.now()), 30_000);
return () => clearInterval(id);
}, []);
const aggregateOn = !!aggregate && AGGREGATABLE.has(entityType);
const baseEndpoint = `/api/v1/${entityType}/${entityId}/notes`;
@@ -179,12 +188,12 @@ export function NotesList({ entityType, entityId, currentUserId, aggregate }: No
// owning entity's timeline records the change.
const selfSource = SELF_SOURCE[entityType];
if (aggregateOn && note.source && note.source !== selfSource) return false;
const elapsed = Date.now() - new Date(note.createdAt).getTime();
const elapsed = now - new Date(note.createdAt).getTime();
return elapsed < NOTE_EDIT_WINDOW_MS;
}
function getTimeRemaining(note: Note): string | null {
const elapsed = Date.now() - new Date(note.createdAt).getTime();
const elapsed = now - new Date(note.createdAt).getTime();
const remaining = NOTE_EDIT_WINDOW_MS - elapsed;
if (remaining <= 0) return null;
const mins = Math.ceil(remaining / 60000);