Merge pull request #69 from FluxStackCore/claude/review-latest-pr-gxUMG

Improve singleton connection handling and prevent state recursion

Author: MarcosBrendonDePaula

app/client/src/components/AppLayout.tsx

@@ -26,6 +26,9 @@ const routeFlameHue: Record<string, string> = {
   '/api-test': '90deg',     // lima
 }
 
+// Cache favicon blob URLs by hue to avoid recreating blobs on every navigation
+const faviconUrlCache = new Map<string, string>()
+
 export function AppLayout() {
   const location = useLocation()
   const [menuOpen, setMenuOpen] = useState(false)
@@ -34,14 +37,18 @@ export function AppLayout() {
     const current = navItems.find(item => item.to === location.pathname)
     document.title = current ? `${current.label} - FluxStack` : 'FluxStack'
 
-    // Dynamic favicon with hue-rotate
+    // Dynamic favicon with hue-rotate (cached per hue value)
     const hue = routeFlameHue[location.pathname] || '0deg'
-    const colored = faviconSvg.replace(
-      '<svg ',
-      `<svg style="filter: hue-rotate(${hue})" `
-    )
-    const blob = new Blob([colored], { type: 'image/svg+xml' })
-    const url = URL.createObjectURL(blob)
+    let url = faviconUrlCache.get(hue)
+    if (!url) {
+      const colored = faviconSvg.replace(
+        '<svg ',
+        `<svg style="filter: hue-rotate(${hue})" `
+      )
+      const blob = new Blob([colored], { type: 'image/svg+xml' })
+      url = URL.createObjectURL(blob)
+      faviconUrlCache.set(hue, url)
+    }
     let link = document.querySelector<HTMLLinkElement>('link[rel="icon"]')
     if (!link) {
       link = document.createElement('link')
@@ -50,7 +57,6 @@ export function AppLayout() {
     }
     link.type = 'image/svg+xml'
     link.href = url
-    return () => URL.revokeObjectURL(url)
   }, [location.pathname])
 
   return (

core/server/live/ComponentRegistry.ts

@@ -15,7 +15,7 @@ import { ANONYMOUS_CONTEXT } from './auth/LiveAuthContext'
 import type { LiveComponentAuth, LiveActionAuthMap } from './auth/types'
 import { liveLog, registerComponentLogging, unregisterComponentLogging } from './LiveLogger'
 import { liveDebugger } from './LiveDebugger'
-import { _setLiveDebugger } from '@core/types/types'
+import { _setLiveDebugger, EMIT_OVERRIDE_KEY } from '@core/types/types'
 
 // Inject debugger into types.ts (server-only) so LiveComponent class can use it
 // without importing the server-side LiveDebugger module directly
@@ -290,7 +290,7 @@ export class ComponentRegistry {
       const existing = this.singletons.get(componentName)
       if (existing) {
         // Add this ws connection to the singleton
-        const connId = ws.data?.connectionId || `ws-${Date.now()}`
+        const connId = ws.data?.connectionId || `ws-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`
         existing.connections.set(connId, ws)
 
         // Initialize WebSocket data if needed
@@ -367,13 +367,13 @@ export class ComponentRegistry {
 
     // 🔗 Register singleton with broadcast emit
     if (isSingleton) {
-      const connId = ws.data.connectionId || `ws-${Date.now()}`
+      const connId = ws.data.connectionId || `ws-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`
       const connections = new Map<string, FluxStackWebSocket>()
       connections.set(connId, ws)
       this.singletons.set(componentName, { instance: component, connections })
 
-      // Override emit to broadcast to all connections
-      ;(component as any)._setEmitOverride((type: string, payload: any) => {
+      // Override emit to broadcast to all connections (via Symbol key)
+      ;(component as any)[EMIT_OVERRIDE_KEY] = (type: string, payload: any) => {
         const message: LiveMessage = {
           type: type as any,
           componentId: component.id,
@@ -383,11 +383,19 @@ export class ComponentRegistry {
         const serialized = JSON.stringify(message)
         const singleton = this.singletons.get(componentName)
         if (singleton) {
-          for (const [, connWs] of singleton.connections) {
-            try { connWs.send(serialized) } catch {}
+          const deadConnections: string[] = []
+          for (const [connId, connWs] of singleton.connections) {
+            try { connWs.send(serialized) } catch {
+              deadConnections.push(connId)
+            }
+          }
+          // Remove dead connections
+          for (const connId of deadConnections) {
+            singleton.connections.delete(connId)
+            liveLog('lifecycle', component.id, `🔗 Singleton '${componentName}' — removed dead connection '${connId}'`)
           }
         }
-      })
+      }
 
       liveLog('lifecycle', component.id, `🔗 Singleton '${componentName}' created`)
     }
@@ -428,6 +436,11 @@ export class ComponentRegistry {
       await (component as any).onMount()
     } catch (err: any) {
       console.error(`[${componentName}] onMount error:`, err?.message || err)
+      // Notify client that mount initialization failed
+      ;(component as any).emit('ERROR', {
+        action: 'onMount',
+        error: `Mount initialization failed: ${err?.message || err}`
+      })
     }
 
     // Debug: track component mount
@@ -597,6 +610,10 @@ export class ComponentRegistry {
         await (component as any).onMount()
       } catch (err: any) {
         console.error(`[${componentName}] onMount error (rehydration):`, err?.message || err)
+        ;(component as any).emit('ERROR', {
+          action: 'onMount',
+          error: `Mount initialization failed (rehydration): ${err?.message || err}`
+        })
       }
 
       return {
@@ -620,7 +637,7 @@ export class ComponentRegistry {
     }
     if (!ws.data) {
       (ws as { data: FluxStackWSData }).data = {
-        connectionId: `ws-${Date.now()}`,
+        connectionId: `ws-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
         components: new Map(),
         subscriptions: new Set(),
         connectedAt: new Date(),
@@ -632,37 +649,41 @@ export class ComponentRegistry {
     }
   }
 
+  /**
+   * Remove a single connection from a singleton. If no connections remain, destroy it.
+   * @returns true if the component was a singleton (handled), false otherwise
+   */
+  private removeSingletonConnection(componentId: string, connId?: string, context = 'unmount'): boolean {
+    for (const [name, singleton] of this.singletons) {
+      if (singleton.instance.id !== componentId) continue
+
+      if (connId) singleton.connections.delete(connId)
+
+      if (singleton.connections.size === 0) {
+        // Last connection gone — destroy singleton fully
+        this.cleanupComponent(componentId)
+        this.singletons.delete(name)
+        liveLog('lifecycle', componentId, `🗑️ Singleton '${name}' destroyed (${context}: no connections remaining)`)
+      } else {
+        liveLog('lifecycle', componentId, `🔗 Singleton '${name}' — connection removed via ${context} (${singleton.connections.size} remaining)`)
+      }
+      return true
+    }
+    return false
+  }
+
   // Unmount component (with singleton awareness)
   async unmountComponent(componentId: string, ws?: FluxStackWebSocket) {
     const component = this.components.get(componentId)
     if (!component) return
 
     // 🔗 Singleton: remove connection, only destroy when last client leaves
-    for (const [name, singleton] of this.singletons) {
-      if (singleton.instance.id === componentId) {
-        if (ws) {
-          const connId = ws.data?.connectionId
-          if (connId) singleton.connections.delete(connId)
-          ws.data?.components?.delete(componentId)
-        }
-
-        if (singleton.connections.size === 0) {
-          // Last connection gone — destroy singleton
-          liveDebugger.trackComponentUnmount(componentId)
-          component.destroy?.()
-          this.unsubscribeFromAllRooms(componentId)
-          this.components.delete(componentId)
-          this.metadata.delete(componentId)
-          this.wsConnections.delete(componentId)
-          this.singletons.delete(name)
-          performanceMonitor.removeComponent(componentId)
-          unregisterComponentLogging(componentId)
-          liveLog('lifecycle', componentId, `🗑️ Singleton '${name}' destroyed (last connection unmounted)`)
-        } else {
-          liveLog('lifecycle', componentId, `🔗 Singleton '${name}' — connection removed (${singleton.connections.size} remaining)`)
-        }
-        return
-      }
+    if (ws) {
+      const connId = ws.data?.connectionId
+      ws.data?.components?.delete(componentId)
+      if (this.removeSingletonConnection(componentId, connId, 'unmount')) return
+    } else {
+      if (this.removeSingletonConnection(componentId, undefined, 'unmount')) return
     }
 
     // Non-singleton: normal unmount
@@ -905,28 +926,8 @@ export class ComponentRegistry {
         }
       }
 
-      // Check if this is a singleton
-      let isSingleton = false
-      for (const [name, singleton] of this.singletons) {
-        if (singleton.instance.id === componentId) {
-          // Remove this connection from the singleton
-          if (connId) singleton.connections.delete(connId)
-
-          if (singleton.connections.size === 0) {
-            // Last connection — destroy singleton
-            this.cleanupComponent(componentId)
-            this.singletons.delete(name)
-            liveLog('lifecycle', componentId, `🔗 Singleton '${name}' destroyed (no more connections)`)
-          } else {
-            liveLog('lifecycle', componentId, `🔗 Singleton '${name}' — connection left (${singleton.connections.size} remaining)`)
-          }
-
-          isSingleton = true
-          break
-        }
-      }
-
-      if (!isSingleton) {
+      // Singleton-aware cleanup via shared helper
+      if (!this.removeSingletonConnection(componentId, connId || undefined, 'disconnect')) {
         this.cleanupComponent(componentId)
       }
     }

core/types/types.ts

@@ -5,6 +5,9 @@ import { liveRoomManager } from '@core/server/live/LiveRoomManager'
 import { ANONYMOUS_CONTEXT } from '@core/server/live/auth/LiveAuthContext'
 import { liveLog, liveWarn } from '@core/server/live/LiveLogger'
 
+/** @internal Symbol key for singleton emit override — prevents accidental access from userland code */
+export const EMIT_OVERRIDE_KEY = Symbol.for('fluxstack:emitOverride')
+
 // ===== Debug Instrumentation (injectable to avoid client-side import) =====
 // The real debugger is injected by ComponentRegistry at server startup.
 // This avoids importing server-only LiveDebugger.ts from this shared types file.
@@ -340,8 +343,11 @@ export abstract class LiveComponent<TState = ComponentState, TPrivate extends Re
   // Cached room handles
   private roomHandles: Map<string, ServerRoomHandle> = new Map()
 
-  // Internal: emit override for singleton broadcasting (injected by ComponentRegistry)
-  private _emitOverride: ((type: string, payload: any) => void) | null = null
+  // Guard against infinite recursion in onStateChange
+  private _inStateChange = false
+
+  // Guard: tracks whether we're inside onStateChange to prevent infinite recursion
+  // Internal: emit override for singleton broadcasting is stored via EMIT_OVERRIDE_KEY symbol
 
   constructor(initialState: Partial<TState>, ws: FluxStackWebSocket, options?: { room?: string; userId?: string }) {
     this.id = this.generateId()
@@ -404,8 +410,11 @@ export abstract class LiveComponent<TState = ComponentState, TPrivate extends Re
           const changes = { [prop]: value } as Partial<TState>
           // Delta sync - send only the changed property
           self.emit('STATE_DELTA', { delta: changes })
-          // Lifecycle hook: onStateChange
-          try { self.onStateChange(changes) } catch {}
+          // Lifecycle hook: onStateChange (with recursion guard)
+          if (!self._inStateChange) {
+            self._inStateChange = true
+            try { self.onStateChange(changes) } catch {} finally { self._inStateChange = false }
+          }
           // Debug: track proxy mutation
           _liveDebugger?.trackStateChange(
             self.id,
@@ -633,10 +642,8 @@ export abstract class LiveComponent<TState = ComponentState, TPrivate extends Re
   // 🔗 Singleton Support (internal)
   // ========================================
 
-  /** @internal Used by ComponentRegistry to override emit for singleton broadcasting */
-  public _setEmitOverride(fn: ((type: string, payload: any) => void) | null): void {
-    this._emitOverride = fn
-  }
+  /** @internal Used by ComponentRegistry to override emit for singleton broadcasting (Symbol-keyed) */
+  public [EMIT_OVERRIDE_KEY]: ((type: string, payload: any) => void) | null = null
 
   // ========================================
   // 🔄 Lifecycle Hooks
@@ -754,8 +761,11 @@ export abstract class LiveComponent<TState = ComponentState, TPrivate extends Re
     Object.assign(this._state as object, newUpdates)
     // Delta sync - send only the changed properties
     this.emit('STATE_DELTA', { delta: newUpdates })
-    // Lifecycle hook: onStateChange
-    try { this.onStateChange(newUpdates) } catch {}
+    // Lifecycle hook: onStateChange (with recursion guard)
+    if (!this._inStateChange) {
+      this._inStateChange = true
+      try { this.onStateChange(newUpdates) } catch {} finally { this._inStateChange = false }
+    }
     // Debug: track state change
     _liveDebugger?.trackStateChange(
       this.id,
@@ -799,8 +809,8 @@ export abstract class LiveComponent<TState = ComponentState, TPrivate extends Re
     '$private', '_privateState',
     // HMR persistence
     '$persistent',
-    // Singleton internals
-    '_setEmitOverride', '_emitOverride',
+    // Singleton internals (Symbol-keyed, but block string equivalents too)
+    '_inStateChange',
     // Room internals
     '$room', '$rooms', 'subscribeToRoom', 'unsubscribeFromRoom',
     'emitRoomEvent', 'onRoomEvent', 'emitRoomEventWithState',
@@ -882,9 +892,10 @@ export abstract class LiveComponent<TState = ComponentState, TPrivate extends Re
 
   // Send message to client (or all clients for singletons)
   protected emit(type: string, payload: any) {
-    // Singleton override: broadcast to all connections
-    if (this._emitOverride) {
-      this._emitOverride(type, payload)
+    // Singleton override: broadcast to all connections (via Symbol key)
+    const override = this[EMIT_OVERRIDE_KEY]
+    if (override) {
+      override(type, payload)
       return
     }