xplato / useUndoable

↪ React hook for undo/redo functionality (with batteries included)
MIT License
168 stars 7 forks source link

Problem with initial rendering in react-flow v10 when useUndoable #11

Closed dincho-kostadinov closed 2 years ago

dincho-kostadinov commented 2 years ago

Hello , I have problem with initial rendering with react-flow don't see the diagram when using the hook. I attached some code

const [nodes, setNodes, { undo: undoNodes, canUndo: canUndoNodes, redo: redoNodes, canRedo: canRedoNodes, resetInitialState: resetInitialStateNodes }] = useUndoable<Node[]>([]); const [edges, setEdges, { undo: undoEdges, canUndo: canUndoEdges, redo: redoEdges, canRedo: canRedoEdges, resetInitialState: resetInitialStateEdges }] = useUndoable<Edge[]>([]);

` useEffect(() => { // get data from the api setNodes(responseFromTheApi); resetInitialStateNodes(responseFromTheApi);

  setEdges(responseFromTheApi);
  resetInitialStateEdges(responseFromTheApi);
}

}, []);`

const onNodesChange = useCallback((changes) => { setNodes((nds) => applyNodeChanges(changes, nds)); }, [nodes]);

const onEdgesChange = useCallback((changes) => { setEdges((eds) => applyEdgeChanges(changes, eds)); }, [edges]);

` <ReactFlow nodes={nodes} edges={edges} onNodesChange={onNodesChange} onNodeDragStop={onNodesDragSTop} onEdgesChange={onEdgesChange}

`

When I load the page I don't see the diagram if I change the useUndoable with sample useState it works as expected. I checked the response from the api it was returned and it looks fine. So any idea?

xplato commented 2 years ago

Unfortunately, there's not enough information here in the state to know why it's not rendering initially. Your code, although a bit hard to read with the formatting, looks somewhat fine to me (more on this later).

If I had to guess, I'd say it is due to some loading issue; perhaps your API call is blocking the render somehow? One thing I can suggest is to manually control the rendering of the <ReactFlow /> component using a pattern similar to the one I spoke about in react-flow #1712. Let me know how this goes.

Now, one thing I did notice here is that you are using two instances of useUndoable. While this is technically fine, I will warn you that there is the chance that your states could easily get out of sync. Generally, it would be preferable to have one instance that tracks nodes and edges separately, in an object that looks similar to:

useUndoable({
    nodes: [],
    edges: [],
})

Obviously, I do not know what your project is or if separating them makes sense, so use your judgement here—I just thought I should point this out as this is where a lot of bugs originate from.

alor commented 2 years ago

I suggest to look at the closed issues and see how to handle the {nodes: [], edges: []} state for the v10. then instead of using useEffect you can exploit the onInit callback of react-flow and set the state inside of it.

xplato commented 2 years ago

I'm going to close this due to inactivity.