refactor: stop routing Composer internal state through 12 DOM CustomEvents #463

Open
opened 2026-07-25 23:35:27 +02:00 by dries · 0 comments
Owner

Code-quality audit (P2). frontend/src/components/assistant/Composer.tsx — two effects inside the same component communicate by dispatching and listening for DOM CustomEvents on inputRef.current.

Dispatch side (:571-574, :605-625, :652-654, :688, :710-714, :724-733, :752):

el.dispatchEvent(new CustomEvent('oc-slash-update', { detail: { show: false, filter: '' } }));
el.dispatchEvent(new CustomEvent('oc-bash-mode', { detail: false }));
el.dispatchEvent(new CustomEvent('oc-clear-images'));

Listen side (:826-852):

el.addEventListener('oc-clear-images', handleClearImages);
el.addEventListener('oc-slash-update', handleSlashUpdate);
...

12 distinct event names: oc-agent-picker-open, oc-bash-mode, oc-clear-files, oc-clear-images, oc-help-open, oc-model-picker-open, oc-paste-images, oc-skill-picker-open, oc-slash-close, oc-slash-nav, oc-slash-select, oc-slash-update.

Why it matters

Internal state transitions are routed through the DOM. detail is any, so none of it is type-checked; a renamed event or a changed payload shape fails silently at runtime. It is also a major contributor to ComposerImpl's complexity score of 82 (ESLint limit 20), and shares a root cause with the ~25 hand-rolled prop-to-ref mirrors in the same file and the composerPropsEqual staleness bug (composerMemo.ts:56, complexity 58).

The reason these were introduced — stale closures in long-lived listener effects — no longer applies. React Compiler is enabled (vite.config.ts:100) and useEffectEvent is already used in the codebase (frontend/src/lib/useClickOutside.ts:8). One cleanup fixes three problems.

Suggested fix

  • Replace each CustomEvent pair with ordinary state/useReducer transitions in the component, wrapping listener callbacks in useEffectEvent where a stable identity is needed.
  • Delete the corresponding prop-to-ref mirrors that exist only to dodge stale closures.
  • Reassess composerPropsEqual once the mirrors are gone — it may become unnecessary under React Compiler.

Acceptance criteria

  • Zero oc-* CustomEvents dispatched or listened to within Composer.tsx.
  • ComposerImpl complexity below 40.
  • Existing composer tests and e2e specs pass unchanged (slash menu, bash mode, image paste, pickers).

Effort: L.

Code-quality audit (P2). `frontend/src/components/assistant/Composer.tsx` — two effects **inside the same component** communicate by dispatching and listening for DOM `CustomEvent`s on `inputRef.current`. Dispatch side (`:571-574`, `:605-625`, `:652-654`, `:688`, `:710-714`, `:724-733`, `:752`): ```js el.dispatchEvent(new CustomEvent('oc-slash-update', { detail: { show: false, filter: '' } })); el.dispatchEvent(new CustomEvent('oc-bash-mode', { detail: false })); el.dispatchEvent(new CustomEvent('oc-clear-images')); ``` Listen side (`:826-852`): ```js el.addEventListener('oc-clear-images', handleClearImages); el.addEventListener('oc-slash-update', handleSlashUpdate); ... ``` 12 distinct event names: `oc-agent-picker-open`, `oc-bash-mode`, `oc-clear-files`, `oc-clear-images`, `oc-help-open`, `oc-model-picker-open`, `oc-paste-images`, `oc-skill-picker-open`, `oc-slash-close`, `oc-slash-nav`, `oc-slash-select`, `oc-slash-update`. ## Why it matters Internal state transitions are routed through the DOM. `detail` is `any`, so none of it is type-checked; a renamed event or a changed payload shape fails silently at runtime. It is also a major contributor to `ComposerImpl`'s complexity score of **82** (ESLint limit 20), and shares a root cause with the ~25 hand-rolled prop-to-ref mirrors in the same file and the `composerPropsEqual` staleness bug (`composerMemo.ts:56`, complexity 58). The reason these were introduced — stale closures in long-lived listener effects — no longer applies. React Compiler is enabled (`vite.config.ts:100`) and `useEffectEvent` is already used in the codebase (`frontend/src/lib/useClickOutside.ts:8`). One cleanup fixes three problems. ## Suggested fix - Replace each CustomEvent pair with ordinary state/`useReducer` transitions in the component, wrapping listener callbacks in `useEffectEvent` where a stable identity is needed. - Delete the corresponding prop-to-ref mirrors that exist only to dodge stale closures. - Reassess `composerPropsEqual` once the mirrors are gone — it may become unnecessary under React Compiler. ## Acceptance criteria - [ ] Zero `oc-*` CustomEvents dispatched or listened to within `Composer.tsx`. - [ ] `ComposerImpl` complexity below 40. - [ ] Existing composer tests and e2e specs pass unchanged (slash menu, bash mode, image paste, pickers). Effort: L.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
dries/ocman#463
No description provided.