fix: stops memory leak from ContextMenu by fixing endless render loop

[codemile]

Closes #278

Summary

There is a memory leak in the app where the WebView2 would hit 2gb of Ram and trigger the "This page is having a problem". I found references to WebView2 having memory leaks in Tauri via Google searches, and on Windows platform Tauri is using Microsoft Edge as it's WebView. I couldn't confirm if Mac users were experiencing this bug or not. I was able to reproduce the problem on Windows by opening multiple projects and leaving the app running.

Here we can see memory increasing at a rate of 1mb/s while the app is idle.

rivet-memory.mp4

I started debugging the leak by removing large chunks of code from the Rivet app until the leaks stopped. This process revealed the leaks to be coming from inside <NodeCanvas> component. There I narrowed the problem down to the <ContextMenu>.

I concluded by assumption that @floating-ui/react is the source of the memory leaks, and found these refs to people discussing memory leaks with the library. We use this library for positioning the popup context menu, and in addition wrap the <ContextMenu> inside a <CssTransition> which keeps the component in the DOM even when it's not visible.

Memory Leak references:

floating-ui/floating-ui#2576

floating-ui/floating-ui#744

These things combined to create a problem, because <ContextMenu> was stuck in an endless render loop. Since <CssTransition> keeps it in the DOM and @floating-ui/react leaks memory with each render, then the render loop was causing the 1mb/s leak.

Here we can see the render loop:

rivet-memory2.mp4

I traced the render loop down to the useContextMenuAddNodeConfiguration() hook where it uses useAsyncEffect() to get the uiData for the menu. The async hook would call setUiData() when done, this would trigger another render and the hook would fetch all the uiData again and call setUiData again in an endless loop.

The fix ended up being the use of a dependency array on the hook. Once applied the memory leaks stopped, and I also moved the constructors variable outside of the hook as it's an external dependency.

[codemile]

I converted this PR to a draft, based on feedback from @abrenneke the constructors dependency is not stable and needs to be handled within the hook.

In our discussion, Andy mentioned there might be an atom already we can use or we need to convert getNodeConstructors() call into an atom. Either way, placing this function call inside the hook triggered endless re-rendering of the context menu.

[abrenneke]

I looked, there wasn't an atom for node constructors, but I think one could be made really easily.

[abrenneke]

Fixed by e90d292