xyflow / xyflow

React Flow | Svelte Flow - Powerful open source libraries for building node-based UIs with React (https://reactflow.dev) or Svelte (https://svelteflow.dev). Ready out-of-the-box and infinitely customizable.
https://xyflow.com
MIT License
22.07k stars 1.46k forks source link

Some edges are not rendered when many nodes change position in one moment. #3198

Open StrawDragon opened 11 months ago

StrawDragon commented 11 months ago

Describe the Bug

Hello!

When I replace many nodes in one moment (after run auto-layout for example), some edges don't render. I solved the problem by little hack (setting viewport by setCenter to far away and back). After that I added a history managment with undo/redo and if I have many nodes with changing position and call a function "Undo" then some edges don't render again. I don't want to use the previous workaround because it's inconvenient. I didn't find any function for force rerender a graph

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

Replace many nodes in one moment after auto-layout or another bulk operations

Expected behavior

All edges are visible

Screenshots or Videos

No response

Platform

Additional context

No response

StrawDragon commented 11 months ago

The attached video shows an example of the bug when importing a graph from a file. As you can see, some edges are not rendered after import. But if I move viewport ouside the graph and back the edges rerendered correctly https://drive.google.com/file/d/1MJC-KL1lppLGwf1AIeoVaOQCSrNZ5rFh/view?usp=sharing

moklick commented 11 months ago

Hey! Normally no forced re-render is required. Do you get any logs in the dev tools? Are you able to reproduce this within a codesandbox https://new.reactflow.dev/react-ts ?

StrawDragon commented 11 months ago

I try to reproduce this within a codesandbox, but all work right and i don't understand there we have trouble. https://codesandbox.io/s/priceless-water-tl3j75?file=/ShafleButton.js

StrawDragon commented 11 months ago

I have many warnings like this -

[React Flow]: Couldn't create edge for source handle id: "caf81d80-775e-48f8-8f39-cc3f291caf37", edge id: caf81d80-775e-48f8-8f39-cc3f291caf37-1_2. Help: https://reactflow.dev/error#008

  | devWarn | @ | index.js:149
  | (anonymous) | @ | index.js:3287
  | (anonymous) | @ | index.js:3263
  | EdgeRenderer | @ | index.js:3263
  | renderWithHooks | @ | react-dom.development.js:16305
  | updateFunctionComponent | @ | react-dom.development.js:19588
  | updateSimpleMemoComponent | @ | react-dom.development.js:19425
  | beginWork | @ | react-dom.development.js:21678
  | beginWork$1 | @ | react-dom.development.js:27426
  | performUnitOfWork | @ | react-dom.development.js:26557
  | workLoopSync | @ | react-dom.development.js:26466
  | renderRootSync | @ | react-dom.development.js:26434
  | performSyncWorkOnRoot | @ | react-dom.development.js:26085
  | flushSyncCallbacks | @ | react-dom.development.js:12042
  | commitRootImpl | @ | react-dom.development.js:26959
  | commitRoot | @ | react-dom.development.js:26682
  | finishConcurrentRender | @ | react-dom.development.js:25981
  | performConcurrentWorkOnRoot | @ | react-dom.development.js:25809
  | workLoop | @ | scheduler.development.js:266
  | flushWork | @ | scheduler.development.js:239
  | performWorkUntilDeadline | @ | scheduler.development.js:533
moklick commented 11 months ago

It seems that some edges can't be created because the nodes are not rendered yet. Could you try to wrap your setEdges function with a setTimeout or a requestAnimationFrame to rule out that we are dealing with race conditions?

StrawDragon commented 11 months ago

We use setTimeout and it help for another places but it does not work here. In video example we add some placeholders nodes and edges for this nodes after auto-layout and we use for this setTimeout.

StrawDragon commented 11 months ago

I resolve my problem only another workaround( But it work.

            setEdges([]);
            setNodes([]);
            setTimeout(() => {
                setNodes(graphNodes);
                setEdges(graphEdges);
                setTimeout(() => {
                    handleRunAutolayout();
                }, 0);
            }, 0);
moklick commented 11 months ago

argh.. how are you managing your state?

StrawDragon commented 11 months ago

Redux and redux-saga. I get the graph data. I put it in redux. After that I put data in reactflow and i manage reactflow state after that only by setNodes and setEdges. But we have Import-button. Because I need replace graph data to another in one moment.

MainFile

import React, {useCallback, useEffect, useMemo} from 'react';
import {useSelector} from 'react-redux';
import ReactFlow, {
    Background,
    Controls,
    MiniMap,
    Node,
    useEdgesState,
    useNodesState,
    useStore,
    NodeDragHandler,
} from 'reactflow';

import {useGraphApiContext} from 'containers/ScenariosEditor/contexts/GraphApiProvider';
import {bound as graphLoadActions} from 'containers/ScenariosEditor/reducers/graph/item/load/actions';
import {isGraphNeedAutolayoutSelector, nodesInitializedSelector} from 'containers/ScenariosEditor/selectors/graph/item/load';
import {graphEdgesSelector} from 'containers/ScenariosEditor/selectors/graph/item/load/graphEdgesSelector';
import {graphNodesSelector} from 'containers/ScenariosEditor/selectors/graph/item/load/graphNodesSelector';

import BackButton from './components/Controls/BackButton';
import BottomControls from './components/Controls/BottomControls';
import TopControls from './components/Controls/TopControls';
import {
    BACKGROUND_NET_COLOR,
    BACKGROUND_NET_GAP,
    DEFAULT_EDGE_OPTIONS,
    MINIMAP_STYLE,
    NODE_TYPES,
    EDGE_TYPES,
    PRO_OPTIONS,
    MIN_ZOOM,
} from './consts';
import styles from './Graph.module.css';
import {useAutoLayout} from './hooks/useAutoLayout';
import {useUndoRedo} from './hooks/useUndoRedo';
import {NodeType, NodeTypeEnum, Props} from './types';
import {movePlaceholders, addPlaceholders} from './utils/placeholderNodes';
import {makeHandleConnection} from './utils/utils';

import 'reactflow/dist/style.css';
import './reset.css';

const Graph: Props = ({handleClose}) => {
    const {setSelectedNode, reactFlowWrapper} = useGraphApiContext();
    const graphNodes = useSelector(graphNodesSelector);
    const graphEdges = useSelector(graphEdgesSelector);
    const isInitialized = useStore(nodesInitializedSelector);
    const isNeedAutolayout = useSelector(isGraphNeedAutolayoutSelector);
    const [nodes, setNodes, onNodesChange] = useNodesState(graphNodes);
    const [edges, setEdges, onEdgesChange] = useEdgesState(graphEdges);
    const {takeSnapshot} = useUndoRedo();
    const handleConnect = useMemo(
        () => makeHandleConnection(setNodes, setEdges, nodes, edges, takeSnapshot),
        [setEdges, setNodes, nodes, edges, takeSnapshot],
    );
    const handleNodeClick = useCallback((_event: React.MouseEvent, node: Node) => {
        if (node.type as NodeTypeEnum === NodeTypeEnum.Input) {
            return;
        }
        setSelectedNode?.({
            ...node,
            type: node.type ? node.type as NodeTypeEnum : NodeTypeEnum.Condition,
        });
    }, [setSelectedNode]);
    const handleNodeDragStop = useCallback((event: React.MouseEvent, node: Node) => {
        graphLoadActions.markAsEdited();
        movePlaceholders(node as NodeType, setNodes);
    }, [setNodes]);
    const handleInit = useCallback(() => {
        addPlaceholders(setNodes, setEdges, edges);
    }, [setNodes, setEdges, edges]);
    const {handleRunAutolayout} = useAutoLayout();

    const onNodeDragStart: NodeDragHandler = useCallback(() => {
        takeSnapshot();
    }, [takeSnapshot]);

    // После импорта графа запускаем автопозиционирование
    useEffect(() => {
        if (isNeedAutolayout && isInitialized) {
            takeSnapshot();
            setNodes(graphNodes);
            setEdges(graphEdges);
            setTimeout(() => {
                handleRunAutolayout();
            }, 0);
            graphLoadActions.setNeedAutolayout(false);
        }
    }, [isNeedAutolayout, graphNodes, graphEdges, setEdges, setNodes, handleRunAutolayout, isInitialized, takeSnapshot]);

    return (
        <div className={styles.wrapper} ref={reactFlowWrapper}>
            <ReactFlow
                nodes={nodes}
                edges={edges}
                defaultEdgeOptions={DEFAULT_EDGE_OPTIONS}
                onNodesChange={onNodesChange}
                onEdgesChange={onEdgesChange}
                onNodeClick={handleNodeClick}
                onConnect={handleConnect}
                onNodeDragStart={onNodeDragStart}
                onNodeDragStop={handleNodeDragStop}
                onlyRenderVisibleElements
                onInit={handleInit}
                elevateEdgesOnSelect
                fitView
                snapToGrid
                minZoom={MIN_ZOOM}
                attributionPosition="top-right"
                nodeTypes={NODE_TYPES}
                edgeTypes={EDGE_TYPES}
                deleteKeyCode={null}
                proOptions={PRO_OPTIONS}
            >
                <Background gap={BACKGROUND_NET_GAP} color={BACKGROUND_NET_COLOR}/>
                <BackButton handleClose={handleClose}/>
                <TopControls/>
                <BottomControls handleClose={handleClose}/>
                <MiniMap style={MINIMAP_STYLE} zoomable pannable/>
                <Controls/>
            </ReactFlow>
        </div>
    );
};

export default Graph;
moklick commented 11 months ago

great that you could solve this! I will leave this issue open as a reminder. Maybe we can improve the internals a bit so that you can get rid of the timeouts and stuff.

moklick commented 10 months ago

It's weird that you are using Redux to manage the nodes state and use useNodesState. I could imagine that this is part of the issue here. You should only use Redux for the nodes and also implement an onNodesChange / onEdgesChange action for handling changes as explained here https://reactflow.dev/docs/guides/state-management/ . Another thing that looks odd is the handleConnect handler. Because you are passing nodes, this handler gets re-created every time you drag a node for example. You can get rid of it by receiving the nodes inside the handler from your Redux store.

Manuelbaun commented 10 months ago

I also get the issue. Some nodes are not rendered yet and ReactFlow wants to render the edges. Currently I set the edges after a timeout. Is there a way to know, when rendering nodes are finished?

moklick commented 10 months ago

@Manuelbaun there are some edges that are invisible or do you get a warning "couldn't create edge .."?

ohad-definity commented 9 months ago

Same issue here. I think there should be a way to "re-initialize" - clear the current state and set new graph of nodes and edges. Also re-run the fitView.

ngnaoh commented 2 months ago

Same issue as well. I just swap the position of two nodes but the position target edge do not update. I check the data still correctly and the edges update when I refesh the page. I think this occured by the edges don't rerender and dont get any errors.

chaseaucoin commented 2 weeks ago

I don't know if this helps anyone but I had to add some jitter to the Y axis to clear up this issue for me. If everything was on the same plane at 0 I would get this problem but adding a random value between 0-1. To leave it all centered I did 0.5 - random()

chaseaucoin commented 2 weeks ago

Follow up, I think I figured out my issue, It is specifically with the Bezier edges. If the edges are on the same plane the Bezier formula explodes.

jbownzino commented 5 days ago

For me, I was facing this issue and did the following:

Now, I get no "couldn't find edge" errors and everything seems to happen asynchronously after the node data is updated