mirror of
https://github.com/reconurge/flowsint.git
synced 2026-11-08 14:35:41 -06:00
fix(frontend): resolve react-hooks/refs
10 findings, three shapes: Six were the same pattern: `const xRef = useRef(x); xRef.current = x` written directly in the render body, to keep a ref in sync with the latest value for callbacks/subscriptions that need to read it without re-subscribing (graph-main.tsx's ForceGraph2D click handler, use-link-creation.ts, template-editor.tsx, use-confirm-dialog.tsx, use-paste-listener.ts). Mutating a ref during render is unsafe — if render runs without committing (Strict Mode, concurrent features), you still perform the mutation. Moved each into useLayoutEffect (not useEffect — this needs to happen before paint, so any interaction right after commit sees the latest ref). use-confirm-dialog.tsx's ref held a freshly-allocated closure every render too; wrapped that in useCallback([]) first since it only closes over stable setState setters, so the effect syncing it to the ref only needs to run once. graph/index.tsx was the same shape but inside a useMemo, using the ref mutation as a side-channel version counter for cache invalidation (useMemo isn't supposed to have side effects, and the existing eslint-disable-next-line on it suggests this was already known to be irregular). Replaced with what it was actually trying to be: real state, bumped in a useLayoutEffect keyed on the same deps — same "one new value per deps change" behavior, done through React's actual update path instead of a ref side-channel. image-view-block.tsx read getComputedStyle(containerRef.current) directly in the render body — null on first render (before mount), and reading DOM layout during render generally isn't safe. Moved to useState + useLayoutEffect, measuring after commit instead. details-panel.tsx: kept as a documented eslint-disable, not fixed. handlePropertyBlur reads a ref but only ever runs from a real onCheckedChange handler, never during render — the lint rule's static analysis treats any function that touches a ref as unsafe wherever it's referenced, not just where it executes, and can't prove otherwise from this callback shape. Restructuring around a rule limitation isn't worth it; disabled with the reasoning inline. Verified: eslint clean, tsc 88 (unchanged), vitest 15/15, yarn build clean.
This commit is contained in:
+10
-3
@@ -68,9 +68,16 @@ export const ImageViewBlock: React.FC<NodeViewProps> = ({
|
||||
|
||||
const aspectRatio = imageState.naturalSize.width / imageState.naturalSize.height
|
||||
const maxWidth = MAX_HEIGHT * aspectRatio
|
||||
const containerMaxWidth = containerRef.current
|
||||
? parseFloat(getComputedStyle(containerRef.current).getPropertyValue('--editor-width'))
|
||||
: Infinity
|
||||
// Reading the DOM (getComputedStyle) during render isn't safe — measure
|
||||
// after mount instead. containerRef.current is null on the first render,
|
||||
// so Infinity is the correct starting value regardless.
|
||||
const [containerMaxWidth, setContainerMaxWidth] = React.useState(Infinity)
|
||||
React.useLayoutEffect(() => {
|
||||
if (!containerRef.current) return
|
||||
setContainerMaxWidth(
|
||||
parseFloat(getComputedStyle(containerRef.current).getPropertyValue('--editor-width'))
|
||||
)
|
||||
})
|
||||
|
||||
const { isLink, onView, onDownload, onCopy, onCopyLink, onRemoveImg } = useImageActions({
|
||||
editor,
|
||||
|
||||
@@ -561,6 +561,10 @@ const DetailsPanel = memo(() => {
|
||||
canEdit ? (
|
||||
<Switch
|
||||
checked={value}
|
||||
// handlePropertyBlur reads formDataRef.current, but only ever runs
|
||||
// from this onCheckedChange callback (a real event handler) — never
|
||||
// during render. The lint rule can't see that from here.
|
||||
// eslint-disable-next-line react-hooks/refs
|
||||
onCheckedChange={(checked) => handlePropertyBlur(key, checked)}
|
||||
className="scale-75"
|
||||
/>
|
||||
|
||||
@@ -1,5 +1,12 @@
|
||||
import { useGraphStore } from '@/stores/graph-store'
|
||||
import React, { useRef, useCallback, useMemo, SetStateAction, Dispatch } from 'react'
|
||||
import React, {
|
||||
useRef,
|
||||
useCallback,
|
||||
useMemo,
|
||||
useLayoutEffect,
|
||||
SetStateAction,
|
||||
Dispatch
|
||||
} from 'react'
|
||||
import GraphViewer from '../index'
|
||||
import NodeContextMenu from '../context-menu/node-context-menu'
|
||||
import BackgroundContextMenu from '../context-menu/background-context-menu'
|
||||
@@ -128,7 +135,9 @@ const GraphMain = () => {
|
||||
|
||||
// Stable ref so ForceGraph2D always calls the latest version
|
||||
const bgClickRef = useRef(handleBackgroundClick)
|
||||
bgClickRef.current = handleBackgroundClick
|
||||
useLayoutEffect(() => {
|
||||
bgClickRef.current = handleBackgroundClick
|
||||
})
|
||||
const stableBackgroundClick = useCallback((event?: MouseEvent) => {
|
||||
bgClickRef.current(event)
|
||||
}, [])
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useRef, useState } from 'react'
|
||||
import { useCallback, useLayoutEffect, useRef, useState } from 'react'
|
||||
import { useGraphStore } from '@/stores/graph-store'
|
||||
import { sketchService } from '@/api/sketch-service'
|
||||
import { toast } from 'sonner'
|
||||
@@ -20,7 +20,9 @@ const INITIAL_STATE: LinkCreationState = {
|
||||
export const useLinkCreation = (sketchId?: string) => {
|
||||
const [state, setState] = useState<LinkCreationState>(INITIAL_STATE)
|
||||
const stateRef = useRef(state)
|
||||
stateRef.current = state
|
||||
useLayoutEffect(() => {
|
||||
stateRef.current = state
|
||||
})
|
||||
|
||||
const [shiftHeld, setShiftHeld] = useState(false)
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import React, { useCallback, useMemo, useEffect, useState, useRef } from 'react'
|
||||
import React, { useCallback, useMemo, useEffect, useLayoutEffect, useState, useRef } from 'react'
|
||||
import { useShallow } from 'zustand/react/shallow'
|
||||
import ForceGraph2D from 'react-force-graph-2d'
|
||||
import { useGraphControls } from '@/stores/graph-controls-store'
|
||||
@@ -383,16 +383,19 @@ const GraphViewer: React.FC<GraphViewerProps> = ({
|
||||
|
||||
// Render context: created once per frame, shared across all node/link render calls.
|
||||
// Invalidated when dependencies change or when the canvas transform changes between frames.
|
||||
const rcRef = useRef<{ rc: RenderContext | null; depsVersion: number; frameKey: string }>({
|
||||
const rcRef = useRef<{ rc: RenderContext | null; frameKey: string }>({
|
||||
rc: null,
|
||||
depsVersion: 0,
|
||||
frameKey: ''
|
||||
})
|
||||
|
||||
// Increment version whenever render context deps change to force recreation
|
||||
const rcDepsVersion = useMemo(() => {
|
||||
rcRef.current.depsVersion++
|
||||
return rcRef.current.depsVersion
|
||||
// Bumped whenever render context deps change, to force cache invalidation in
|
||||
// getOrCreateRC below even when globalScale alone doesn't. This used to be a
|
||||
// ref mutated straight in a useMemo body — invalid (refs can't be written
|
||||
// during render) and only worked by accident. Real state + a layout effect
|
||||
// gets the same "one value per deps change" behavior the correct way.
|
||||
const [rcDepsVersion, setRcDepsVersion] = useState(0)
|
||||
useLayoutEffect(() => {
|
||||
setRcDepsVersion((v) => v + 1)
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [highlightNodes, highlightLinks, selectedEdges, theme])
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useState, useCallback, useMemo, useEffect, useRef } from 'react'
|
||||
import { useState, useCallback, useMemo, useEffect, useLayoutEffect, useRef } from 'react'
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query'
|
||||
import { useNavigate } from '@tanstack/react-router'
|
||||
import { parse as parseYaml, stringify as stringifyYaml } from 'yaml'
|
||||
@@ -117,7 +117,9 @@ export function TemplateEditor({ templateId, initialContent, importedYaml }: Tem
|
||||
const paramKeys = Object.keys(templateParams)
|
||||
|
||||
const stateRef = useRef({ hasErrors, hasChanges, data: validationResult.data, content })
|
||||
stateRef.current = { hasErrors, hasChanges, data: validationResult.data, content }
|
||||
useLayoutEffect(() => {
|
||||
stateRef.current = { hasErrors, hasChanges, data: validationResult.data, content }
|
||||
})
|
||||
|
||||
const createMutation = useMutation({
|
||||
mutationFn: (data: TemplateData) =>
|
||||
|
||||
@@ -14,6 +14,7 @@ import React, {
|
||||
type ReactNode,
|
||||
useCallback,
|
||||
useContext,
|
||||
useLayoutEffect,
|
||||
useRef,
|
||||
useState
|
||||
} from 'react'
|
||||
@@ -62,12 +63,18 @@ function ConfirmDialogWithContext() {
|
||||
|
||||
const { confirmRef } = useContext(ConfirmContext)
|
||||
|
||||
confirmRef.current = (dialogProps: ConfirmProps) =>
|
||||
new Promise<boolean>((resolve) => {
|
||||
setProps(dialogProps)
|
||||
setOpen(true)
|
||||
resolveRef.current = resolve
|
||||
})
|
||||
const confirm = useCallback(
|
||||
(dialogProps: ConfirmProps) =>
|
||||
new Promise<boolean>((resolve) => {
|
||||
setProps(dialogProps)
|
||||
setOpen(true)
|
||||
resolveRef.current = resolve
|
||||
}),
|
||||
[]
|
||||
)
|
||||
useLayoutEffect(() => {
|
||||
confirmRef.current = confirm
|
||||
}, [confirmRef, confirm])
|
||||
|
||||
const onConfirm = () => {
|
||||
resolveRef.current(true)
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useEffect, useRef } from 'react'
|
||||
import { useCallback, useEffect, useLayoutEffect, useRef } from 'react'
|
||||
|
||||
export type ClipboardPayload =
|
||||
| { type: 'text'; text: string }
|
||||
@@ -49,7 +49,9 @@ export function usePasteListener(handlers: PasteHandlers, options: UsePasteListe
|
||||
const { global = false, preventDefault = false } = options
|
||||
|
||||
const handlersRef = useRef(handlers)
|
||||
handlersRef.current = handlers
|
||||
useLayoutEffect(() => {
|
||||
handlersRef.current = handlers
|
||||
})
|
||||
|
||||
const listener = useCallback(
|
||||
(event: ClipboardEvent | React.ClipboardEvent) => {
|
||||
|
||||
Reference in New Issue
Block a user