| # CreateEffect Simplification Implementation Spec |
|
|
| Reduce reactive misuse across `packages/app`. |
|
|
| --- |
|
|
| ## Context |
|
|
| This work targets `packages/app/src`, which currently has 101 `createEffect` calls across 37 files. |
|
|
| The biggest clusters are `pages/session.tsx` (19), `pages/layout.tsx` (13), `pages/session/file-tabs.tsx` (6), and several context providers that mirror one store into another. |
|
|
| Key issues from the audit: |
|
|
| - Derived state is being written through effects instead of computed directly |
| - Session and file resets are handled by watch-and-clear effects instead of keyed state boundaries |
| - User-driven actions are hidden inside reactive effects |
| - Context layers mirror and hydrate child stores with multiple sync effects |
| - Several areas repeat the same imperative trigger pattern in multiple effects |
|
|
| Keep the implementation focused on removing unnecessary effects, not on broad UI redesign. |
|
|
| ## Goals |
|
|
| - Cut high-churn `createEffect` usage in the hottest files first |
| - Replace effect-driven derived state with reactive derivation |
| - Replace reset-on-key effects with keyed ownership boundaries |
| - Move event-driven work to direct actions and write paths |
| - Remove mirrored store hydration where a single source of truth can exist |
| - Leave necessary external sync effects in place, but make them narrower and clearer |
|
|
| ## Non-Goals |
|
|
| - Do not rewrite unrelated component structure just to reduce the count |
| - Do not change product behavior, navigation flow, or persisted data shape unless required for a cleaner write boundary |
| - Do not remove effects that bridge to DOM, editors, polling, or external APIs unless there is a clearly safer equivalent |
| - Do not attempt a repo-wide cleanup outside `packages/app` |
|
|
| ## Effect Taxonomy And Replacement Rules |
|
|
| Use these rules during implementation. |
|
|
| ### Prefer `createMemo` |
|
|
| Use `createMemo` when the target value is pure derived state from other signals or stores. |
|
|
| Do this when an effect only reads reactive inputs and writes another reactive value that could be computed instead. |
|
|
| Apply this to: |
|
|
| - `packages/app/src/pages/session.tsx:141` |
| - `packages/app/src/pages/layout.tsx:557` |
| - `packages/app/src/components/terminal.tsx:261` |
| - `packages/app/src/components/session/session-header.tsx:309` |
|
|
| Rules: |
|
|
| - If no external system is touched, do not use `createEffect` |
| - Derive once, then read the memo where needed |
| - If normalization is required, prefer normalizing at the write boundary before falling back to a memo |
|
|
| ### Prefer Keyed Remounts |
|
|
| Use keyed remounts when local UI state should reset because an identity changed. |
|
|
| Do this with `sessionKey`, `scope()`, or another stable identity instead of watching the key and manually clearing signals. |
|
|
| Apply this to: |
|
|
| - `packages/app/src/pages/session.tsx:325` |
| - `packages/app/src/pages/session.tsx:336` |
| - `packages/app/src/pages/session.tsx:477` |
| - `packages/app/src/pages/session.tsx:869` |
| - `packages/app/src/pages/session.tsx:963` |
| - `packages/app/src/pages/session/message-timeline.tsx:149` |
| - `packages/app/src/context/file.tsx:100` |
|
|
| Rules: |
|
|
| - If the desired behavior is "new identity, fresh local state," key the owner subtree |
| - Keep state local to the keyed boundary so teardown and recreation handle the reset naturally |
|
|
| ### Prefer Event Handlers And Actions |
|
|
| Use direct handlers, store actions, and async command functions when work happens because a user clicked, selected, reloaded, or navigated. |
|
|
| Do this when an effect is just watching for a flag change, command token, or event-bus signal to trigger imperative logic. |
|
|
| Apply this to: |
|
|
| - `packages/app/src/pages/layout.tsx:484` |
| - `packages/app/src/pages/layout.tsx:652` |
| - `packages/app/src/pages/layout.tsx:776` |
| - `packages/app/src/pages/layout.tsx:1489` |
| - `packages/app/src/pages/layout.tsx:1519` |
| - `packages/app/src/components/file-tree.tsx:328` |
| - `packages/app/src/pages/session/terminal-panel.tsx:55` |
| - `packages/app/src/context/global-sync.tsx:148` |
| - Duplicated trigger sets in: |
| - `packages/app/src/pages/session/review-tab.tsx:122` |
| - `packages/app/src/pages/session/review-tab.tsx:130` |
| - `packages/app/src/pages/session/review-tab.tsx:138` |
| - `packages/app/src/pages/session/file-tabs.tsx:367` |
| - `packages/app/src/pages/session/file-tabs.tsx:378` |
| - `packages/app/src/pages/session/file-tabs.tsx:389` |
| - `packages/app/src/pages/session/use-session-hash-scroll.ts:144` |
| - `packages/app/src/pages/session/use-session-hash-scroll.ts:149` |
| - `packages/app/src/pages/session/use-session-hash-scroll.ts:167` |
|
|
| Rules: |
|
|
| - If the trigger is user intent, call the action at the source of that intent |
| - If the same imperative work is triggered from multiple places, extract one function and call it directly |
|
|
| ### Prefer `onMount` And `onCleanup` |
|
|
| Use `onMount` and `onCleanup` for lifecycle-only setup and teardown. |
|
|
| This is the right fit for subscriptions, one-time wiring, timers, and imperative integration that should not rerun for ordinary reactive changes. |
|
|
| Use this when: |
|
|
| - Setup should happen once per owner lifecycle |
| - Cleanup should always pair with teardown |
| - The work is not conceptually derived state |
|
|
| ### Keep `createEffect` When It Is A Real Bridge |
|
|
| Keep `createEffect` when it synchronizes reactive data to an external imperative sink. |
|
|
| Examples that should remain, though they may be narrowed or split: |
|
|
| - DOM/editor sync in `packages/app/src/components/prompt-input.tsx:690` |
| - Scroll sync in `packages/app/src/pages/session.tsx:685` |
| - Scroll/hash sync in `packages/app/src/pages/session/use-session-hash-scroll.ts:149` |
| - External sync in: |
| - `packages/app/src/context/language.tsx:207` |
| - `packages/app/src/context/settings.tsx:110` |
| - `packages/app/src/context/sdk.tsx:26` |
| - Polling in: |
| - `packages/app/src/components/status-popover.tsx:59` |
| - `packages/app/src/components/dialog-select-server.tsx:273` |
|
|
| Rules: |
|
|
| - Keep the effect single-purpose |
| - Make dependencies explicit and narrow |
| - Avoid writing back into the same reactive graph unless absolutely required |
|
|
| ## Implementation Plan |
|
|
| ### Phase 0: Classification Pass |
|
|
| Before changing code, tag each targeted effect as one of: derive, reset, event, lifecycle, or external bridge. |
|
|
| Acceptance criteria: |
|
|
| - Every targeted effect in this spec is tagged with a replacement strategy before refactoring starts |
| - Shared helpers to be introduced are identified up front to avoid repeating patterns |
|
|
| ### Phase 1: Derived-State Cleanup |
|
|
| Tackle highest-value, lowest-risk derived-state cleanup first. |
|
|
| Priority items: |
|
|
| - Normalize tabs at write boundaries and remove `packages/app/src/pages/session.tsx:141` |
| - Stop syncing `workspaceOrder` in `packages/app/src/pages/layout.tsx:557` |
| - Make prompt slash filtering reactive so `packages/app/src/components/prompt-input.tsx:652` can be removed |
| - Replace other obvious derived-state effects in terminal and session header |
|
|
| Acceptance criteria: |
|
|
| - No behavior change in tab ordering, prompt filtering, terminal display, or header state |
| - Targeted derived-state effects are deleted, not just moved |
|
|
| ### Phase 2: Keyed Reset Cleanup |
|
|
| Replace reset-on-key effects with keyed ownership boundaries. |
|
|
| Priority items: |
|
|
| - Key session-scoped UI and state by `sessionKey` |
| - Key file-scoped state by `scope()` |
| - Remove manual clear-and-reseed effects in session and file context |
|
|
| Acceptance criteria: |
|
|
| - Switching session or file scope recreates the intended local state cleanly |
| - No stale state leaks across session or scope changes |
| - Target reset effects are deleted |
|
|
| ### Phase 3: Event-Driven Work Extraction |
|
|
| Move event-driven work out of reactive effects. |
|
|
| Priority items: |
|
|
| - Replace `globalStore.reload` effect dispatching with direct calls |
| - Split mixed-responsibility effect in `packages/app/src/pages/layout.tsx:1489` |
| - Collapse duplicated imperative trigger triplets into single functions |
| - Move file-tree and terminal-panel imperative work to explicit handlers |
|
|
| Acceptance criteria: |
|
|
| - User-triggered behavior still fires exactly once per intended action |
| - No effect remains whose only job is to notice a command-like state and trigger an imperative function |
|
|
| ### Phase 4: Context Ownership Cleanup |
|
|
| Remove mirrored child-store hydration patterns. |
|
|
| Priority items: |
|
|
| - Remove child-store hydration mirrors in `packages/app/src/context/global-sync/child-store.ts:184`, `:190`, `:193` |
| - Simplify mirror logic in `packages/app/src/context/global-sync.tsx:130`, `:138` |
| - Revisit `packages/app/src/context/layout.tsx:424` if it still mirrors instead of deriving |
|
|
| Acceptance criteria: |
|
|
| - There is one clear source of truth for each synced value |
| - Child stores no longer need effect-based hydration to stay consistent |
| - Initialization and updates both work without manual mirror effects |
|
|
| ### Phase 5: Cleanup And Keeper Review |
|
|
| Clean up remaining targeted hotspots and narrow the effects that should stay. |
|
|
| Acceptance criteria: |
|
|
| - Remaining `createEffect` calls in touched files are all true bridges or clearly justified lifecycle sync |
| - Mixed-responsibility effects are split into smaller units where still needed |
|
|
| ## Detailed Work Items By Area |
|
|
| ### 1. Normalize Tab State |
|
|
| Files: |
|
|
| - `packages/app/src/pages/session.tsx:141` |
|
|
| Work: |
|
|
| - Move tab normalization into the functions that create, load, or update tab state |
| - Make readers consume already-normalized tab data |
| - Remove the effect that rewrites derived tab state after the fact |
|
|
| Rationale: |
|
|
| - Tabs should become valid when written, not be repaired later |
| - This removes a feedback loop and makes state easier to trust |
|
|
| Acceptance criteria: |
|
|
| - The effect at `packages/app/src/pages/session.tsx:141` is removed |
| - Newly created and restored tabs are normalized before they enter local state |
| - Tab rendering still matches current behavior for valid and edge-case inputs |
|
|
| ### 2. Key Session-Owned State |
|
|
| Files: |
|
|
| - `packages/app/src/pages/session.tsx:325` |
| - `packages/app/src/pages/session.tsx:336` |
| - `packages/app/src/pages/session.tsx:477` |
| - `packages/app/src/pages/session.tsx:869` |
| - `packages/app/src/pages/session.tsx:963` |
| - `packages/app/src/pages/session/message-timeline.tsx:149` |
|
|
| Work: |
|
|
| - Identify state that should reset when `sessionKey` changes |
| - Move that state under a keyed subtree or keyed owner boundary |
| - Remove effects that watch `sessionKey` just to clear local state, refs, or temporary UI flags |
|
|
| Rationale: |
|
|
| - Session identity already defines the lifetime of this UI state |
| - Keyed ownership makes reset behavior automatic and easier to reason about |
|
|
| Acceptance criteria: |
|
|
| - The targeted reset effects are removed |
| - Changing sessions resets only the intended session-local state |
| - Scroll and editor state that should persist are not accidentally reset |
|
|
| ### 3. Derive Workspace Order |
|
|
| Files: |
|
|
| - `packages/app/src/pages/layout.tsx:557` |
|
|
| Work: |
|
|
| - Stop writing `workspaceOrder` from live workspace data in an effect |
| - Represent user overrides separately from live workspace data |
| - Compute effective order from current data plus overrides with a memo or pure helper |
|
|
| Rationale: |
|
|
| - Persisted user intent and live source data should not mirror each other through an effect |
| - A computed effective order avoids drift and racey resync behavior |
|
|
| Acceptance criteria: |
|
|
| - The effect at `packages/app/src/pages/layout.tsx:557` is removed |
| - Workspace order updates correctly when workspaces appear, disappear, or are reordered by the user |
| - User overrides persist without requiring a sync-back effect |
|
|
| ### 4. Remove Child-Store Mirrors |
|
|
| Files: |
|
|
| - `packages/app/src/context/global-sync.tsx:130` |
| - `packages/app/src/context/global-sync.tsx:138` |
| - `packages/app/src/context/global-sync.tsx:148` |
| - `packages/app/src/context/global-sync/child-store.ts:184` |
| - `packages/app/src/context/global-sync/child-store.ts:190` |
| - `packages/app/src/context/global-sync/child-store.ts:193` |
| - `packages/app/src/context/layout.tsx:424` |
|
|
| Work: |
|
|
| - Trace the actual ownership of global and child store values |
| - Replace hydration and mirror effects with explicit initialization and direct updates |
| - Remove the `globalStore.reload` event-bus pattern and call the needed reload paths directly |
|
|
| Rationale: |
|
|
| - Mirrors make it hard to tell which state is authoritative |
| - Event-bus style state toggles hide control flow and create accidental reruns |
|
|
| Acceptance criteria: |
|
|
| - Child store hydration no longer depends on effect-based copying |
| - Reload work can be followed from the event source to the handler without a reactive relay |
| - State remains correct on first load, child creation, and subsequent updates |
|
|
| ### 5. Key File-Scoped State |
|
|
| Files: |
|
|
| - `packages/app/src/context/file.tsx:100` |
|
|
| Work: |
|
|
| - Move file-scoped local state under a boundary keyed by `scope()` |
| - Remove any effect that watches `scope()` only to reset file-local state |
|
|
| Rationale: |
|
|
| - File scope changes are identity changes |
| - Keyed ownership gives a cleaner reset than manual clear logic |
|
|
| Acceptance criteria: |
|
|
| - The effect at `packages/app/src/context/file.tsx:100` is removed |
| - Switching scopes resets only scope-local state |
| - No previous-scope data appears after a scope change |
|
|
| ### 6. Split Layout Side Effects |
|
|
| Files: |
|
|
| - `packages/app/src/pages/layout.tsx:1489` |
| - Related event-driven effects near `packages/app/src/pages/layout.tsx:484`, `:652`, `:776`, `:1519` |
|
|
| Work: |
|
|
| - Break the mixed-responsibility effect at `:1489` into direct actions and smaller bridge effects only where required |
| - Move user-triggered branches into the actual command or handler that causes them |
| - Remove any branch that only exists because one effect is handling unrelated concerns |
|
|
| Rationale: |
|
|
| - Mixed effects hide cause and make reruns hard to predict |
| - Smaller units reduce accidental coupling and make future cleanup safer |
|
|
| Acceptance criteria: |
|
|
| - The effect at `packages/app/src/pages/layout.tsx:1489` no longer mixes unrelated responsibilities |
| - Event-driven branches execute from direct handlers |
| - Remaining effects in this area each have one clear external sync purpose |
|
|
| ### 7. Remove Duplicate Triggers |
|
|
| Files: |
|
|
| - `packages/app/src/pages/session/review-tab.tsx:122` |
| - `packages/app/src/pages/session/review-tab.tsx:130` |
| - `packages/app/src/pages/session/review-tab.tsx:138` |
| - `packages/app/src/pages/session/file-tabs.tsx:367` |
| - `packages/app/src/pages/session/file-tabs.tsx:378` |
| - `packages/app/src/pages/session/file-tabs.tsx:389` |
| - `packages/app/src/pages/session/use-session-hash-scroll.ts:144` |
| - `packages/app/src/pages/session/use-session-hash-scroll.ts:149` |
| - `packages/app/src/pages/session/use-session-hash-scroll.ts:167` |
|
|
| Work: |
|
|
| - Extract one explicit imperative function per behavior |
| - Call that function from each source event instead of replicating the same effect pattern multiple times |
| - Preserve the scroll-sync effect that is truly syncing with the DOM, but remove duplicate trigger scaffolding around it |
|
|
| Rationale: |
|
|
| - Duplicate triggers make it easy to miss a case or fire twice |
| - One named action is easier to test and reason about |
|
|
| Acceptance criteria: |
|
|
| - Repeated imperative effect triplets are collapsed into shared functions |
| - Scroll behavior still works, including hash-based navigation |
| - No duplicate firing is introduced |
|
|
| ### 8. Make Prompt Filtering Reactive |
|
|
| Files: |
|
|
| - `packages/app/src/components/prompt-input.tsx:652` |
| - Keep `packages/app/src/components/prompt-input.tsx:690` as needed |
|
|
| Work: |
|
|
| - Convert slash filtering into a pure reactive derivation from the current input and candidate command list |
| - Keep only the editor or DOM bridge effect if it is still needed for imperative syncing |
|
|
| Rationale: |
|
|
| - Filtering is classic derived state |
| - It should not need an effect if it can be computed from current inputs |
|
|
| Acceptance criteria: |
|
|
| - The effect at `packages/app/src/components/prompt-input.tsx:652` is removed |
| - Filtered slash-command results update correctly as the input changes |
| - The editor sync effect at `:690` still behaves correctly |
|
|
| ### 9. Clean Up Smaller Derived-State Cases |
|
|
| Files: |
|
|
| - `packages/app/src/components/terminal.tsx:261` |
| - `packages/app/src/components/session/session-header.tsx:309` |
|
|
| Work: |
|
|
| - Replace effect-written local state with memos or inline derivation |
| - Remove intermediate setters when the value can be computed directly |
|
|
| Rationale: |
|
|
| - These are low-risk wins that reinforce the same pattern |
| - They also help keep follow-up cleanup consistent |
|
|
| Acceptance criteria: |
|
|
| - Targeted effects are removed |
| - UI output remains unchanged under the same inputs |
|
|
| ## Verification And Regression Checks |
|
|
| Run focused checks after each phase, not only at the end. |
|
|
| ### Suggested Verification |
|
|
| - Switch between sessions rapidly and confirm local session UI resets only where intended |
| - Open, close, and reorder tabs and confirm order and normalization remain stable |
| - Change workspaces, reload workspace data, and verify effective ordering is correct |
| - Change file scope and confirm stale file state does not bleed across scopes |
| - Trigger layout actions that previously depended on effects and confirm they still fire once |
| - Use slash commands in the prompt and verify filtering updates as you type |
| - Test review tab, file tab, and hash-scroll flows for duplicate or missing triggers |
| - Verify global sync initialization, reload, and child-store creation paths |
|
|
| ### Regression Checks |
|
|
| - No accidental infinite reruns |
| - No double-firing network or command actions |
| - No lost cleanup for listeners, timers, or scroll handlers |
| - No preserved stale state after identity changes |
| - No removed effect that was actually bridging to DOM or an external API |
|
|
| If available, add or update tests around pure helpers introduced during this cleanup. |
|
|
| Favor tests for derived ordering, normalization, and action extraction, since those are easiest to lock down. |
|
|
| ## Definition Of Done |
|
|
| This work is done when all of the following are true: |
|
|
| - The highest-leverage targets in this spec are implemented |
| - Each removed effect has been replaced by a clearer pattern: memo, keyed boundary, direct action, or lifecycle hook |
| - The "should remain" effects still exist only where they serve a real external sync purpose |
| - Touched files have fewer mixed-responsibility effects and clearer ownership of state |
| - Manual verification covers session switching, file scope changes, workspace ordering, prompt filtering, and reload flows |
| - No behavior regressions are found in the targeted areas |
|
|
| A reduced raw `createEffect` count is helpful, but it is not the main success metric. |
|
|
| The main success metric is clearer ownership and fewer effect-driven state repairs. |
|
|
| ## Risks And Rollout Notes |
|
|
| Main risks: |
|
|
| - Keyed remounts can reset too much if state boundaries are drawn too high |
| - Store mirror removal can break initialization order if ownership is not mapped first |
| - Moving event work out of effects can accidentally skip triggers that were previously implicit |
|
|
| Rollout notes: |
|
|
| - Land in small phases, with each phase keeping the app behaviorally stable |
| - Prefer isolated PRs by phase or by file cluster, especially for context-store changes |
| - Review each remaining effect in touched files and leave it only if it clearly bridges to something external |
|
|
