Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

Commit cfdf89b

Browse files
authored
fix(ui): clear react-doctor errors in artifact comments
Three blocking react-doctor findings in the PR's own files: - ActivityPanel and TaskCommentsList adjusted state inside an effect on a prop/store change (no-adjust-state-on-prop-change); moved both to render-time adjustment with the existing prev-value refs, so no stale tab/filter commits before the switch. - The comment-focus pulse created a setTimeout in an effect without returning cleanup (effect-needs-cleanup); the timer now lives in its own effect keyed on pulseThreadId and is cleared on the next pulse or unmount, dropping the stray ref. Behavior unchanged; ActivityPanel and TaskCommentsList suites pass. Generated-By: PostHog Code Task-Id: 0331ac58-0a1c-4b4e-b884-2ff7d8e71986
1 parent b131850 commit cfdf89b

2 files changed

Lines changed: 20 additions & 22 deletions

File tree

‎packages/ui/src/features/canvas/components/ActivityPanel.tsx‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,16 +173,17 @@ function ActivityConversation({
173173
// Tracks the task too: this panel is reused across tasks without remounting,
174174
// so a nonce seen for the previous task says nothing about this one.
175175
const seenFocus = useRef({ taskId, nonce: commentFocus?.nonce });
176-
useEffect(() => {
177-
if (seenFocus.current.taskId !== taskId) {
178-
seenFocus.current = { taskId, nonce: commentFocus?.nonce };
179-
return;
180-
}
181-
if (!commentFocus || commentFocus.nonce === seenFocus.current.nonce) return;
176+
// Adjust during render rather than in an effect, so the panel never commits a
177+
// stale tab before switching. A new task resets the baseline (an old focus
178+
// must not hijack it); a fresh focus nonce for this task brings the Comments
179+
// tab with the pick.
180+
if (seenFocus.current.taskId !== taskId) {
181+
seenFocus.current = { taskId, nonce: commentFocus?.nonce };
182+
} else if (commentFocus && commentFocus.nonce !== seenFocus.current.nonce) {
182183
seenFocus.current = { taskId, nonce: commentFocus.nonce };
183184
// Not handleTabChange: a programmatic switch isn't a user tab change.
184185
setTab("comments");
185-
}, [commentFocus, taskId]);
186+
}
186187

187188
const scrollRef = useRef<HTMLDivElement>(null);
188189
// biome-ignore lint/correctness/useExhaustiveDependencies: scroll when rendered thread content changes

‎packages/ui/src/features/canvas/components/TaskCommentsList.tsx‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,11 @@ export function TaskCommentsList({
326326
}, [sources, prUrls]);
327327

328328
// A source that can't exist any more (its artifact left the task) can't stay
329-
// selected, or the pane reads as empty with no hint why.
330-
useEffect(() => {
331-
if (sourceFilter !== ALL_SOURCES && !knownSourceKeys.has(sourceFilter)) {
332-
setSourceFilter(ALL_SOURCES);
333-
}
334-
}, [sourceFilter, knownSourceKeys]);
329+
// selected, or the pane reads as empty with no hint why. Adjust during render
330+
// rather than in an effect, so the stale filter never commits first.
331+
if (sourceFilter !== ALL_SOURCES && !knownSourceKeys.has(sourceFilter)) {
332+
setSourceFilter(ALL_SOURCES);
333+
}
335334

336335
// Follow the artifact on screen until the reader picks a source themselves;
337336
// after that the filter is theirs, not the pane's.
@@ -358,7 +357,6 @@ export function TaskCommentsList({
358357
// filter is hiding it. Each request is honoured once, by nonce: resolving the
359358
// focused thread later must not drag the filters along with it.
360359
const focusedThreadId = focus?.threadId ?? null;
361-
const pulseTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
362360
const handledNonceRef = useRef<number | null>(null);
363361
useEffect(() => {
364362
if (!focus || handledNonceRef.current === focus.nonce) return;
@@ -373,8 +371,6 @@ export function TaskCommentsList({
373371
: ALL_SOURCES,
374372
);
375373
setPulseThreadId(focus.threadId);
376-
if (pulseTimerRef.current) clearTimeout(pulseTimerRef.current);
377-
pulseTimerRef.current = setTimeout(() => setPulseThreadId(null), PULSE_MS);
378374
requestAnimationFrame(() => {
379375
document
380376
.querySelector(
@@ -383,12 +379,13 @@ export function TaskCommentsList({
383379
?.scrollIntoView({ behavior: "smooth", block: "nearest" });
384380
});
385381
}, [focus, threads]);
386-
useEffect(
387-
() => () => {
388-
if (pulseTimerRef.current) clearTimeout(pulseTimerRef.current);
389-
},
390-
[],
391-
);
382+
// The pulse fades on its own; owning the timer in its own effect keeps it
383+
// cleaned up on the next pulse or on unmount, without a stray ref.
384+
useEffect(() => {
385+
if (!pulseThreadId) return;
386+
const timer = setTimeout(() => setPulseThreadId(null), PULSE_MS);
387+
return () => clearTimeout(timer);
388+
}, [pulseThreadId]);
392389

393390
const openThread = (thread: TaskCommentThread) => {
394391
const origin = thread.origin;

0 commit comments

Comments
 (0)