Use onNodesChange to change nodes, causing nodes to be overwritten

[zyh374892663]

Describe the Bug

When using onNodesChange to change the nodechange of type reset, the existing nodes are overwritten by the item of the reset nodechange

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

1. During initialization, map nodes and edges to the source data and cache them
2. After connecting, remap the nodes and edges, compare the cached data, find out the node that needs to be changed, and set the nodechanges array (because the custom data will carry some necessary data, the node needs to be reset)
3. Process nodechanges through onNodesChange

Expected behavior

The original number of nodes remains unchanged, and the data data of the corresponding node in the reset is updated

Screenshots or Videos

Uploading 20220615_103320.mp4…

Platform

• OS: Windows
• Browser: Electron
• Version: ^10.3.4

Additional context

No response

[zyh374892663]

Replenish：During initialization, the nodes are mapped out through the source data, and then converted into nodechanges through nodes, and new nodes are added through onNodesChange

[zyh374892663]

The problem may be here: after filtering nodeChanges, return directly, elements give up directly

[moklick]

Hey @zyh374892663

thanks for the report. Could you explain the issue here? Sorry, but I don't get it.

[zyh374892663]

@moklick I have a set of source data from which nodes are generated and updated on the view via onNodesChange. When the source data changes, generate nodes again and compare it with the old nodes to find out which nodes need to be changed. For data with differences, an array with a structure of NodeResetChange is generated and updated to the view through onNodesChange. It is expected that only nodes with differences will be updated, and nodes without differences will not be changed. It turns out that the difference is updated to the view, and the node with no difference is overwritten and disappeared. Is this description understandable?

[zyh374892663]

@moklick This picture is ugly, please forgive me

[moklick]

Thanks for the explanation but I think a codesandbox would be more helpful here. Could you create a simplified example?

[zyh374892663]

@moklick https://codesandbox.io/s/elastic-surf-zocuhj?file=/src/ReactFlow.js This is an online demo, hope it helps

[moklick]

What are you trying to achieve here? Normally you don't need to care about node changes and you don't need to trigger them. They are meant to be used by the library to communicate changes to user land. In you case you could do something like:

const add = (nodeId) => {
  setNodes(nds => nds.map((node) => {
    if (node.id === nodeId) {
      node.data = {
        ...node.data,
        count: node.data.count + 1;
      };
    }

   return node;
  }));
}

[zyh374892663]

@moklick For example: if current needs to be displayed on the node title, the current that simply modifies the source data cannot be updated to the view, and it is expected to update the current node through reset

export function sourceMapNode(source) {
  return source.map((item) => {
    return {
      id: item.id,
      type: "new-node",
      data: {
        label: "id: " + item.id + ", current:" + item.count,
        count: item.count
      },
      position: item.position
    };
  });
}

Examples are updated, please follow the TODO comments

[zyh374892663]

https://user-images.githubusercontent.com/33707926/174569073-ed939d0a-fe44-4923-9456-b420b803d823.mp4

[moklick]

using a change event from type reset is not the way to update nodes. You should use the setNodes function and not use the internal events for that kind of task.

[zyh374892663]

@moklick What are the usage scenarios for the types NodeResetChange and EdgeResetChange? According to the data structure ({type: 'reset', item: Node}), the current node should be reset. If it's to reset the canvas, why use this form. It is not more convenient to directly cover all old nodes without filter. If you use NodeResetChange, NodeAddChange and NodeRemoveChange at the same time, won't only NodeResetChange be triggered. Or is it possible to add the NodeUpdateChange type later?

[zyh374892663]

As shown above, if changes contains NodeAddChange, NodeRemoveChange, NodeResetChange at the same time, it only takes effect for NodeResetChange

[moklick]

We need the reset type to be able to use hooks like setNodes or setEdges with a controlled flow. As I said there is no reason to use these events in user land. It's only meant to be used internally. A reset events means that the changes overwrite anything that is in the user state. That's why it gets overwritten.