#139

bug: useCallback missing deps in App.tsx cause stale closure and ESLint failure

Openkuishou68opened 6mo ago

Bug

In flowchart/src/App.tsx, the handleNext, handlePrev, and handleReset callbacks each omit getNodes and getEdgeVisibility from their useCallback dependency arrays. Because eslint-plugin-react-hooks is installed (v7) and the lint script runs ESLint, this causes npm run lint to fail with react-hooks/exhaustive-deps errors.

Affected code

// handleNext — getNodes and getEdgeVisibility missing from deps
const handleNext = useCallback(() => {
  ...
  setNodes(getNodes(newCount));                          // getNodes not in deps
  setEdges(edgeConnections.map((conn) =>
    createEdge(conn, getEdgeVisibility(conn, newCount))  // getEdgeVisibility not in deps
  ));
}, [visibleCount, setNodes, setEdges]);  // ← incomplete

// handlePrev — same issue
const handlePrev = useCallback(() => {
  ...
  setNodes(getNodes(newCount));                          // getNodes not in deps
  setEdges(edgeConnections.map((conn) =>
    createEdge(conn, getEdgeVisibility(conn, newCount))  // getEdgeVisibility not in deps
  ));
}, [visibleCount, setNodes, setEdges]);  // ← incomplete

// handleReset — getNodes missing from deps
const handleReset = useCallback(() => {
  ...
  setNodes(getNodes(1));  // getNodes not in deps
  ...
}, [setNodes, setEdges]); // ← incomplete

Root cause

getNodes and getEdgeVisibility are plain functions defined inside the App component. They are recreated on every render. Omitting them from useCallback deps means the callbacks capture stale function references, violating the React hooks exhaustive-deps rule.

Impact

  • npm run lint (inside flowchart/) fails with react-hooks/exhaustive-deps errors
  • Future renders could theoretically use a stale getNodes/getEdgeVisibility if the functions ever close over component state

Fix

Wrap getNodes and getEdgeVisibility in useCallback (or move them outside the component / convert to stable utilities), then add them to the dependent callback arrays.

const getNodes = useCallback((count: number) => {
  const stepNodes = allSteps.map((step, index) =>
    createNode(step, index < count, nodePositions.current[step.id])
  );
  const noteNodes = notes.map(note => {
    const noteVisible = count >= note.appearsWithStep;
    return createNoteNode(note, noteVisible, nodePositions.current[note.id]);
  });
  return [...stepNodes, ...noteNodes];
}, []);

const getEdgeVisibility = useCallback(
  (conn: typeof edgeConnections[0], visibleStepCount: number) => {
    const sourceIndex = allSteps.findIndex(s => s.id === conn.source);
    const targetIndex = allSteps.findIndex(s => s.id === conn.target);
    return sourceIndex < visibleStepCount && targetIndex < visibleStepCount;
  },
  []
);

Then update the dependent callbacks to include them:

const handleNext = useCallback(() => { ... }, [visibleCount, setNodes, setEdges, getNodes, getEdgeVisibility]);
const handlePrev = useCallback(() => { ... }, [visibleCount, setNodes, setEdges, getNodes, getEdgeVisibility]);
const handleReset = useCallback(() => { ... }, [setNodes, setEdges, getNodes]);

Activity

3
kuishou686mo ago

I've opened PR #140 to fix this issue.

kuishou686mo ago

I've opened PR #140 to fix this issue.

lib4u6mo ago

https://github.com/lib4u/rufler I just made a ruflo implementation that's similar to Docker. You can create agents via a YAML file. This might be interesting.