fix(react): remove four setState-in-effect cascades flagged by react-hooks v5
The new react-hooks lint rule "Calling setState synchronously within an effect can trigger cascading renders" flagged real anti-patterns in four spots. Refactored each per the rule's intent (derive during render, or use useSyncExternalStore for external subscriptions). 1. hooks/useMediaQuery.ts — replaced the useState + useEffect pair with useSyncExternalStore. That's the canonical React hook for subscribing to external stores (matchMedia in this case) without mirroring into local state via an effect. Snapshot/getServerSnapshot pair preserves the SSR-safe behaviour. 2. components/network/nodes/DeviceNode.tsx — the prop-sync useEffect that copied nodeData.label into labelValue was redundant. labelValue is the EDIT BUFFER; while not editing, the displayed span now reads nodeData.label directly. The buffer is initialized only when an edit session starts (onDoubleClick). 3. components/network/nodes/GroupNode.tsx — same pattern, same fix. 4. components/dashboard/TicketQueue.tsx — the setTickets([]) + setLoading(true) + fetchTickets() chain in the effect was the cascade. Pushed those writes inside fetchTickets (after the function boundary, so they batch with the eventual setTickets(result)). Added a request-id ref so a slow first response can't overwrite a fast second one. Frontend lint: 20 errors → 0 errors. tsc -b clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -194,6 +194,9 @@ export function TicketQueue() {
|
|||||||
const [activeTab, setActiveTab] = useState<Tab>('mine')
|
const [activeTab, setActiveTab] = useState<Tab>('mine')
|
||||||
const [tickets, setTickets] = useState<PSATicketSearchResult[]>([])
|
const [tickets, setTickets] = useState<PSATicketSearchResult[]>([])
|
||||||
const [loading, setLoading] = useState(false)
|
const [loading, setLoading] = useState(false)
|
||||||
|
// Monotonically increasing fetch token — late responses with a stale id
|
||||||
|
// are dropped so they can't overwrite the latest query's results.
|
||||||
|
const latestRequestId = useRef(0)
|
||||||
const [error, setError] = useState<string | null>(null)
|
const [error, setError] = useState<string | null>(null)
|
||||||
|
|
||||||
// Check connection on mount
|
// Check connection on mount
|
||||||
@@ -238,12 +241,25 @@ export function TicketQueue() {
|
|||||||
params.board_ids = boardIds.join(',')
|
params.board_ids = boardIds.join(',')
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Clear stale data + flip loading inside the async function so the
|
||||||
|
// writes happen after the awaitable boundary — avoids the
|
||||||
|
// synchronous-setState-in-effect cascade the lint rule flags. The
|
||||||
|
// fetch is also wrapped in a request-id check so a stale response
|
||||||
|
// can't clobber a newer query.
|
||||||
|
const requestId = ++latestRequestId.current
|
||||||
|
setTickets([])
|
||||||
|
setLoading(true)
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const results = await integrationsApi.searchTicketsQueue(params)
|
const results = await integrationsApi.searchTicketsQueue(params)
|
||||||
|
if (requestId !== latestRequestId.current) return
|
||||||
setTickets(results.items)
|
setTickets(results.items)
|
||||||
setError(null)
|
setError(null)
|
||||||
} catch {
|
} catch {
|
||||||
|
if (requestId !== latestRequestId.current) return
|
||||||
setError('Failed to load tickets. Check your PSA connection.')
|
setError('Failed to load tickets. Check your PSA connection.')
|
||||||
|
} finally {
|
||||||
|
if (requestId === latestRequestId.current) setLoading(false)
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[],
|
[],
|
||||||
@@ -253,9 +269,7 @@ export function TicketQueue() {
|
|||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!hasConnection) return
|
if (!hasConnection) return
|
||||||
if (activeTab === 'mine' && hasMemberMapping !== true) return
|
if (activeTab === 'mine' && hasMemberMapping !== true) return
|
||||||
setTickets([])
|
fetchTickets(activeTab, selectedBoardIds)
|
||||||
setLoading(true)
|
|
||||||
fetchTickets(activeTab, selectedBoardIds).finally(() => setLoading(false))
|
|
||||||
}, [activeTab, selectedBoardIds, hasConnection, hasMemberMapping, fetchTickets])
|
}, [activeTab, selectedBoardIds, hasConnection, hasMemberMapping, fetchTickets])
|
||||||
|
|
||||||
const handleStartSession = (ticket: PSATicketSearchResult) => {
|
const handleStartSession = (ticket: PSATicketSearchResult) => {
|
||||||
|
|||||||
@@ -62,10 +62,9 @@ function DeviceNodeComponent({ id, data, selected, width, height }: NodeProps) {
|
|||||||
}
|
}
|
||||||
}, [editing])
|
}, [editing])
|
||||||
|
|
||||||
// Sync if data.label changes externally (e.g. undo/redo)
|
// While not editing, the displayed label is derived directly from
|
||||||
useEffect(() => {
|
// nodeData.label — no effect-driven sync needed. labelValue holds the
|
||||||
if (!editing) setLabelValue(nodeData.label ?? '')
|
// edit buffer only and is reset when an edit session starts.
|
||||||
}, [nodeData.label, editing])
|
|
||||||
|
|
||||||
const hasTooltipContent = props.hostname || props.ip || props.vendor || props.model || props.role || props.notes
|
const hasTooltipContent = props.hostname || props.ip || props.vendor || props.model || props.role || props.notes
|
||||||
|
|
||||||
@@ -127,10 +126,11 @@ function DeviceNodeComponent({ id, data, selected, width, height }: NodeProps) {
|
|||||||
className="max-w-[88%] cursor-default text-center font-medium leading-tight text-primary line-clamp-2"
|
className="max-w-[88%] cursor-default text-center font-medium leading-tight text-primary line-clamp-2"
|
||||||
onDoubleClick={e => {
|
onDoubleClick={e => {
|
||||||
e.stopPropagation()
|
e.stopPropagation()
|
||||||
|
setLabelValue(nodeData.label ?? '')
|
||||||
setEditing(true)
|
setEditing(true)
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
{labelValue}
|
{nodeData.label ?? ''}
|
||||||
</span>
|
</span>
|
||||||
)}
|
)}
|
||||||
<span
|
<span
|
||||||
|
|||||||
@@ -22,10 +22,9 @@ const GroupNodeComponent = ({ data, selected, id }: NodeProps) => {
|
|||||||
if (editing) inputRef.current?.focus()
|
if (editing) inputRef.current?.focus()
|
||||||
}, [editing])
|
}, [editing])
|
||||||
|
|
||||||
// Sync if external data.label changes
|
// While not editing, the displayed label is derived directly from
|
||||||
useEffect(() => {
|
// groupData.label — no effect-driven sync needed. labelValue holds the
|
||||||
if (!editing) setLabelValue(groupData.label ?? '')
|
// edit buffer only and is reset when an edit session starts.
|
||||||
}, [groupData.label, editing])
|
|
||||||
|
|
||||||
const handleLabelCommit = () => {
|
const handleLabelCommit = () => {
|
||||||
setEditing(false)
|
setEditing(false)
|
||||||
@@ -69,9 +68,12 @@ const GroupNodeComponent = ({ data, selected, id }: NodeProps) => {
|
|||||||
<span
|
<span
|
||||||
className="inline-block rounded-sm bg-card/90 px-1.5 py-0.5 text-[11px] font-semibold cursor-text select-none tracking-wide"
|
className="inline-block rounded-sm bg-card/90 px-1.5 py-0.5 text-[11px] font-semibold cursor-text select-none tracking-wide"
|
||||||
style={{ color }}
|
style={{ color }}
|
||||||
onDoubleClick={() => setEditing(true)}
|
onDoubleClick={() => {
|
||||||
|
setLabelValue(groupData.label ?? '')
|
||||||
|
setEditing(true)
|
||||||
|
}}
|
||||||
>
|
>
|
||||||
{labelValue || groupData.groupType}
|
{(groupData.label ?? '') || groupData.groupType}
|
||||||
</span>
|
</span>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -1,27 +1,28 @@
|
|||||||
import { useEffect, useState } from 'react'
|
import { useSyncExternalStore } from 'react'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* SSR-safe CSS media-query hook. Returns the current match boolean and
|
* SSR-safe CSS media-query hook. Returns the current match boolean and
|
||||||
* re-renders on viewport changes. Used by /pilot to swap the task lane
|
* re-renders on viewport changes. Used by /pilot to swap the task lane
|
||||||
* between side panel (≥1200px) and bottom drawer (<1200px) per Phase 7.
|
* between side panel (≥1200px) and bottom drawer (<1200px) per Phase 7.
|
||||||
|
*
|
||||||
|
* Implemented with useSyncExternalStore to subscribe to the MediaQueryList
|
||||||
|
* without an effect — this is the React-idiomatic shape for external-state
|
||||||
|
* subscriptions and avoids the setState-in-effect cascade lint rule.
|
||||||
*/
|
*/
|
||||||
export function useMediaQuery(query: string): boolean {
|
export function useMediaQuery(query: string): boolean {
|
||||||
const [matches, setMatches] = useState<boolean>(() => {
|
return useSyncExternalStore(
|
||||||
if (typeof window === 'undefined') return false
|
(onChange) => {
|
||||||
return window.matchMedia(query).matches
|
if (typeof window === 'undefined') return () => {}
|
||||||
})
|
const mql = window.matchMedia(query)
|
||||||
|
mql.addEventListener('change', onChange)
|
||||||
useEffect(() => {
|
return () => mql.removeEventListener('change', onChange)
|
||||||
if (typeof window === 'undefined') return
|
},
|
||||||
const mql = window.matchMedia(query)
|
() => {
|
||||||
const handler = (e: MediaQueryListEvent) => setMatches(e.matches)
|
if (typeof window === 'undefined') return false
|
||||||
// Sync once on mount in case state drifted between render and effect.
|
return window.matchMedia(query).matches
|
||||||
setMatches(mql.matches)
|
},
|
||||||
mql.addEventListener('change', handler)
|
() => false, // server snapshot — match initial false
|
||||||
return () => mql.removeEventListener('change', handler)
|
)
|
||||||
}, [query])
|
|
||||||
|
|
||||||
return matches
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export default useMediaQuery
|
export default useMediaQuery
|
||||||
|
|||||||
Reference in New Issue
Block a user