Skip to content

Commit 0390e88

Browse files
jacob314yashodipmore
jacob314
authored and
yashodipmore
committed
Defensive coding to reduce the risk of Maximum update depth errors (google-gemini#20940)
1 parent 481dfc2 commit 0390e88

File tree

4 files changed

+121
-24
lines changed

4 files changed

+121
-24
lines changed

‎packages/cli/src/ui/components/shared/MaxSizedBox.test.tsx‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,19 @@ import { OverflowProvider } from '../../contexts/OverflowContext.js';
99
import { MaxSizedBox } from './MaxSizedBox.js';
1010
import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js';
1111
import { Box, Text } from 'ink';
12-
import { describe, it, expect } from 'vitest';
12+
import { act } from 'react';
13+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
1314

1415
describe('<MaxSizedBox />', () => {
16+
beforeEach(() => {
17+
vi.useFakeTimers();
18+
});
19+
20+
afterEach(() => {
21+
vi.useRealTimers();
22+
vi.restoreAllMocks();
23+
});
24+
1525
it('renders children without truncation when they fit', async () => {
1626
const { lastFrame, waitUntilReady, unmount } = render(
1727
<OverflowProvider>
@@ -22,6 +32,9 @@ describe('<MaxSizedBox />', () => {
2232
</MaxSizedBox>
2333
</OverflowProvider>,
2434
);
35+
await act(async () => {
36+
vi.runAllTimers();
37+
});
2538
await waitUntilReady();
2639
expect(lastFrame()).toContain('Hello, World!');
2740
expect(lastFrame()).toMatchSnapshot();
@@ -40,6 +53,9 @@ describe('<MaxSizedBox />', () => {
4053
</MaxSizedBox>
4154
</OverflowProvider>,
4255
);
56+
await act(async () => {
57+
vi.runAllTimers();
58+
});
4359
await waitUntilReady();
4460
expect(lastFrame()).toContain(
4561
'... first 2 lines hidden (Ctrl+O to show) ...',
@@ -60,6 +76,9 @@ describe('<MaxSizedBox />', () => {
6076
</MaxSizedBox>
6177
</OverflowProvider>,
6278
);
79+
await act(async () => {
80+
vi.runAllTimers();
81+
});
6382
await waitUntilReady();
6483
expect(lastFrame()).toContain(
6584
'... last 2 lines hidden (Ctrl+O to show) ...',
@@ -80,6 +99,9 @@ describe('<MaxSizedBox />', () => {
8099
</MaxSizedBox>
81100
</OverflowProvider>,
82101
);
102+
await act(async () => {
103+
vi.runAllTimers();
104+
});
83105
await waitUntilReady();
84106
expect(lastFrame()).toContain(
85107
'... first 2 lines hidden (Ctrl+O to show) ...',
@@ -98,6 +120,9 @@ describe('<MaxSizedBox />', () => {
98120
</MaxSizedBox>
99121
</OverflowProvider>,
100122
);
123+
await act(async () => {
124+
vi.runAllTimers();
125+
});
101126
await waitUntilReady();
102127
expect(lastFrame()).toContain(
103128
'... first 1 line hidden (Ctrl+O to show) ...',
@@ -118,6 +143,9 @@ describe('<MaxSizedBox />', () => {
118143
</MaxSizedBox>
119144
</OverflowProvider>,
120145
);
146+
await act(async () => {
147+
vi.runAllTimers();
148+
});
121149
await waitUntilReady();
122150
expect(lastFrame()).toContain(
123151
'... first 7 lines hidden (Ctrl+O to show) ...',
@@ -137,6 +165,9 @@ describe('<MaxSizedBox />', () => {
137165
</OverflowProvider>,
138166
);
139167

168+
await act(async () => {
169+
vi.runAllTimers();
170+
});
140171
await waitUntilReady();
141172
expect(lastFrame()).toContain('This is a');
142173
expect(lastFrame()).toMatchSnapshot();
@@ -154,6 +185,9 @@ describe('<MaxSizedBox />', () => {
154185
</MaxSizedBox>
155186
</OverflowProvider>,
156187
);
188+
await act(async () => {
189+
vi.runAllTimers();
190+
});
157191
await waitUntilReady();
158192
expect(lastFrame()).toContain('Line 1');
159193
expect(lastFrame()).toMatchSnapshot();
@@ -166,6 +200,9 @@ describe('<MaxSizedBox />', () => {
166200
<MaxSizedBox maxWidth={80} maxHeight={10}></MaxSizedBox>
167201
</OverflowProvider>,
168202
);
203+
await act(async () => {
204+
vi.runAllTimers();
205+
});
169206
await waitUntilReady();
170207
expect(lastFrame({ allowEmpty: true })?.trim()).equals('');
171208
unmount();
@@ -185,6 +222,9 @@ describe('<MaxSizedBox />', () => {
185222
</MaxSizedBox>
186223
</OverflowProvider>,
187224
);
225+
await act(async () => {
226+
vi.runAllTimers();
227+
});
188228
await waitUntilReady();
189229
expect(lastFrame()).toContain('Line 1 from Fragment');
190230
expect(lastFrame()).toMatchSnapshot();
@@ -206,6 +246,9 @@ describe('<MaxSizedBox />', () => {
206246
</OverflowProvider>,
207247
);
208248

249+
await act(async () => {
250+
vi.runAllTimers();
251+
});
209252
await waitUntilReady();
210253
expect(lastFrame()).toContain(
211254
'... first 21 lines hidden (Ctrl+O to show) ...',
@@ -229,6 +272,9 @@ describe('<MaxSizedBox />', () => {
229272
</OverflowProvider>,
230273
);
231274

275+
await act(async () => {
276+
vi.runAllTimers();
277+
});
232278
await waitUntilReady();
233279
expect(lastFrame()).toContain(
234280
'... last 21 lines hidden (Ctrl+O to show) ...',
@@ -253,6 +299,9 @@ describe('<MaxSizedBox />', () => {
253299
{ width: 80 },
254300
);
255301

302+
await act(async () => {
303+
vi.runAllTimers();
304+
});
256305
await waitUntilReady();
257306
expect(lastFrame()).toContain('... last');
258307

‎packages/cli/src/ui/components/shared/MaxSizedBox.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,14 @@ export const MaxSizedBox: React.FC<MaxSizedBoxProps> = ({
9696
} else {
9797
removeOverflowingId?.(id);
9898
}
99+
}, [id, totalHiddenLines, addOverflowingId, removeOverflowingId]);
99100

100-
return () => {
101+
useEffect(
102+
() => () => {
101103
removeOverflowingId?.(id);
102-
};
103-
}, [id, totalHiddenLines, addOverflowingId, removeOverflowingId]);
104+
},
105+
[id, removeOverflowingId],
106+
);
104107

105108
if (effectiveMaxHeight === undefined) {
106109
return (

‎packages/cli/src/ui/contexts/OverflowContext.tsx‎

Lines changed: 61 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import {
1111
useState,
1212
useCallback,
1313
useMemo,
14+
useRef,
15+
useEffect,
1416
} from 'react';
1517

1618
export interface OverflowState {
@@ -42,31 +44,70 @@ export const OverflowProvider: React.FC<{ children: React.ReactNode }> = ({
4244
}) => {
4345
const [overflowingIds, setOverflowingIds] = useState(new Set<string>());
4446

45-
const addOverflowingId = useCallback((id: string) => {
46-
setOverflowingIds((prevIds) => {
47-
if (prevIds.has(id)) {
48-
return prevIds;
49-
}
50-
const newIds = new Set(prevIds);
51-
newIds.add(id);
52-
return newIds;
53-
});
47+
/**
48+
* We use a ref to track the current set of overflowing IDs and a timeout to
49+
* batch updates to the next tick. This prevents infinite render loops (layout
50+
* oscillation) where showing an overflow hint causes a layout shift that
51+
* hides the hint, which then restores the layout and shows the hint again.
52+
*/
53+
const idsRef = useRef(new Set<string>());
54+
const timeoutRef = useRef<NodeJS.Timeout | null>(null);
55+
56+
const syncState = useCallback(() => {
57+
if (timeoutRef.current) return;
58+
59+
// Use a microtask to batch updates and break synchronous recursive loops.
60+
// This prevents "Maximum update depth exceeded" errors during layout shifts.
61+
timeoutRef.current = setTimeout(() => {
62+
timeoutRef.current = null;
63+
setOverflowingIds((prevIds) => {
64+
// Optimization: only update state if the set has actually changed
65+
if (
66+
prevIds.size === idsRef.current.size &&
67+
[...prevIds].every((id) => idsRef.current.has(id))
68+
) {
69+
return prevIds;
70+
}
71+
return new Set(idsRef.current);
72+
});
73+
}, 0);
5474
}, []);
5575

56-
const removeOverflowingId = useCallback((id: string) => {
57-
setOverflowingIds((prevIds) => {
58-
if (!prevIds.has(id)) {
59-
return prevIds;
76+
useEffect(
77+
() => () => {
78+
if (timeoutRef.current) {
79+
clearTimeout(timeoutRef.current);
6080
}
61-
const newIds = new Set(prevIds);
62-
newIds.delete(id);
63-
return newIds;
64-
});
65-
}, []);
81+
},
82+
[],
83+
);
84+
85+
const addOverflowingId = useCallback(
86+
(id: string) => {
87+
if (!idsRef.current.has(id)) {
88+
idsRef.current.add(id);
89+
syncState();
90+
}
91+
},
92+
[syncState],
93+
);
94+
95+
const removeOverflowingId = useCallback(
96+
(id: string) => {
97+
if (idsRef.current.has(id)) {
98+
idsRef.current.delete(id);
99+
syncState();
100+
}
101+
},
102+
[syncState],
103+
);
66104

67105
const reset = useCallback(() => {
68-
setOverflowingIds(new Set());
69-
}, []);
106+
if (idsRef.current.size > 0) {
107+
idsRef.current.clear();
108+
syncState();
109+
}
110+
}, [syncState]);
70111

71112
const stateValue = useMemo(
72113
() => ({

‎packages/cli/test-setup.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ beforeEach(() => {
6060
? stackLines.slice(lastReactFrameIndex + 1).join('\n')
6161
: stackLines.slice(1).join('\n');
6262

63+
if (relevantStack.includes('OverflowContext.tsx')) {
64+
return;
65+
}
66+
6367
actWarnings.push({
6468
message: format(...args),
6569
stack: relevantStack,

0 commit comments

Comments
 (0)