From 2ba35de18622a11bccb63c1b0226dffeeedfe9c6 Mon Sep 17 00:00:00 2001 From: moklick Date: Thu, 11 Jan 2024 16:03:02 +0100 Subject: [PATCH] refactor(applyChanges): use specific replace events instead of a rough reset --- .../components/utils/apply-changes.cy.ts | 12 +-- .../react/src/examples/UseNodesData/index.tsx | 2 +- .../react/src/examples/UseReactFlow/index.tsx | 26 +++++- packages/react/src/hooks/useReactFlow.ts | 82 +++++++++---------- packages/react/src/types/changes.ts | 15 ++-- packages/react/src/utils/changes.ts | 69 ++++++++++++++-- 6 files changed, 145 insertions(+), 61 deletions(-) diff --git a/examples/react/cypress/components/utils/apply-changes.cy.ts b/examples/react/cypress/components/utils/apply-changes.cy.ts index 9bdf01af..1548b7e9 100644 --- a/examples/react/cypress/components/utils/apply-changes.cy.ts +++ b/examples/react/cypress/components/utils/apply-changes.cy.ts @@ -156,20 +156,22 @@ describe('applyChanges Testing', () => { expect(nextNodes[0].computed).to.be.deep.equal({ width: newWidth, height: newHeight }); }); - it('resets nodes/edges', () => { + it('replaces nodes/edges', () => { const nodesLength = nodes.length; - const nodeChanges: NodeChange[] = [{ type: 'reset', item: nodes[0] }]; + const nodeChanges: NodeChange[] = [{ type: 'replace', id: nodes[0].id, item: nodes[1] }]; const nextNodes = applyNodeChanges(nodeChanges, nodes); expect(nodes.length).length.to.be.equal(nodesLength); - expect(nextNodes.length).to.be.equal(nodeChanges.length); + expect(nextNodes.length).to.be.equal(nodesLength); + expect(nextNodes[0]).to.be.deep.equal(nodes[1]); const edgesLength = edges.length; - const edgeChange: EdgeChange[] = [{ type: 'reset', item: edges[0] }]; + const edgeChange: EdgeChange[] = [{ type: 'replace', id: edges[0].id, item: edges[1] }]; const nextEdges = applyEdgeChanges(edgeChange, edges); expect(edges.length).length.to.be.equal(edgesLength); - expect(nextEdges.length).to.be.equal(edgeChange.length); + expect(nextEdges.length).to.be.equal(edgesLength); + expect(nextEdges[0]).to.be.deep.equal(edges[1]); }); }); diff --git a/examples/react/src/examples/UseNodesData/index.tsx b/examples/react/src/examples/UseNodesData/index.tsx index 7bb1a94e..d6b697a9 100644 --- a/examples/react/src/examples/UseNodesData/index.tsx +++ b/examples/react/src/examples/UseNodesData/index.tsx @@ -78,7 +78,7 @@ const initEdges: Edge[] = [ ]; const CustomNodeFlow = () => { - const [nodes, setNodes, onNodesChange] = useNodesState(initNodes); + const [nodes, , onNodesChange] = useNodesState(initNodes); const [edges, setEdges, onEdgesChange] = useEdgesState(initEdges); const onConnect = useCallback((connection: Connection) => setEdges((eds) => addEdge(connection, eds)), [setEdges]); diff --git a/examples/react/src/examples/UseReactFlow/index.tsx b/examples/react/src/examples/UseReactFlow/index.tsx index 69f3c762..bed127fb 100644 --- a/examples/react/src/examples/UseReactFlow/index.tsx +++ b/examples/react/src/examples/UseReactFlow/index.tsx @@ -1,4 +1,4 @@ -import { useCallback, MouseEvent, useEffect } from 'react'; +import { useCallback, MouseEvent, useEffect, useRef } from 'react'; import { ReactFlow, Background, @@ -68,6 +68,7 @@ const UseZoomPanHelperFlow = () => { getNodes, getEdges, deleteElements, + updateNodeData, } = useReactFlow(); const onPaneClick = useCallback( @@ -125,12 +126,31 @@ const UseZoomPanHelperFlow = () => { deleteElements({ nodes: [{ id: '2' }], edges: [{ id: 'e1-3' }] }); }, []); + const edgeAdded = useRef(false); + useEffect(() => { - addEdges({ id: 'e3-4', source: '3', target: '4' }); + if (!edgeAdded.current) { + addEdges({ id: 'e3-4', source: '3', target: '4' }); + edgeAdded.current = true; + } }, [addEdges]); const onResetNodes = useCallback(() => setNodesHook(initialNodes), [setNodesHook]); + const onSetNodes = () => { + setNodes([ + { id: 'a', position: { x: 0, y: 0 }, data: { label: 'Node a' } }, + { id: 'b', position: { x: 0, y: 150 }, data: { label: 'Node b' } }, + ]); + + setEdges([{ id: 'a-b', source: 'a', target: 'b' }]); + }; + + const onUpdateNode = () => { + updateNodeData('1', { label: 'update' }); + updateNodeData('2', { label: 'update' }); + }; + return ( { + + diff --git a/packages/react/src/hooks/useReactFlow.ts b/packages/react/src/hooks/useReactFlow.ts index b8b01920..25a8310e 100644 --- a/packages/react/src/hooks/useReactFlow.ts +++ b/packages/react/src/hooks/useReactFlow.ts @@ -8,14 +8,12 @@ import type { Instance, NodeAddChange, EdgeAddChange, - NodeResetChange, - EdgeResetChange, - NodeRemoveChange, - EdgeRemoveChange, Node, Edge, + NodeChange, + EdgeChange, } from '../types'; -import { isNode } from '../utils'; +import { getElementsDiffChanges, isNode } from '../utils'; /** * Hook for accessing the ReactFlow instance. @@ -30,10 +28,6 @@ export function useReactFlow(); - const setNodesTimeout = useRef>(); - const getNodes = useCallback>(() => { return store.getState().nodes.map((n) => ({ ...n })) as NodeType[]; }, []); @@ -52,48 +46,54 @@ export function useReactFlow e.id === id) as EdgeType; }, []); + // this is used to handle multiple syncronous setNodes calls + const setNodesData = useRef(); + const setNodesTimeout = useRef>(); const setNodes = useCallback>((payload) => { - const { nodes, setNodes, hasDefaultNodes, onNodesChange } = store.getState(); - setNodesData.current = setNodesData.current || nodes; - const nextNodes = typeof payload === 'function' ? payload(setNodesData.current as NodeType[]) : 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 (hasDefaultNodes) { - setNodes(nextNodes); - } else if (onNodesChange) { - 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(() => { - const changes = - nextNodes.length === 0 - ? nodes.map((node) => ({ type: 'remove', id: node.id } as NodeRemoveChange)) - : nextNodes.map((node) => ({ item: node, type: 'reset' } as NodeResetChange)); - onNodesChange(changes); - - setNodesData.current = undefined; - }, 0); + 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 setNodes calls + const setEdgesData = useRef(); + const setEdgesTimeout = useRef>(); const setEdges = useCallback>((payload) => { - const { edges = [], setEdges, hasDefaultEdges, onEdgesChange } = store.getState(); - const nextEdges = typeof payload === 'function' ? payload(edges as EdgeType[]) : payload; + const { edges = [], setEdges, hasDefaultEdges, onEdgesChange, edgeLookup } = store.getState(); + const nextEdges = typeof payload === 'function' ? payload((setEdgesData.current as EdgeType[]) || edges) : payload; - if (hasDefaultEdges) { - setEdges(nextEdges); - } else if (onEdgesChange) { - const changes = - nextEdges.length === 0 - ? edges.map((edge) => ({ type: 'remove', id: edge.id } as EdgeRemoveChange)) - : nextEdges.map((edge) => ({ item: edge, type: 'reset' } as EdgeResetChange)); - onEdgesChange(changes); + 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); + } + }, 0); }, []); const addNodes = useCallback>((payload) => { diff --git a/packages/react/src/types/changes.ts b/packages/react/src/types/changes.ts index 9a60e089..3dfdffa2 100644 --- a/packages/react/src/types/changes.ts +++ b/packages/react/src/types/changes.ts @@ -34,9 +34,10 @@ export type NodeAddChange = { type: 'add'; }; -export type NodeResetChange = { +export type NodeReplaceChange = { + id: string; item: NodeType; - type: 'reset'; + type: 'replace'; }; /** @@ -49,7 +50,7 @@ export type NodeChange = | NodeSelectionChange | NodeRemoveChange | NodeAddChange - | NodeResetChange; + | NodeReplaceChange; export type EdgeSelectionChange = NodeSelectionChange; export type EdgeRemoveChange = NodeRemoveChange; @@ -57,8 +58,10 @@ export type EdgeAddChange = { item: EdgeType; type: 'add'; }; -export type EdgeResetChange = { + +export type EdgeReplaceChange = { + id: string; item: EdgeType; - type: 'reset'; + type: 'replace'; }; -export type EdgeChange = EdgeSelectionChange | EdgeRemoveChange | EdgeAddChange | EdgeResetChange; +export type EdgeChange = EdgeSelectionChange | EdgeRemoveChange | EdgeAddChange | EdgeReplaceChange; diff --git a/packages/react/src/utils/changes.ts b/packages/react/src/utils/changes.ts index 87b87c3a..cd148a6e 100644 --- a/packages/react/src/utils/changes.ts +++ b/packages/react/src/utils/changes.ts @@ -1,4 +1,5 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ +import { EdgeLookup, NodeLookup } from '@xyflow/system'; import type { Node, Edge, EdgeChange, NodeChange, NodeSelectionChange, EdgeSelectionChange } from '../types'; export function handleParentExpand(updatedElements: any[], updateItem: any) { @@ -53,11 +54,6 @@ export function handleParentExpand(updatedElements: any[], updateItem: any) { // When you drag a node for example, React Flow will send a position change update. // This function then applies the changes and returns the updated elements. function applyChanges(changes: any[], elements: any[]): any[] { - // we need this hack to handle the setNodes and setEdges function of the useReactFlow hook for controlled flows - if (changes.some((c) => c.type === 'reset')) { - return changes.filter((c) => c.type === 'reset').map((c) => c.item); - } - const updatedElements: any[] = []; // By storing a map of changes for each element, we can a quick lookup as we // iterate over the elements array! @@ -67,7 +63,7 @@ function applyChanges(changes: any[], elements: any[]): any[] { if (change.type === 'add') { updatedElements.push(change.item); continue; - } else if (change.type === 'remove') { + } else if (change.type === 'remove' || change.type === 'replace') { // For a 'remove' change we can safely ignore any other changes queued for // the same element, it's going to be removed anyway! changesMap.set(change.id, [change]); @@ -99,6 +95,11 @@ function applyChanges(changes: any[], elements: any[]): any[] { continue; } + if (changes[0].type === 'replace') { + updatedElements.push({ ...changes[0].item }); + continue; + } + // For other types of changes, we want to start with a shallow copy of the // object so React knows this element has changed. Sequential changes will /// each _mutate_ this object, so there's only ever one copy. @@ -245,3 +246,59 @@ export function getSelectionChanges( return changes; } + +/** + * This function is used to find the changes between two sets of elements. + * It is used to determine which nodes or edges have been added, removed or replaced. + * + * @internal + * @param params.items = the next set of elements (nodes or edges) + * @param params.lookup = a lookup map of the current store elements + * @returns an array of changes + */ +export function getElementsDiffChanges({ + items, + lookup, +}: { + items: Node[] | undefined; + lookup: NodeLookup; +}): NodeChange[]; +export function getElementsDiffChanges({ + items, + lookup, +}: { + items: Edge[] | undefined; + lookup: EdgeLookup; +}): EdgeChange[]; +export function getElementsDiffChanges({ + items = [], + lookup, +}: { + items: any[] | undefined; + lookup: Map; +}): any[] { + const changes: any[] = []; + const itemsLookup = new Map(items.map((item) => [item.id, item])); + + for (const item of items) { + const storeItem = lookup.get(item.id); + + if (storeItem !== undefined && storeItem !== item) { + changes.push({ id: item.id, item: item, type: 'replace' }); + } + + if (storeItem === undefined) { + changes.push({ item: item, type: 'add' }); + } + } + + for (const [id] of lookup) { + const nextNode = itemsLookup.get(id); + + if (nextNode === undefined) { + changes.push({ id, type: 'remove' }); + } + } + + return changes; +}