Skip to content

Commit 84a5341

Browse files
bsunderhusclaude
andcommitted
refactor(react-overflow,priority-overflow): make manager creation pure and lifecycle strict-mode safe
Restructure the priority-overflow manager and the react-overflow container hook so that manager creation is render-pure and the observation lifecycle is colocated. priority-overflow: - createOverflowManager now accepts initialOptions and merges them over defaults - new setOptions method updates engine options without re-creating the manager - observe(container) returns a cleanup function that stops observation only (item, divider, and overflow menu registrations are no longer torn down) - disconnect is preserved as a shortcut for the latest observe cleanup - introduce OverflowManagerOptions type alias and complete tsdoc on the surface react-overflow: - create the manager via useMemo so React renders stay pure - start observation in a single useIsomorphicLayoutEffect and return the observe cleanup as the effect cleanup - propagate option changes through setOptions instead of rebuilding the manager - ref-guard the overflow menu and divider deregistration so strict-mode double invocation does not unregister a freshly mounted element Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 833fd4c commit 84a5341

6 files changed

Lines changed: 409 additions & 170 deletions

File tree

packages/react-components/priority-overflow/etc/priority-overflow.api.md

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
55
```ts
66

7-
// @internal (undocumented)
8-
export function createOverflowManager(): OverflowManager;
7+
// @internal
8+
export function createOverflowManager(initialOptions?: Partial<ObserveOptions>): OverflowManager;
99

10-
// @public (undocumented)
10+
// @public
1111
export interface ObserveOptions {
1212
hasHiddenItems?: boolean;
1313
minimumVisible?: number;
@@ -18,67 +18,61 @@ export interface ObserveOptions {
1818
padding?: number;
1919
}
2020

21-
// @public (undocumented)
21+
// @public
2222
export type OnUpdateItemVisibility = (data: OnUpdateItemVisibilityPayload) => void;
2323

24-
// @public (undocumented)
24+
// @public
2525
export interface OnUpdateItemVisibilityPayload {
26-
// (undocumented)
2726
item: OverflowItemEntry;
28-
// (undocumented)
2927
visible: boolean;
3028
}
3129

3230
// @public
3331
export type OnUpdateOverflow = (data: OverflowEventPayload) => void;
3432

35-
// @public (undocumented)
33+
// @public
3634
export type OverflowAxis = 'horizontal' | 'vertical';
3735

38-
// @public (undocumented)
36+
// @public
3937
export type OverflowDirection = 'start' | 'end';
4038

41-
// @public (undocumented)
39+
// @public
4240
export interface OverflowDividerEntry {
4341
element: HTMLElement;
44-
// (undocumented)
4542
groupId: string;
4643
}
4744

4845
// @public
4946
export interface OverflowEventPayload {
50-
// (undocumented)
5147
groupVisibility: Record<string, OverflowGroupState>;
52-
// (undocumented)
5348
invisibleItems: OverflowItemEntry[];
54-
// (undocumented)
5549
visibleItems: OverflowItemEntry[];
5650
}
5751

58-
// @public (undocumented)
52+
// @public
5953
export type OverflowGroupState = 'visible' | 'hidden' | 'overflow';
6054

61-
// @public (undocumented)
55+
// @public
6256
export interface OverflowItemEntry {
6357
element: HTMLElement;
64-
// (undocumented)
6558
groupId?: string;
6659
id: string;
6760
pinned?: boolean;
6861
priority: number;
6962
}
7063

71-
// @internal (undocumented)
64+
// @internal
7265
export interface OverflowManager {
7366
addDivider: (divider: OverflowDividerEntry) => void;
74-
addItem: (items: OverflowItemEntry) => void;
67+
addItem: (item: OverflowItemEntry) => void;
7568
addOverflowMenu: (element: HTMLElement) => void;
7669
disconnect: () => void;
7770
forceUpdate: () => void;
78-
observe: (container: HTMLElement, options: ObserveOptions) => void;
71+
observe: (container: HTMLElement) => () => void;
7972
removeDivider: (groupId: string) => void;
8073
removeItem: (itemId: string) => void;
8174
removeOverflowMenu: () => void;
75+
setOptions: (options: Partial<ObserveOptions>) => void;
8276
update: () => void;
8377
}
8478

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
import { createOverflowManager } from './overflowManager';
2+
import type { ObserveOptions, OverflowEventPayload } from './types';
3+
4+
describe('overflowManager', () => {
5+
beforeAll(() => {
6+
global.ResizeObserver = class ResizeObserver {
7+
public observe() {
8+
// do nothing
9+
}
10+
11+
public unobserve() {
12+
// do nothing
13+
}
14+
15+
public disconnect() {
16+
// do nothing
17+
}
18+
} as unknown as typeof ResizeObserver;
19+
});
20+
21+
const createElementWithSize = (tagName: string, width: number) => {
22+
const element = document.createElement(tagName);
23+
Object.defineProperty(element, 'offsetWidth', { configurable: true, value: width });
24+
Object.defineProperty(element, 'offsetHeight', { configurable: true, value: width });
25+
26+
return element;
27+
};
28+
29+
const createContainer = (width: number) => {
30+
const container = document.createElement('div');
31+
Object.defineProperty(container, 'clientWidth', { configurable: true, value: width });
32+
Object.defineProperty(container, 'clientHeight', { configurable: true, value: width });
33+
34+
return container;
35+
};
36+
37+
const createObserveOptions = (options: Partial<ObserveOptions> = {}): ObserveOptions => ({
38+
overflowAxis: 'horizontal',
39+
overflowDirection: 'end',
40+
padding: 10,
41+
minimumVisible: 0,
42+
hasHiddenItems: false,
43+
onUpdateItemVisibility: jest.fn(),
44+
onUpdateOverflow: jest.fn(),
45+
...options,
46+
});
47+
48+
const lastDispatch = (onUpdateOverflow: jest.Mock): OverflowEventPayload =>
49+
onUpdateOverflow.mock.calls[onUpdateOverflow.mock.calls.length - 1][0];
50+
51+
it('should dispatch overflow update after forceUpdate', () => {
52+
const onUpdateOverflow = jest.fn();
53+
const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow }));
54+
const container = createContainer(100);
55+
const itemA = createElementWithSize('button', 40);
56+
const itemB = createElementWithSize('button', 40);
57+
const menu = createElementWithSize('button', 20);
58+
59+
manager.addItem({ element: itemA, id: 'a', priority: 1 });
60+
manager.addItem({ element: itemB, id: 'b', priority: 0 });
61+
manager.addOverflowMenu(menu);
62+
manager.observe(container);
63+
manager.forceUpdate();
64+
65+
const dispatch = lastDispatch(onUpdateOverflow);
66+
expect(dispatch.visibleItems.map(item => item.id).sort()).toEqual(['a', 'b']);
67+
expect(dispatch.invisibleItems).toEqual([]);
68+
expect(dispatch.groupVisibility).toEqual({});
69+
});
70+
71+
it('should re-dispatch when setOptions changes a relevant option', () => {
72+
const onUpdateOverflow = jest.fn();
73+
const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow }));
74+
const container = createContainer(100);
75+
const itemA = createElementWithSize('button', 40);
76+
const itemB = createElementWithSize('button', 40);
77+
const menu = createElementWithSize('button', 20);
78+
79+
manager.addItem({ element: itemA, id: 'a', priority: 1 });
80+
manager.addItem({ element: itemB, id: 'b', priority: 0 });
81+
manager.addOverflowMenu(menu);
82+
manager.observe(container);
83+
manager.forceUpdate();
84+
85+
onUpdateOverflow.mockClear();
86+
manager.setOptions({ padding: 30 });
87+
88+
expect(onUpdateOverflow).toHaveBeenCalled();
89+
const dispatch = lastDispatch(onUpdateOverflow);
90+
expect(dispatch.visibleItems.map(item => item.id)).toEqual(['a']);
91+
expect(dispatch.invisibleItems.map(item => item.id)).toEqual(['b']);
92+
});
93+
94+
it('observe should return a cleanup that allows re-observation', () => {
95+
const onUpdateOverflow = jest.fn();
96+
const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow }));
97+
const container = createContainer(100);
98+
const item = createElementWithSize('button', 40);
99+
100+
manager.addItem({ element: item, id: 'a', priority: 1 });
101+
const cleanup = manager.observe(container);
102+
manager.forceUpdate();
103+
expect(lastDispatch(onUpdateOverflow).visibleItems.map(i => i.id)).toEqual(['a']);
104+
105+
cleanup();
106+
onUpdateOverflow.mockClear();
107+
108+
manager.observe(container);
109+
manager.forceUpdate();
110+
expect(onUpdateOverflow).toHaveBeenCalled();
111+
expect(lastDispatch(onUpdateOverflow).visibleItems.map(i => i.id)).toEqual(['a']);
112+
});
113+
114+
it('should remove items through removeItem', () => {
115+
const onUpdateOverflow = jest.fn();
116+
const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow }));
117+
const container = createContainer(100);
118+
const item = createElementWithSize('button', 40);
119+
120+
manager.addItem({ element: item, id: 'a', priority: 1 });
121+
manager.observe(container);
122+
manager.forceUpdate();
123+
124+
expect(lastDispatch(onUpdateOverflow).visibleItems.map(i => i.id)).toEqual(['a']);
125+
126+
manager.removeItem('a');
127+
manager.forceUpdate();
128+
129+
const dispatch = lastDispatch(onUpdateOverflow);
130+
expect(dispatch.visibleItems).toEqual([]);
131+
expect(dispatch.invisibleItems).toEqual([]);
132+
});
133+
});

packages/react-components/priority-overflow/src/overflowManager.ts

Lines changed: 69 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,24 @@ import type {
1111
OverflowDividerEntry,
1212
} from './types';
1313

14+
const DEFAULT_OPTIONS: Required<ObserveOptions> = {
15+
overflowAxis: 'horizontal',
16+
overflowDirection: 'end',
17+
padding: 10,
18+
minimumVisible: 0,
19+
hasHiddenItems: false,
20+
onUpdateItemVisibility: () => undefined,
21+
onUpdateOverflow: () => undefined,
22+
};
23+
1424
/**
25+
* Creates an overflow manager instance for a single container.
26+
*
1527
* @internal
28+
* @param initialOptions - Initial observe options. Missing values are filled with defaults.
1629
* @returns overflow manager instance
1730
*/
18-
export function createOverflowManager(): OverflowManager {
31+
export function createOverflowManager(initialOptions: Partial<ObserveOptions> = {}): OverflowManager {
1932
// calls to `offsetWidth or offsetHeight` can happen multiple times in an update
2033
// Use a cache to avoid causing too many recalcs and avoid scripting time to meausure sizes
2134
const sizeCache = new Map<HTMLElement, number>();
@@ -26,16 +39,7 @@ export function createOverflowManager(): OverflowManager {
2639
// If true, next update will dispatch to onUpdateOverflow even if queue top states don't change
2740
// Initially true to force dispatch on first mount
2841
let forceDispatch = true;
29-
const options: Required<ObserveOptions> = {
30-
padding: 10,
31-
overflowAxis: 'horizontal',
32-
overflowDirection: 'end',
33-
minimumVisible: 0,
34-
onUpdateItemVisibility: () => undefined,
35-
onUpdateOverflow: () => undefined,
36-
hasHiddenItems: false,
37-
};
38-
42+
const options: Required<ObserveOptions> = { ...DEFAULT_OPTIONS, ...initialOptions };
3943
const overflowItems: Record<string, OverflowItemEntry> = {};
4044
const overflowDividers: Record<string, OverflowDividerEntry> = {};
4145
let disposeResizeObserver: () => void = () => null;
@@ -205,21 +209,65 @@ export function createOverflowManager(): OverflowManager {
205209

206210
const update: OverflowManager['update'] = debounce(forceUpdate);
207211

208-
const observe: OverflowManager['observe'] = (observedContainer, userOptions) => {
209-
Object.assign(options, userOptions);
210-
observing = true;
211-
Object.values(overflowItems).forEach(item => visibleItemQueue.enqueue(item.id));
212+
const setOptions: OverflowManager['setOptions'] = nextOptions => {
213+
if (options === nextOptions) {
214+
return;
215+
}
216+
const previousAxis = options.overflowAxis;
217+
const previousDirection = options.overflowDirection;
218+
const previousPadding = options.padding;
219+
const previousMinimumVisible = options.minimumVisible;
220+
const previousHasHiddenItems = options.hasHiddenItems;
221+
222+
Object.assign(options, nextOptions);
223+
224+
if (
225+
previousAxis !== options.overflowAxis ||
226+
previousDirection !== options.overflowDirection ||
227+
previousPadding !== options.padding ||
228+
previousMinimumVisible !== options.minimumVisible ||
229+
previousHasHiddenItems !== options.hasHiddenItems
230+
) {
231+
forceDispatch = true;
232+
update();
233+
}
234+
};
235+
236+
let observeCleanup: () => void = () => null;
237+
238+
const observe: OverflowManager['observe'] = observedContainer => {
239+
Object.values(overflowItems).forEach(item => {
240+
if (!visibleItemQueue.contains(item.id) && !invisibleItemQueue.contains(item.id)) {
241+
visibleItemQueue.enqueue(item.id);
242+
}
243+
});
212244

213245
container = observedContainer;
246+
observing = true;
214247
disposeResizeObserver = observeResize(container, entries => {
215248
if (!entries[0] || !container) {
216249
return;
217250
}
218-
219251
update();
220252
});
253+
254+
const cleanup = () => {
255+
if (container !== observedContainer) {
256+
return;
257+
}
258+
disposeResizeObserver();
259+
disposeResizeObserver = () => null;
260+
container = undefined;
261+
observing = false;
262+
forceDispatch = true;
263+
};
264+
265+
observeCleanup = cleanup;
266+
return cleanup;
221267
};
222268

269+
const disconnect: OverflowManager['disconnect'] = () => observeCleanup();
270+
223271
const addItem: OverflowManager['addItem'] = item => {
224272
if (overflowItems[item.id]) {
225273
return;
@@ -234,14 +282,13 @@ export function createOverflowManager(): OverflowManager {
234282
// force a dispatch on the next batched update
235283
forceDispatch = true;
236284
visibleItemQueue.enqueue(item.id);
285+
update();
237286
}
238287

239288
if (item.groupId) {
240289
groupManager.addItem(item.id, item.groupId);
241290
item.element.setAttribute(DATA_OVERFLOW_GROUP, item.groupId);
242291
}
243-
244-
update();
245292
};
246293

247294
const addOverflowMenu: OverflowManager['addOverflowMenu'] = el => {
@@ -294,22 +341,9 @@ export function createOverflowManager(): OverflowManager {
294341

295342
sizeCache.delete(item.element);
296343
delete overflowItems[itemId];
297-
update();
298-
};
299-
300-
const disconnect: OverflowManager['disconnect'] = () => {
301-
disposeResizeObserver();
302-
303-
// reset flags
304-
container = undefined;
305-
observing = false;
306-
forceDispatch = true;
307-
308-
// clear all entries
309-
Object.keys(overflowItems).forEach(itemId => removeItem(itemId));
310-
Object.keys(overflowDividers).forEach(dividerId => removeDivider(dividerId));
311-
removeOverflowMenu();
312-
sizeCache.clear();
344+
if (observing) {
345+
update();
346+
}
313347
};
314348

315349
return {
@@ -323,6 +357,7 @@ export function createOverflowManager(): OverflowManager {
323357
removeOverflowMenu,
324358
addDivider,
325359
removeDivider,
360+
setOptions,
326361
};
327362
}
328363

0 commit comments

Comments
 (0)