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
21.84k stars 1.45k forks source link

useUpdateNodeInternals doesn't update edges after moving handles #2008

Closed AndrewRayCode closed 1 year ago

AndrewRayCode commented 2 years ago

Describe the Bug

I move my component handles around dynamically. The edges to those handles don't move, and any additional attempt to connect edges to the moved handles fails, the edges get connected to the wrong place. I'm using useUpdateNodeInternals but it doesn't work as I would expect.

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

I have a custom handle component like this:

const CustomHandle = ({ nodeId, id, handleIndex, ...props }: any) => {
  const updateNodeInternals = useUpdateNodeInternals();
  useEffect(() => {
      updateNodeInternals(nodeId);
  }, [nodeId, updateNodeInternals, handleIndex, id]);

  return <Handle id={id} {...props} />;
};

this is to try to force the edges to update correctly after I move handles around. I render it like this:

{data.inputs.map((input, index) => (
          <React.Fragment key={input.name}>
            <div
              className="react-flow_handle_label"
              style={{
                top: `${handleTop - textHeight + index * 20}px`,
                left: 15,
              }}
            >
              {input.name}
            </div>
            <CustomHandle
              handleIndex={index}
              nodeId={id}
              id={input.name}
              className={cx({ validTarget: input.validTarget })}
              type="target"
              position={Position.Left}
              style={{ top: `${handleTop + index * 20}px` }}
            />
          </React.Fragment>
        ))}

Expected behavior

In this case the effect fires, but the edge does not get moved to the right place. The edge stays stuck to the stale handle even after the handles update and the effect runs.

Sadly, this works:

const CustomHandle = ({ nodeId, id, handleIndex, ...props }: any) => {
  const updateNodeInternals = useUpdateNodeInternals();
  useEffect(() => {
    setTimeout(() => {
      updateNodeInternals(nodeId);
    }, 0);
  }, [nodeId, updateNodeInternals, handleIndex, id]);

  return <Handle id={id} {...props} />;
};

the settimeout indicates to me there's some state/updating issue going on.

Screenshots or Videos

No response

Platform

Mac Chrome 10.0.3

Additional context

Related to https://github.com/wbkd/react-flow/issues/916

Discord convo starts here https://discord.com/channels/771389069270712320/859774873500778517/956236629603926046

As a consumer of react-flow, useUpdateNodeInternals is the first part of the API I personally find "ugly" - it's an imperative manual user step and it's unclear when it should be fired, and this issue indicates to me it's exposing some state update ordering that the end consumer shouldn't have to worry about. Also this is not something obvious to google for, I didn't discover it in the docs myself, someone in discord pointed me to this API. Ideally updating handles would "just work" out of the box.

AndrewRayCode commented 2 years ago

Actually after my setTimeout update, it put the edge right in the middle of the node, so I think i'm still missing something

Screen Shot 2022-03-23 at 11 56 12 AM
Sec-ant commented 2 years ago

do you have animations when moving the handles?

AndrewRayCode commented 2 years ago

I don't animate the position of anything in my graph. I have some css animations which animate the opacity of drop shadows and border colors when dragging connection lines to indicate which targets are droppable, but I don't think that's what you mean

onlaps commented 2 years ago

having same problem with repositioning edge when node's size is chaning

andreas-schultz commented 2 years ago

This might be a regression. I had the same problem (putting all edges into the middle) with a 9.x version, changing the version to 9.6.1 made updateNodeInternals work again for me. Just checked, version 9.7.4 also seems to work fine.

CamboimGabriel commented 2 years ago

did you solved?

moklick commented 1 year ago

Did you try to add props.style to the useEffect ? Could someone create a codesandbox or repo for this specific case?

moklick commented 1 year ago

Could you try to use v10.3.15 ? It should work better now.

RAFA3L commented 1 year ago

I have a similar problem trying to delete orphans edges dynamically, my handles are dependent of values in fields, so I need to delete the edges when the fields are updated and the handles removed.

This is weird, inside a custom node I try this:

useEffect(() => {
  console.log(nodeInternals); // updated
  console.log(nodeInternals.get(props.id)); // old
}, [nodeInternals, props.id]);

The first log print the updated state of the handleBounds (inspecting the node in the tree map), but the second print the node with the old state.

Using the setTimeout suggested by @AndrewRayCode works, so I can get the updated handleBounds and remove the edges:

  useEffect(() => {
    setTimeout(() => {
      const node = nodeInternals.get(props.id);
      const propSymbols = Object.getOwnPropertySymbols(node);
      const handleBounds = node[propSymbols[0]].handleBounds;
      const orphans = edges.filter(
        (e) =>
          e.source === node.id &&
          !handleBounds.source.find((h) => h.id === e.sourceHandle)
      );
      reactFlowInstance.deleteElements({ edges: orphans });
    }, 0);
  }, [nodeInternals, props.id, edges, reactFlowInstance]);

And in my field changes handle something like this:

  const handleChanges = (e) => {
    updateNodeFields(node.id, e.formData, e.errors);
    updateNodeInternals(node.id);
  };

Regards

Timot05 commented 3 weeks ago

I'm running into similar issues using reactflow 11.11.2. Is there additional documentation about how to solve this issue?

moklick commented 3 weeks ago

Hey @Timot05 could you provide a codesandbox or repo for this?

Timot05 commented 3 weeks ago

Here is a codesandbox that is a modified version of this example. Pressing on the button moves the handles. The links don't update though.

bcakmakoglu commented 3 weeks ago

@Timot05 You're passing data.id to updateNodeInternals but there is no id in your data object. The data object is meant for you to put in some arbitrary data you need for your business logic, it doesn't contain the node properties.

  function handleUpdateNodes() {
    setPos(pos + 5);
    updateNodeInternals(data.id); // <--- `data.id` doesn't exist, you're passing `undefined` here
  }

To fix the problem you need to pass the actual node id to updateNodeInternals

// `id` is a prop that is passed to custom nodes
export default memo(({ id }) => {
  const updateNodeInternals = useUpdateNodeInternals();

  const [pos, setPos] = useState(5);

  function handleUpdateNodes() {
    setPos(pos + 5);
    updateNodeInternals(id); // <-- pass the node id to `updateNodeInternals`
  }

Here's your sandbox corrected.

Timot05 commented 3 weeks ago

Thanks @bcakmakoglu, your sandbox doesn't seem to be available publicly.

I applied the change myself and this indeed solved the issue. In my project, I was using updateNodeInternals from the parent object, which didn't seem to be working properly. But moving it inside solved it. Thanks!