From 5f5099d1fc67c511a1fbcb6583c2f2f419473b7a Mon Sep 17 00:00:00 2001 From: Hayleigh Thompson Date: Fri, 19 Jan 2024 09:38:32 +0000 Subject: [PATCH 1/4] :construction: Janky explorations. --- packages/react/src/hooks/useReactFlow.ts | 84 +++++++++++++++++------- 1 file changed, 62 insertions(+), 22 deletions(-) diff --git a/packages/react/src/hooks/useReactFlow.ts b/packages/react/src/hooks/useReactFlow.ts index bdd05aa0..097c78b6 100644 --- a/packages/react/src/hooks/useReactFlow.ts +++ b/packages/react/src/hooks/useReactFlow.ts @@ -1,4 +1,4 @@ -import { useCallback, useMemo, useRef } from 'react'; +import { useCallback, useLayoutEffect, useMemo, useRef, useState } from 'react'; import { getElementsToRemove, getOverlappingArea, isRectObject, nodeToRect, type Rect } from '@xyflow/system'; import useViewportHelper from './useViewportHelper'; @@ -46,33 +46,73 @@ export function useReactFlow e.id === id) as EdgeType; }, []); - // this is used to handle multiple syncronous setNodes calls - const setNodesData = useRef(); - const setNodesTimeout = useRef>(); + const setNodesQueue = useRef<(NodeType[] | ((nodes: NodeType[]) => NodeType[]))[]>([]); + const setNodesHasStateReplacement = useRef(false); + const [setNodesShouldFlushNodesQueue, setSetNodesShouldFlushNodesQueue] = useState(false); const setNodes = useCallback>((payload) => { - const { nodes = [], setNodes, hasDefaultNodes, onNodesChange, nodeLookup } = store.getState(); - const nextNodes = typeof payload === 'function' ? payload((setNodesData.current as NodeType[]) || nodes) : payload; + if (typeof payload === 'function' && !setNodesHasStateReplacement.current) { + setNodesQueue.current.push(payload); + setSetNodesShouldFlushNodesQueue(true); + } else { + setNodesQueue.current = [payload]; + setNodesHasStateReplacement.current = true; + setSetNodesShouldFlushNodesQueue(true); + } + }, []); - setNodesData.current = nextNodes; + useLayoutEffect(() => { + if (!setNodesShouldFlushNodesQueue) { + setNodesQueue.current = []; + setNodesHasStateReplacement.current = false; - if (setNodesTimeout.current) { - clearTimeout(setNodesTimeout.current); + return; } - // if there are multiple synchronous setNodes calls, we only want to call onNodesChange once - // for this, we use a timeout to wait for the last call and store updated nodes in setNodesData - // this is not perfect, but should work in most cases - setNodesTimeout.current = setTimeout(() => { - if (hasDefaultNodes) { - setNodes(nextNodes); - } else if (onNodesChange) { - const changes: NodeChange[] = getElementsDiffChanges({ items: setNodesData.current, lookup: nodeLookup }); - onNodesChange(changes); - } + const { nodes = [], setNodes, hasDefaultNodes, onNodesChange, nodeLookup } = store.getState(); + const nextNodes = setNodesQueue.current.reduce( + (prev: NodeType[], payload) => (typeof payload === 'function' ? payload(prev) : payload), + nodes as NodeType[] + ); - setNodesData.current = undefined; - }, 0); - }, []); + if (hasDefaultNodes) { + setNodes(nextNodes); + } else if (onNodesChange) { + const changes: NodeChange[] = getElementsDiffChanges({ items: nextNodes, lookup: nodeLookup }); + onNodesChange(changes); + } + + setNodesQueue.current = []; + setNodesHasStateReplacement.current = false; + setSetNodesShouldFlushNodesQueue(false); + }, [setNodesShouldFlushNodesQueue]); + + // // this is used to handle multiple syncronous setNodes calls + // const setNodesData = useRef(); + // const setNodesTimeout = useRef>(); + // const setNodes = useCallback>((payload) => { + // const { nodes = [], setNodes, hasDefaultNodes, onNodesChange, nodeLookup } = store.getState(); + // const nextNodes = typeof payload === 'function' ? payload((setNodesData.current as NodeType[]) || nodes) : payload; + + // setNodesData.current = nextNodes; + + // if (setNodesTimeout.current) { + // clearTimeout(setNodesTimeout.current); + // } + + // // if there are multiple synchronous setNodes calls, we only want to call onNodesChange once + // // for this, we use a timeout to wait for the last call and store updated nodes in setNodesData + // // this is not perfect, but should work in most cases + // setNodesTimeout.current = setTimeout(() => { + // if (hasDefaultNodes) { + // setNodes(nextNodes); + // } else if (onNodesChange) { + // const changes: NodeChange[] = getElementsDiffChanges({ items: setNodesData.current, lookup: nodeLookup }); + // onNodesChange(changes); + // } + + // setNodesData.current = undefined; + // }, 0); + // }, []); // this is used to handle multiple syncronous setEdges calls const setEdgesData = useRef(); From a25854646a1ebd8ea978bd4372f276c341398a29 Mon Sep 17 00:00:00 2001 From: Hayleigh Thompson Date: Fri, 19 Jan 2024 16:09:51 +0000 Subject: [PATCH 2/4] :sparkles: Batch calls to setNodes and addNodes for better perf. --- packages/react/src/hooks/useReactFlow.ts | 187 +++++++++++------------ 1 file changed, 87 insertions(+), 100 deletions(-) diff --git a/packages/react/src/hooks/useReactFlow.ts b/packages/react/src/hooks/useReactFlow.ts index 097c78b6..18de6fc6 100644 --- a/packages/react/src/hooks/useReactFlow.ts +++ b/packages/react/src/hooks/useReactFlow.ts @@ -46,122 +46,109 @@ export function useReactFlow e.id === id) as EdgeType; }, []); - const setNodesQueue = useRef<(NodeType[] | ((nodes: NodeType[]) => NodeType[]))[]>([]); - const setNodesHasStateReplacement = useRef(false); - const [setNodesShouldFlushNodesQueue, setSetNodesShouldFlushNodesQueue] = useState(false); - const setNodes = useCallback>((payload) => { - if (typeof payload === 'function' && !setNodesHasStateReplacement.current) { - setNodesQueue.current.push(payload); - setSetNodesShouldFlushNodesQueue(true); - } else { - setNodesQueue.current = [payload]; - setNodesHasStateReplacement.current = true; - setSetNodesShouldFlushNodesQueue(true); - } - }, []); + type SetElementsQueue = { + nodes: (NodeType[] | ((nodes: NodeType[]) => NodeType[]))[]; + edges: (EdgeType[] | ((edges: EdgeType[]) => EdgeType[]))[]; + }; + // A reference of all the batched updates to process before the next render. We + // want a mutable reference here so multiple synchronous calls to `setNodes` etc + // can be batched together. + const setElementsQueue = useRef({ nodes: [], edges: [] }); + // Because we're using a ref above, we need some way to let React know when to + // actually process the queue. We flip this bit of state to `true` any time we + // mutate the queue and then flip it back to `false` after flushing the queue. + const [shouldFlushQueue, setShouldFlushQueue] = useState(false); + + // Layout effects are guaranteed to run before the next render which means we + // shouldn't run into any issues with stale state or weird issues that come from + // rendering things one frame later than expected (we used to use `setTimeout`). useLayoutEffect(() => { - if (!setNodesShouldFlushNodesQueue) { - setNodesQueue.current = []; - setNodesHasStateReplacement.current = false; - + // Because we need to flip the state back to false after flushing, this should + // trigger the hook again (!). If the hook is being run again we know that any + // updates should have been processed by now and we can safely clear the queue + // and bail early. + if (!shouldFlushQueue) { + setElementsQueue.current = { nodes: [], edges: [] }; return; } - const { nodes = [], setNodes, hasDefaultNodes, onNodesChange, nodeLookup } = store.getState(); - const nextNodes = setNodesQueue.current.reduce( - (prev: NodeType[], payload) => (typeof payload === 'function' ? payload(prev) : payload), - nodes as NodeType[] - ); + if (setElementsQueue.current.nodes.length) { + const { nodes = [], setNodes, hasDefaultNodes, onNodesChange, nodeLookup } = store.getState(); - if (hasDefaultNodes) { - setNodes(nextNodes); - } else if (onNodesChange) { - const changes: NodeChange[] = getElementsDiffChanges({ items: nextNodes, lookup: nodeLookup }); - onNodesChange(changes); - } - - setNodesQueue.current = []; - setNodesHasStateReplacement.current = false; - setSetNodesShouldFlushNodesQueue(false); - }, [setNodesShouldFlushNodesQueue]); - - // // this is used to handle multiple syncronous setNodes calls - // const setNodesData = useRef(); - // const setNodesTimeout = useRef>(); - // const setNodes = useCallback>((payload) => { - // const { nodes = [], setNodes, hasDefaultNodes, onNodesChange, nodeLookup } = store.getState(); - // const nextNodes = typeof payload === 'function' ? payload((setNodesData.current as NodeType[]) || nodes) : payload; - - // setNodesData.current = nextNodes; - - // if (setNodesTimeout.current) { - // clearTimeout(setNodesTimeout.current); - // } - - // // if there are multiple synchronous setNodes calls, we only want to call onNodesChange once - // // for this, we use a timeout to wait for the last call and store updated nodes in setNodesData - // // this is not perfect, but should work in most cases - // setNodesTimeout.current = setTimeout(() => { - // if (hasDefaultNodes) { - // setNodes(nextNodes); - // } else if (onNodesChange) { - // const changes: NodeChange[] = getElementsDiffChanges({ items: setNodesData.current, lookup: nodeLookup }); - // onNodesChange(changes); - // } - - // setNodesData.current = undefined; - // }, 0); - // }, []); - - // this is used to handle multiple syncronous setEdges calls - const setEdgesData = useRef(); - const setEdgesTimeout = useRef>(); - const setEdges = useCallback>((payload) => { - const { edges = [], setEdges, hasDefaultEdges, onEdgesChange, edgeLookup } = store.getState(); - const nextEdges = typeof payload === 'function' ? payload((setEdgesData.current as EdgeType[]) || edges) : payload; - - setEdgesData.current = nextEdges; - - if (setEdgesTimeout.current) { - clearTimeout(setEdgesTimeout.current); - } - - setEdgesTimeout.current = setTimeout(() => { - if (hasDefaultEdges) { - setEdges(nextEdges); - } else if (onEdgesChange) { - const changes: EdgeChange[] = getElementsDiffChanges({ items: nextEdges, lookup: edgeLookup }); - onEdgesChange(changes); + // This is essentially an `Array.reduce` in imperative clothing. Processing + // this queue is a relatively hot path so we'd like to avoid the overhead of + // array methods where we can. + let next = nodes as NodeType[]; + for (const payload of setElementsQueue.current.nodes) { + next = typeof payload === 'function' ? payload(next) : payload; } - setEdgesData.current = undefined; - }, 0); + if (hasDefaultNodes) { + setNodes(next); + } else if (onNodesChange) { + onNodesChange( + getElementsDiffChanges({ + items: next, + lookup: nodeLookup, + }) + ); + } + + setElementsQueue.current.nodes = []; + } + + if (setElementsQueue.current.edges.length) { + const { edges = [], setEdges, hasDefaultEdges, onEdgesChange, edgeLookup } = store.getState(); + + let next = edges as EdgeType[]; + for (const payload of setElementsQueue.current.edges) { + next = typeof payload === 'function' ? payload(next) : payload; + } + + if (hasDefaultEdges) { + setEdges(next); + } else if (onEdgesChange) { + onEdgesChange( + getElementsDiffChanges({ + items: next, + lookup: edgeLookup, + }) + ); + } + + setElementsQueue.current.edges = []; + } + + // Beacuse we're using reactive state to trigger this effect, we need to flip + // it back to false. + setShouldFlushQueue(false); + }, [shouldFlushQueue]); + + const setNodes = useCallback>((payload) => { + setElementsQueue.current.nodes.push(payload); + setShouldFlushQueue(true); + }, []); + + const setEdges = useCallback>((payload) => { + setElementsQueue.current.edges.push(payload); + setShouldFlushQueue(true); }, []); const addNodes = useCallback>((payload) => { - const nodes = Array.isArray(payload) ? payload : [payload]; - const { nodes: currentNodes, hasDefaultNodes, onNodesChange, setNodes } = store.getState(); + const newNodes = Array.isArray(payload) ? payload : [payload]; - if (hasDefaultNodes) { - const nextNodes = [...currentNodes, ...nodes]; - setNodes(nextNodes); - } else if (onNodesChange) { - const changes = nodes.map((node) => ({ item: node, type: 'add' } as NodeAddChange)); - onNodesChange(changes); - } + // Queueing a functional update means that we won't worry about other calls + // to `setNodes` that might happen elsewhere. + setElementsQueue.current.nodes.push((nodes) => [...nodes, ...newNodes]); + setShouldFlushQueue(true); }, []); const addEdges = useCallback>((payload) => { - const nextEdges = Array.isArray(payload) ? payload : [payload]; - const { edges = [], setEdges, hasDefaultEdges, onEdgesChange } = store.getState(); + const newEdges = Array.isArray(payload) ? payload : [payload]; - if (hasDefaultEdges) { - setEdges([...edges, ...nextEdges]); - } else if (onEdgesChange) { - const changes = nextEdges.map((edge) => ({ item: edge, type: 'add' } as EdgeAddChange)); - onEdgesChange(changes); - } + setElementsQueue.current.edges.push((edges) => [...edges, ...newEdges]); + setShouldFlushQueue(true); }, []); const toObject = useCallback>(() => { From 81b5ea61412cfccecdabdad5abacc0998fc70840 Mon Sep 17 00:00:00 2001 From: Hayleigh Thompson Date: Fri, 19 Jan 2024 16:10:06 +0000 Subject: [PATCH 3/4] :sparkles: Add an example demonstrating how batched updates work. --- examples/react/src/App/routes.ts | 6 ++ .../src/examples/SetNodesBatching/index.tsx | 70 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 examples/react/src/examples/SetNodesBatching/index.tsx diff --git a/examples/react/src/App/routes.ts b/examples/react/src/App/routes.ts index 76ab08e4..444586ca 100644 --- a/examples/react/src/App/routes.ts +++ b/examples/react/src/App/routes.ts @@ -28,6 +28,7 @@ import NodeTypesObjectChange from '../examples/NodeTypesObjectChange'; import Overview from '../examples/Overview'; import Provider from '../examples/Provider'; import SaveRestore from '../examples/SaveRestore'; +import SetNodesBatching from '../examples/SetNodesBatching'; import Stress from '../examples/Stress'; import Subflow from '../examples/Subflow'; import SwitchFlow from '../examples/Switch'; @@ -226,6 +227,11 @@ const routes: IRoute[] = [ path: 'save-restore', component: SaveRestore, }, + { + name: 'SetNodes Batching', + path: 'setnodes-batching', + component: SetNodesBatching, + }, { name: 'Stress', path: 'stress', diff --git a/examples/react/src/examples/SetNodesBatching/index.tsx b/examples/react/src/examples/SetNodesBatching/index.tsx new file mode 100644 index 00000000..2dc602eb --- /dev/null +++ b/examples/react/src/examples/SetNodesBatching/index.tsx @@ -0,0 +1,70 @@ +import { useCallback } from 'react'; +import { + ReactFlow, + MiniMap, + Background, + BackgroundVariant, + Controls, + ReactFlowProvider, + Node, + Edge, + useReactFlow, + Panel, +} from '@xyflow/react'; + +const a = { id: 'a', data: { label: 'A' }, position: { x: 250, y: 5 } }; +const b = { id: 'b', data: { label: 'B' }, position: { x: 100, y: 100 } }; +const c = { id: 'c', data: { label: 'C' }, position: { x: 400, y: 100 } }; + +const SetNotesBatchingFlow = () => { + const { setNodes, updateNode } = useReactFlow(); + + const triggerMultipleSetNodes = useCallback(() => { + setNodes([a]); + setNodes((nodes) => [...nodes, b]); + setNodes((nodes) => [...nodes, c]); + setNodes((nodes) => + nodes.map((node) => + node.id === 'a' ? { ...node, position: { x: node.position.x + 20, y: node.position.y + 20 } } : node + ) + ); + }, []); + + const triggerMultipleUpdateNodes = useCallback(() => { + triggerMultipleSetNodes(); + updateNode('a', (a) => ({ position: { x: a.position.x + 20, y: a.position.y + 20 } })); + updateNode('b', (b) => ({ position: { x: b.position.x + 20, y: b.position.y + 20 } })); + updateNode('c', (c) => ({ position: { x: c.position.x + 20, y: c.position.y + 20 } })); + updateNode('a', (a) => ({ data: { ...a.data, label: `A ${Date.now()}` } })); + updateNode('b', (b) => ({ data: { ...b.data, label: `B ${Date.now()}` } })); + updateNode('c', (c) => ({ data: { ...c.data, label: `C ${Date.now()}` } })); + }, []); + + return ( + + + + + + + + + + + ); +}; + +export default function App() { + return ( + + + + ); +} From 2b0fa97afebf1b9c38169e723a41419a37c9fbeb Mon Sep 17 00:00:00 2001 From: moklick Date: Mon, 22 Jan 2024 11:42:20 +0100 Subject: [PATCH 4/4] chore(examples): second input for useNodesData --- .../react/src/examples/UseNodesData/TextNode.tsx | 12 ++++++++++-- packages/react/src/hooks/useReactFlow.ts | 11 +---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/examples/react/src/examples/UseNodesData/TextNode.tsx b/examples/react/src/examples/UseNodesData/TextNode.tsx index 113d41fb..dd9f25d1 100644 --- a/examples/react/src/examples/UseNodesData/TextNode.tsx +++ b/examples/react/src/examples/UseNodesData/TextNode.tsx @@ -14,8 +14,16 @@ function TextNode({ id, data }: NodeProps) { return (
node {id}
-
- updateText(evt.target.value)} value={text} /> +
+ + updateText(evt.target.value)} value={text} style={{ display: 'block' }} /> + + + updateNodeData(id, { text: evt.target.value })} + value={data.text} + style={{ display: 'block' }} + />
diff --git a/packages/react/src/hooks/useReactFlow.ts b/packages/react/src/hooks/useReactFlow.ts index 18de6fc6..02080731 100644 --- a/packages/react/src/hooks/useReactFlow.ts +++ b/packages/react/src/hooks/useReactFlow.ts @@ -3,16 +3,7 @@ import { getElementsToRemove, getOverlappingArea, isRectObject, nodeToRect, type import useViewportHelper from './useViewportHelper'; import { useStoreApi } from './useStore'; -import type { - ReactFlowInstance, - Instance, - NodeAddChange, - EdgeAddChange, - Node, - Edge, - NodeChange, - EdgeChange, -} from '../types'; +import type { ReactFlowInstance, Instance, Node, Edge } from '../types'; import { getElementsDiffChanges, isNode } from '../utils'; /**