Buckets:
| # 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 | |
Xet Storage Details
- Size:
- 18.9 kB
- Xet hash:
- 98d64de0f354021d7f17f6a38cb8f7fe4c47ad497fa5e2c5899516bbb23da7bd
·
Xet efficiently stores files, intelligently splitting them into unique chunks and accelerating uploads and downloads. More info.