fix(gui-client): remove bad hook dependencies#9537
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a performance issue in the GUI client by removing unnecessary hook dependencies that led to an endless update loop causing CPU and memory spikes.
- Removed 'location' and 'handleClick' from the useEffect dependency array.
- Prevents repeated sending of the update_state command to the backend.
| logsRecountedUnlisten.then((unlistenFn) => unlistenFn()); | ||
| }; | ||
| }, [location, handleClick]); | ||
| }, []); |
There was a problem hiding this comment.
Ensure that none of the values 'location' or 'handleClick' are referenced within the effect body after removing them from the dependency array to avoid stale closures.
jamilbk
left a comment
jamilbk
left a comment
There was a problem hiding this comment.
It's a bit worrying this only popped up when wired up to the Rust proc. I think the lesson learned here is that making GUI changes requires the developer to be able to spin up the full GUI client, not just run the UI with pnpm vite dev as it risks these types of side effect issues.
We also need more static analysis. I believe eslint would have caught this with the React plugin. Not resulting issue but the violation of the Rules of Hooks. |
The removed hook dependencies are invalid because the side-effect specified in
useEffectdoes in fact not depend on them. However, as a result of these dependencies, theuseEffectclosure appears to run in an end-less loop, constantly sending theupdate_statecommand to the backend which in turn re-sends all state to the frontend, causing a massive CPU and memory spike.Resolves: #9519