Conversation
| // Null out removed layers in this store. Do NOT call layerAtoms.remove() — | ||
| // the atomFamily cache is global (shared across all compositor stores), and | ||
| // removing an entry would invalidate other stores that share the same layer ID. | ||
| for (const prevId of prevIds) { | ||
| if (!newIdSet.has(prevId)) { | ||
| store.set(layerAtoms(prevId), null); | ||
| } | ||
| } |
There was a problem hiding this comment.
🚩 Global atomFamily cache grows unboundedly for compositor overlay IDs
The layerAtoms, textOverlayAtoms, and imageOverlayAtoms atomFamily caches are global (shared across all compositor stores). The comment at ui/src/hooks/compositorAtoms.ts:174-176 explains that remove() isn't called because it would invalidate other stores sharing the same layer ID. However, text/image overlays use crypto.randomUUID() for IDs (ui/src/hooks/compositorOverlays.ts:338,409), meaning each add/remove cycle creates a permanent atomFamily entry. Additionally, selectedLayerKindAtom (ui/src/hooks/compositorAtoms.ts:85-92) creates phantom entries when checking if an ID belongs to a different layer type (e.g., calling get(layerAtoms(textOverlayId)) creates a null entry in layerAtoms for a text overlay ID). For long-running sessions with frequent overlay additions/removals, this accumulates stale entries. Consider periodic cleanup when a compositor unmounts, or a WeakRef-based approach.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
PR replaces coarse-grained Zustand subscriptions with per-entity Jotai atoms in the three highest-frequency state paths: compositor layers, session node states, and node parameters.