Ironclad / rivet

The open-source visual AI programming environment and TypeScript library
https://rivet.ironcladapp.com
MIT License
2.56k stars 226 forks source link

fix: adds a temp cache of node heights to fix node flickering #280

Closed codemile closed 5 months ago

codemile commented 6 months ago

This PR closes #277 by uses a memory "cache" to store node heights. These heights are then used by the UnknownNodeBody component when the body is not yet available. This resolves the reported issue, because the components are unmounted and mounted when being moved in the DOM structure from the canvas to the dragging list and then back again. The flickering happens because the body component is rendered async and isn't ready on the first render pass. The cache acts as a "last known height" value when the body is not ready.

The fix is a bit hacky in nature. I tried my best to isolate it to separate hooks, and this approach seemed to have the smallest footprint of changes. It did require a "cache" prop to be prop drilled down to the NodeBody component. There is a nodes.reduce() call that rebuilds the cache when nodes change to prevent memory leaks. The performance seems to be fine as the the nodes dependency only changes when nodes are selected.

Note: I couldn't figure out how useUnknownNodeComponentDescriptorFor worked, and only applied the height fix to the UnknownNodeBody component. It's possible other node types could use this fix as well. Maybe at a later date, when I know more, I can apply the fixes there.

Video recording before the PR fix:

https://github.com/Ironclad/rivet/assets/50146659/9fe5586c-d654-4388-96e6-88af4be10bba

Video recording after the PR fix:

https://github.com/Ironclad/rivet/assets/50146659/5a562b95-beb9-4efa-9e81-3f5fed43cf9d