The previouscreate function was super weird because it was responsible for three things

• Creating a new WebSocket connection from the outside effect
• Being able to create new connections by calling itself recursively
• Also acting as auseEffect cleanup function

Figured it'd be better to split up those responsibilities

This is needed to remove Biome'suseExhaustiveDependencies warning without introducing unnecessary renders

This is in regards touseEffect? I think if applicable it'd be better to use a biome comment to ignore linting on the line with an explanation why

// biome-ignore lint/correctness/useExhaustiveDependencies: reasonuseEffect(()=>{if(cachedVersionId!==templateVersionId){setCachedVersionId(templateVersionId);setLogs([]);}},[...]);

Well, the linter warning is actually valid – theuseEffect approach introduces additional renders and has a risk of screen flickering, while the if statement approach avoids all of that

Was mainly calling it out, because while it's the "correct" way to do things (and is shouted out in the React docs), I do think it's an ugly pattern, and you need a lot of context for how React works under the hood to knowwhy it's better than theuseEffect approach

The previous version of the effect was re-synchronizing way too often, because we put a function without a stable memory reference andtemplateVersionStatus directly into the dependency array:

• Function: Every render would create a new reference and destroy the existing connection for no good reason
• Status: The value can be a lot of things that indicate that we shouldn't watch (including undefined). Plus, if the status ever switches from pending to running, that's a new value for the array, and triggers a re-sync, destroying a perfectly good connection

Best option seemed to be to wrap the callback, and also collapse the status to a boolean

Biome was complaining about having default parameters not be at the end of the function signature, but also, we were never calling these functions with an explicit value ofundefined

This was removed because it introduces unnecessary renders – better to just usestorybookMetadata as the initial value for the state and avoid a bunch of state syncs

Really wanted to just make sure that the logs is always an array, and get rid of theundefined case to make the state updates cleaner, but a bunch of our other components currently depends on that setup

This is in regards touseEffect? I think if applicable it'd be better to use a biome comment to ignore linting on the line with an explanation why

// biome-ignore lint/correctness/useExhaustiveDependencies: reasonuseEffect(()=>{if(cachedVersionId!==templateVersionId){setCachedVersionId(templateVersionId);setLogs([]);}},[...]);

Sadly, this is the best we can do as far as giving ourselves type-safety (without updating GUTS). It'll be less of an issue for WebSocket connections that don't send SSE-formatted events, but the ServerSentEvent type is structured like this:

// From codersdk/serversentevents.goexportinterfaceServerSentEvent{readonlytype:ServerSentEventType;// empty interface{} type, falling back to unknownreadonlydata:unknown;}

There's no way to pass a type parameter to the type and makedata more specific