From 2ea9db54d38df92cdd21116817a5dae274db4c2b Mon Sep 17 00:00:00 2001 From: dextmorgn <64375473+dextmorgn@users.noreply.github.com> Date: Mon, 24 Aug 2026 11:17:12 +0200 Subject: [PATCH] fix(frontend): resolve react-hooks/refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../image/components/image-view-block.tsx | 13 ++++++++++--- .../sketches/details-panel/details-panel.tsx | 4 ++++ .../sketches/graph/components/graph-main.tsx | 13 +++++++++++-- .../sketches/graph/hooks/use-link-creation.ts | 6 ++++-- .../src/components/sketches/graph/index.tsx | 17 ++++++++++------- .../components/templates/template-editor.tsx | 6 ++++-- .../src/components/use-confirm-dialog.tsx | 19 +++++++++++++------ flowsint-app/src/hooks/use-paste-listener.ts | 6 ++++-- 8 files changed, 60 insertions(+), 24 deletions(-) diff --git a/flowsint-app/src/components/analyses/editor/extensions/image/components/image-view-block.tsx b/flowsint-app/src/components/analyses/editor/extensions/image/components/image-view-block.tsx index cad68066..ed195283 100644 --- a/flowsint-app/src/components/analyses/editor/extensions/image/components/image-view-block.tsx +++ b/flowsint-app/src/components/analyses/editor/extensions/image/components/image-view-block.tsx @@ -68,9 +68,16 @@ export const ImageViewBlock: React.FC = ({ 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, diff --git a/flowsint-app/src/components/sketches/details-panel/details-panel.tsx b/flowsint-app/src/components/sketches/details-panel/details-panel.tsx index 5bd9567e..f8486c40 100644 --- a/flowsint-app/src/components/sketches/details-panel/details-panel.tsx +++ b/flowsint-app/src/components/sketches/details-panel/details-panel.tsx @@ -561,6 +561,10 @@ const DetailsPanel = memo(() => { canEdit ? ( handlePropertyBlur(key, checked)} className="scale-75" /> diff --git a/flowsint-app/src/components/sketches/graph/components/graph-main.tsx b/flowsint-app/src/components/sketches/graph/components/graph-main.tsx index 8ccd944f..b805b276 100644 --- a/flowsint-app/src/components/sketches/graph/components/graph-main.tsx +++ b/flowsint-app/src/components/sketches/graph/components/graph-main.tsx @@ -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) }, []) diff --git a/flowsint-app/src/components/sketches/graph/hooks/use-link-creation.ts b/flowsint-app/src/components/sketches/graph/hooks/use-link-creation.ts index a1d90390..865cbc06 100644 --- a/flowsint-app/src/components/sketches/graph/hooks/use-link-creation.ts +++ b/flowsint-app/src/components/sketches/graph/hooks/use-link-creation.ts @@ -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(INITIAL_STATE) const stateRef = useRef(state) - stateRef.current = state + useLayoutEffect(() => { + stateRef.current = state + }) const [shiftHeld, setShiftHeld] = useState(false) diff --git a/flowsint-app/src/components/sketches/graph/index.tsx b/flowsint-app/src/components/sketches/graph/index.tsx index 77bfb4c5..d71831a6 100644 --- a/flowsint-app/src/components/sketches/graph/index.tsx +++ b/flowsint-app/src/components/sketches/graph/index.tsx @@ -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 = ({ // 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]) diff --git a/flowsint-app/src/components/templates/template-editor.tsx b/flowsint-app/src/components/templates/template-editor.tsx index 4a4175c4..cf020f5b 100644 --- a/flowsint-app/src/components/templates/template-editor.tsx +++ b/flowsint-app/src/components/templates/template-editor.tsx @@ -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) => diff --git a/flowsint-app/src/components/use-confirm-dialog.tsx b/flowsint-app/src/components/use-confirm-dialog.tsx index 998f10f9..f257f2d1 100644 --- a/flowsint-app/src/components/use-confirm-dialog.tsx +++ b/flowsint-app/src/components/use-confirm-dialog.tsx @@ -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((resolve) => { - setProps(dialogProps) - setOpen(true) - resolveRef.current = resolve - }) + const confirm = useCallback( + (dialogProps: ConfirmProps) => + new Promise((resolve) => { + setProps(dialogProps) + setOpen(true) + resolveRef.current = resolve + }), + [] + ) + useLayoutEffect(() => { + confirmRef.current = confirm + }, [confirmRef, confirm]) const onConfirm = () => { resolveRef.current(true) diff --git a/flowsint-app/src/hooks/use-paste-listener.ts b/flowsint-app/src/hooks/use-paste-listener.ts index 7b580844..f7df80b8 100644 --- a/flowsint-app/src/hooks/use-paste-listener.ts +++ b/flowsint-app/src/hooks/use-paste-listener.ts @@ -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) => {