Skip to content

Commit a33e931

Browse files
committed
Remove unnecessary React effects that dual-state or lag a paint
Derive permission queue depth from queuedApprovals, adjust cursor and Work-panel expansion during render, clear path suggestions at exit sites, sync refs without effects, and cancel stale sent-history loads. CL-5351 slice B.
1 parent 2921e45 commit a33e931

6 files changed

Lines changed: 40 additions & 33 deletions

File tree

src/tui/app.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -517,13 +517,12 @@ export function App({
517517

518518
const { goalActive, goalPhase, showAcceptance, workPrimary } = resolveGoalChrome({ goalSnapshot });
519519
// Default-expand Work when entering implementing; Ctrl+T can still collapse.
520+
// Adjust during render so the panel opens in the same paint as the phase flip.
520521
const wasWorkPrimary = useRef(false);
521-
useEffect(() => {
522-
if (workPrimary && !wasWorkPrimary.current) {
523-
setTasksExpanded(true);
524-
}
525-
wasWorkPrimary.current = workPrimary;
526-
}, [workPrimary]);
522+
if (workPrimary && !wasWorkPrimary.current) {
523+
setTasksExpanded(true);
524+
}
525+
wasWorkPrimary.current = workPrimary;
527526
const workExpanded = tasksExpanded;
528527
const goalChromeRows = goalChromeRowCount({
529528
goalActive,

src/tui/components/chat-input.tsx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Box, Text, useInput, usePaste } from "ink";
2-
import { useState, useMemo, useEffect, useRef } from "react";
2+
import { useState, useMemo, useRef } from "react";
33
import type { ReactNode } from "react";
44
import { getCommand, listCommands } from "../commands/registry.js";
55
import type { CommandContext, CommandResult, SubcommandDefinition } from "../commands/registry.js";
@@ -368,10 +368,15 @@ export function ChatInput({
368368
const selfSetValue = useRef<string | null>(null);
369369

370370
// Reset the cursor to the end only when value changes from the OUTSIDE.
371-
useEffect(() => {
372-
if (value === selfSetValue.current) return;
373-
setCursor(value.length);
374-
}, [value]);
371+
// Adjust during render (not an effect) so the caret lands in the same paint
372+
// as the external value update — no extra commit for the cursor alone.
373+
const [prevValue, setPrevValue] = useState(value);
374+
if (value !== prevValue) {
375+
setPrevValue(value);
376+
if (value !== selfSetValue.current) {
377+
setCursor(value.length);
378+
}
379+
}
375380

376381
const atMention = useAtSuggestions(cwd);
377382

src/tui/components/plugins-manager.tsx

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Box, Text, useInput } from "ink";
2-
import { useState, useRef, useEffect } from "react";
2+
import { useState, useRef } from "react";
33
import type { ReactNode } from "react";
44
import { color } from "../theme.js";
55
import { listPathSuggestions } from "./at-mention/index.js";
@@ -86,14 +86,15 @@ export function PluginsManager({ admin, onClose, cwd }: PluginsManagerProps): Re
8686
const isEnabled = (id: string): boolean => config[id]?.enabled === true;
8787
const isConsented = (id: string): boolean => config[id]?.consented === true;
8888

89-
useEffect(() => {
90-
if (addingPath === null) {
91-
pathGeneration.current++;
92-
lastPathPrefix.current = null;
93-
setPathSuggestions([]);
94-
setPathSelectedIdx(0);
95-
}
96-
}, [addingPath]);
89+
// Clear path-suggestion state when leaving add-by-path (call at every exit site
90+
// instead of watching addingPath with an effect).
91+
const clearAddingPath = (): void => {
92+
pathGeneration.current++;
93+
lastPathPrefix.current = null;
94+
setPathSuggestions([]);
95+
setPathSelectedIdx(0);
96+
setAddingPath(null);
97+
};
9798

9899
const fetchPathSuggestions = (prefix: string, gen: number) => {
99100
void listPathSuggestions(prefix, cwd).then((results) => {
@@ -142,7 +143,7 @@ export function PluginsManager({ admin, onClose, cwd }: PluginsManagerProps): Re
142143
void Promise.resolve(admin.addPath(path)).then(
143144
(result) => {
144145
setAddStatus({ ok: result.ok, message: result.message });
145-
if (result.ok) { setAddingPath(null); setVersion((v) => v + 1); }
146+
if (result.ok) { clearAddingPath(); setVersion((v) => v + 1); }
146147
},
147148
(err: unknown) => setAddStatus({ ok: false, message: err instanceof Error ? err.message : String(err) }),
148149
);
@@ -160,7 +161,7 @@ export function PluginsManager({ admin, onClose, cwd }: PluginsManagerProps): Re
160161

161162
if (addingPath !== null) {
162163
if (key.escape) {
163-
setAddingPath(null);
164+
clearAddingPath();
164165
setAddStatus(null);
165166
return;
166167
}

src/tui/components/session-resume-picker.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Box, Text, useApp, useInput } from "ink";
22
import type { ReactNode } from "react";
3-
import { useEffect, useMemo, useRef, useState } from "react";
3+
import { useMemo, useRef, useState } from "react";
44

55
import type { SessionSummary } from "../../session/index.js";
66
import { formatRelativeTime } from "../format-relative-time.js";
@@ -21,10 +21,9 @@ function formatLabel(session: SessionSummary): string {
2121
export function SessionResumePicker({ sessions, onSelect, onCancel }: SessionResumePickerProps): ReactNode {
2222
const { exit } = useApp();
2323
const [cursor, setCursor] = useState(0);
24-
const cursorRef = useRef(0);
25-
useEffect(() => {
26-
cursorRef.current = cursor;
27-
}, [cursor]);
24+
// Keep the latest cursor available to useInput without an effect round-trip.
25+
const cursorRef = useRef(cursor);
26+
cursorRef.current = cursor;
2827
const rows = useMemo(() => sessions.map((s) => ({ session: s, label: formatLabel(s) })), [sessions]);
2928
const clamped = rows.length > 0 ? Math.min(cursor, rows.length - 1) : 0;
3029

src/tui/hooks/use-gates.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,10 @@ export function useGates({
138138
activationBlocked = false,
139139
}: UseGatesArgs): GateController {
140140
const [activeApproval, setActiveApproval] = useState<ActiveApproval | null>(null);
141-
const [permissionQueueDepth, setPermissionQueueDepth] = useState(0);
142141
const [queuedApprovals, setQueuedApprovals] = useState<readonly QueuedApprovalSummary[]>([]);
143142
const queue = useRef<GateQueueEntry[]>([]);
143+
// Depth is the length of the permission summary list — one source of truth.
144+
const permissionQueueDepth = queuedApprovals.length;
144145

145146
function syncQueuedApprovals(): void {
146147
setQueuedApprovals(
@@ -194,7 +195,6 @@ export function useGates({
194195
clearEntryTimer(entry);
195196
detachEntryAbort(entry);
196197
if (entry.kind === "permission") {
197-
setPermissionQueueDepth((depth) => Math.max(0, depth - 1));
198198
syncQueuedApprovals();
199199
}
200200
setGatePendingRef.current(false);
@@ -245,7 +245,6 @@ export function useGates({
245245
function enqueue(entry: GateQueueEntry): void {
246246
queue.current.push(entry);
247247
if (entry.kind === "permission") {
248-
setPermissionQueueDepth((depth) => depth + 1);
249248
syncQueuedApprovals();
250249
}
251250
setGatePendingRef.current(true);
@@ -256,7 +255,6 @@ export function useGates({
256255
const remaining = queue.current.splice(0);
257256
activeId.current = null;
258257
setActiveApproval(null);
259-
setPermissionQueueDepth(0);
260258
setQueuedApprovals([]);
261259
for (const entry of remaining) {
262260
clearEntryTimer(entry);

src/tui/hooks/use-message-pipeline.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,13 +244,18 @@ export function useMessagePipeline({
244244
};
245245
const startNewSession = () => startNewSessionRef.current();
246246

247-
// Send the initial task once the App (and its gate listeners) is mounted, so
248-
// the run is driven through the same abortable path as interactive sends.
247+
// Hydrate sent-message history for the active session. Cancel stale loads so a
248+
// session switch or unmount cannot write history from a prior session id.
249249
useEffect(() => {
250250
if (getSessionId === undefined) return;
251+
let cancelled = false;
251252
void loadSentMessages(cwd, getSessionId()).then((sent) => {
253+
if (cancelled) return;
252254
setSentHistoryBrowse(createSentHistoryBrowse(sent));
253255
});
256+
return () => {
257+
cancelled = true;
258+
};
254259
}, [cwd, getSessionId]);
255260

256261
useEffect(() => {

0 commit comments

Comments
 (0)