bcakmakoglu / vue-flow

A highly customizable Flowchart component for Vue 3. Features seamless zoom & pan 🔎, additional components like a Minimap 🗺 and utilities to interact with state and graph.
https://vueflow.dev/
MIT License
4.03k stars 259 forks source link

🐛 [BUG]: node change events not triggered when removing nodes #1486

Closed chrschdev closed 4 months ago

chrschdev commented 4 months ago

Is there an existing issue for this?

Current Behavior

Use this example from docs to delete a node from nodes array: https://vueflow.dev/guide/node.html#removing-nodes-from-the-graph

<VueFlow :nodes="nodes" @nodes-change="() => console.log('nodes change')" @nodes-initialized="() => console.log('nodes initialized')">

Events nodes-change and nodes-initialized not emitted.

const { onNodesChange } = useVueFlow()

onNodesChange also not emitted

but the watcher

const { getNodes } = useVueFlow()
watch(getNodes, (nodes) => console.log('nodes changed', nodes))

react to the changes.


But when adding a node to the same nodes array, all of those events will be emitted

Expected Behavior

I expect that

onNodesChange and @nodes-change and @nodes-initialized (?)

will be emitted when deleting some nodes from array

Steps To Reproduce

https://vueflow.dev/guide/node.html#removing-nodes-from-the-graph

<script setup>
import { ref } from 'vue'
import { VueFlow, Panel } from '@vue-flow/core'

const nodes = ref([
  {
    id: '1',
    position: { x: 50, y: 50 },
    data: { label: 'Node 1' },
  },
  {
    id: '2',
    position: { x: 150, y: 50 },
    data: { label: 'Node 2' },
  }
])

function removeNode(id) {
  nodes.value = nodes.value.filter((node) => node.id !== id)
}
</script>

<template>
  <VueFlow :nodes="nodes">
    <Panel>
      <button type="button" @click="removeNode('1')">Remove Node 1</button>
      <button type="button" @click="removeNode('2')">Remove Node 2</button>
    </Panel>
  </VueFlow>
</template>

Relevant log output

No response

Anything else?

My use-case is, that i read-in the tree from a ordered list of items (that a part of a store), and when all nodes are initialized, i do some additional layouting with dagre library. That means, after each change of nodes property, i need to know if vue-flow nodes are initialized so that i can apply the dagre layout.

When i add nodes, @nodes-initialized event is fired, that seems to be useful to apply dagre layout. but this event is not emitted when removing nodes from nodes array.

No response

bcakmakoglu commented 4 months ago

onNodesInitialized will not trigger when removing nodes. Logically there is no reason to emit a node "initialized" event if a node is removed - no nodes were initialized (meaning a size was measured by resize observer).

Removing nodes create a change just as would be expected, so I can't reproduce that part of the problem and if it exists please provide a proper reproduction of the problem.

bcakmakoglu commented 4 months ago

I overlooked one part which seems crucial for the explanation regarding this issue.

function removeNode(id) {
  nodes.value = nodes.value.filter((node) => node.id !== id)
}

when you use the function above to remove a node (which is a completely valid way to do it, mind you ^^), there won't be any changes created. <VueFlow> will not try to track your nodes and react to filtering out a node by creating a change, communicating that back to you etc. The function above does not go the path of creating a change.

The idea of onNodesChange is communicating changes triggered either internally, for example by pressing Backspace on a node to delete it, or through the use of helpers, for example removeNodes of useVueFlow, to the userland so you can decide (given you turn of automatic change application) whether those changes should even be applied or not.

So if you want onNodesChange to trigger in this case you would want to use this instead

const { removeNodes } = useVueFlow()

removeNodes(nodeId)

Looks like this needs to be documented more clearly to prevent this sort of misunderstanding and clearly explain what changes actually mean (it does not mean any change as explained above) and when these are communicated through the given events like onNodesChange.

chrschdev commented 4 months ago

Thanks for your quick answers.

The misleading part of that for me is, that pushing a new entry to the nodes array triggers onNodesChange, but removing not.

bcakmakoglu commented 4 months ago

It does but if you inspect the change closely it's not a change of type add but a change of type dimensions which will always be generated for nodes that are mounted and rendered as these are created and pushed whenever a ResizeObserver measures dimensions of a node

chrschdev commented 4 months ago

Okay thanks, didn't realized that

rdvo commented 3 months ago

is there a way to prevent a ndoe being deleted from the 'del' or 'bacjspace' key ?

I want some nodes always on the canvas