[V12.next12] updateNodeData / updateNode will reset node width / height if not provided in the update

[bogris]

Describe the Bug

Using v12.next12, I implemented updateNodeData function to ... update the node data 👯

it will do the update as expected, but the node width/height gets reseted after the update.

the folowing 2 function will reset the node width / height after execution. Data is updated corectly.

updateNodeData(node.id, data)

updateNode(node.id, {
        data
      })

this on the other hand works as expected:

 updateNode(node.id, {
        data,
        width: node.computed?.width,
        height: node.computed?.height,
      })

Note: when I insert the node I provide the width and hight as props for the node:

{
    type: 'custom-one',
    id: Math.random().toString(36).substring(7),
    data: {
      label: 'Test node 0-2',
      expanded: true,
      className: 'flex flex-col gap-2 p-1 pt-20',
    },
    parentNode,
    extent: 'parent',
    width: 220,
    height: 100,
    position: { x: x || 10, y: y || 50 },
  }

the nodes in question are children of another node. Troughout the lyfecicle of the node I am updating the width and height, and I am reading it from "computed" (very clear now)

My oppinion: It looks like it is doing a node rebuild based on the info I provided instead of merging it with the info that was already in the node. I would not expect this as the typing on the updateNode is Partial. I feel that updateNodeData and updateNode maybe share the same internals?... idk.

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

1. trigger updateNode() with new data
2. the widht / height of the node are recalculated based on the content I have inside.

Expected behavior

width and height should remain unchenged from what was there.

Screenshots or Videos

No response

Platform

• OS: macOS 14.0
• Browser: Chrome 123
• Version: 12.next12

Additional context

No response

[moklick]

you can pass options to updateNode and updateNodeData as a third param. The default is { replace: true } for updateNode and { replace: false } for updateNodeData. Maybe a bit confusing 🤔

[bogris]

i don't know if this was the problem, because even if we replace, I would expect to replace it with a merged node (old + my changes).

in the mean time, I managed to update my code to next14 (parentNode >> parentId took me a while...) --> i really like this one 2 :)

the good news is that in 14 it works as expected in both functions, with or without the 3-rd param.

I don't have any functionality linked to node replacements to go deeper into this.

Regarding the default, my vote goes to: either one, but make them the same on both functions. For me at least, (where I had the functions manually implemented in prev versions) I feel that the API for both should be the same as they kind of serve simmilar purpose.

I don't have a proper understanding on the side effects for the replace to say true/false.

I think we can close this one corect?

[bogris]

before we close it, on the same topic,

in next14, when I call updateNode / updateNodeData, the change that comes in contains a replace for all the nodes.

I am logging the changes array in the onChange function

I also have zustand implemented, but I feel that it should't affect this.

[moklick]

New default since 12.0.0-next.15 is replace: false for updateNode and updateNodeData