From 84a53412c16c3317ba060395bcf60ba3da6bdf45 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Thu, 28 May 2026 11:39:09 +0200 Subject: [PATCH 1/7] 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) --- .../etc/priority-overflow.api.md | 34 ++--- .../src/overflowManager.test.ts | 133 ++++++++++++++++++ .../priority-overflow/src/overflowManager.ts | 103 +++++++++----- .../priority-overflow/src/types.ts | 118 ++++++++++++---- ....test.ts => useOverflowContainer.test.tsx} | 111 +++++++++------ .../library/src/useOverflowContainer.ts | 80 +++++------ 6 files changed, 409 insertions(+), 170 deletions(-) create mode 100644 packages/react-components/priority-overflow/src/overflowManager.test.ts rename packages/react-components/react-overflow/library/src/{useOverflowContainer.test.ts => useOverflowContainer.test.tsx} (59%) diff --git a/packages/react-components/priority-overflow/etc/priority-overflow.api.md b/packages/react-components/priority-overflow/etc/priority-overflow.api.md index bf10a02394bb8..73ac72034be45 100644 --- a/packages/react-components/priority-overflow/etc/priority-overflow.api.md +++ b/packages/react-components/priority-overflow/etc/priority-overflow.api.md @@ -4,10 +4,10 @@ ```ts -// @internal (undocumented) -export function createOverflowManager(): OverflowManager; +// @internal +export function createOverflowManager(initialOptions?: Partial): OverflowManager; -// @public (undocumented) +// @public export interface ObserveOptions { hasHiddenItems?: boolean; minimumVisible?: number; @@ -18,67 +18,61 @@ export interface ObserveOptions { padding?: number; } -// @public (undocumented) +// @public export type OnUpdateItemVisibility = (data: OnUpdateItemVisibilityPayload) => void; -// @public (undocumented) +// @public export interface OnUpdateItemVisibilityPayload { - // (undocumented) item: OverflowItemEntry; - // (undocumented) visible: boolean; } // @public export type OnUpdateOverflow = (data: OverflowEventPayload) => void; -// @public (undocumented) +// @public export type OverflowAxis = 'horizontal' | 'vertical'; -// @public (undocumented) +// @public export type OverflowDirection = 'start' | 'end'; -// @public (undocumented) +// @public export interface OverflowDividerEntry { element: HTMLElement; - // (undocumented) groupId: string; } // @public export interface OverflowEventPayload { - // (undocumented) groupVisibility: Record; - // (undocumented) invisibleItems: OverflowItemEntry[]; - // (undocumented) visibleItems: OverflowItemEntry[]; } -// @public (undocumented) +// @public export type OverflowGroupState = 'visible' | 'hidden' | 'overflow'; -// @public (undocumented) +// @public export interface OverflowItemEntry { element: HTMLElement; - // (undocumented) groupId?: string; id: string; pinned?: boolean; priority: number; } -// @internal (undocumented) +// @internal export interface OverflowManager { addDivider: (divider: OverflowDividerEntry) => void; - addItem: (items: OverflowItemEntry) => void; + addItem: (item: OverflowItemEntry) => void; addOverflowMenu: (element: HTMLElement) => void; disconnect: () => void; forceUpdate: () => void; - observe: (container: HTMLElement, options: ObserveOptions) => void; + observe: (container: HTMLElement) => () => void; removeDivider: (groupId: string) => void; removeItem: (itemId: string) => void; removeOverflowMenu: () => void; + setOptions: (options: Partial) => void; update: () => void; } diff --git a/packages/react-components/priority-overflow/src/overflowManager.test.ts b/packages/react-components/priority-overflow/src/overflowManager.test.ts new file mode 100644 index 0000000000000..33caa56dc164d --- /dev/null +++ b/packages/react-components/priority-overflow/src/overflowManager.test.ts @@ -0,0 +1,133 @@ +import { createOverflowManager } from './overflowManager'; +import type { ObserveOptions, OverflowEventPayload } from './types'; + +describe('overflowManager', () => { + beforeAll(() => { + global.ResizeObserver = class ResizeObserver { + public observe() { + // do nothing + } + + public unobserve() { + // do nothing + } + + public disconnect() { + // do nothing + } + } as unknown as typeof ResizeObserver; + }); + + const createElementWithSize = (tagName: string, width: number) => { + const element = document.createElement(tagName); + Object.defineProperty(element, 'offsetWidth', { configurable: true, value: width }); + Object.defineProperty(element, 'offsetHeight', { configurable: true, value: width }); + + return element; + }; + + const createContainer = (width: number) => { + const container = document.createElement('div'); + Object.defineProperty(container, 'clientWidth', { configurable: true, value: width }); + Object.defineProperty(container, 'clientHeight', { configurable: true, value: width }); + + return container; + }; + + const createObserveOptions = (options: Partial = {}): ObserveOptions => ({ + overflowAxis: 'horizontal', + overflowDirection: 'end', + padding: 10, + minimumVisible: 0, + hasHiddenItems: false, + onUpdateItemVisibility: jest.fn(), + onUpdateOverflow: jest.fn(), + ...options, + }); + + const lastDispatch = (onUpdateOverflow: jest.Mock): OverflowEventPayload => + onUpdateOverflow.mock.calls[onUpdateOverflow.mock.calls.length - 1][0]; + + it('should dispatch overflow update after forceUpdate', () => { + const onUpdateOverflow = jest.fn(); + const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const container = createContainer(100); + const itemA = createElementWithSize('button', 40); + const itemB = createElementWithSize('button', 40); + const menu = createElementWithSize('button', 20); + + manager.addItem({ element: itemA, id: 'a', priority: 1 }); + manager.addItem({ element: itemB, id: 'b', priority: 0 }); + manager.addOverflowMenu(menu); + manager.observe(container); + manager.forceUpdate(); + + const dispatch = lastDispatch(onUpdateOverflow); + expect(dispatch.visibleItems.map(item => item.id).sort()).toEqual(['a', 'b']); + expect(dispatch.invisibleItems).toEqual([]); + expect(dispatch.groupVisibility).toEqual({}); + }); + + it('should re-dispatch when setOptions changes a relevant option', () => { + const onUpdateOverflow = jest.fn(); + const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const container = createContainer(100); + const itemA = createElementWithSize('button', 40); + const itemB = createElementWithSize('button', 40); + const menu = createElementWithSize('button', 20); + + manager.addItem({ element: itemA, id: 'a', priority: 1 }); + manager.addItem({ element: itemB, id: 'b', priority: 0 }); + manager.addOverflowMenu(menu); + manager.observe(container); + manager.forceUpdate(); + + onUpdateOverflow.mockClear(); + manager.setOptions({ padding: 30 }); + + expect(onUpdateOverflow).toHaveBeenCalled(); + const dispatch = lastDispatch(onUpdateOverflow); + expect(dispatch.visibleItems.map(item => item.id)).toEqual(['a']); + expect(dispatch.invisibleItems.map(item => item.id)).toEqual(['b']); + }); + + it('observe should return a cleanup that allows re-observation', () => { + const onUpdateOverflow = jest.fn(); + const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const container = createContainer(100); + const item = createElementWithSize('button', 40); + + manager.addItem({ element: item, id: 'a', priority: 1 }); + const cleanup = manager.observe(container); + manager.forceUpdate(); + expect(lastDispatch(onUpdateOverflow).visibleItems.map(i => i.id)).toEqual(['a']); + + cleanup(); + onUpdateOverflow.mockClear(); + + manager.observe(container); + manager.forceUpdate(); + expect(onUpdateOverflow).toHaveBeenCalled(); + expect(lastDispatch(onUpdateOverflow).visibleItems.map(i => i.id)).toEqual(['a']); + }); + + it('should remove items through removeItem', () => { + const onUpdateOverflow = jest.fn(); + const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const container = createContainer(100); + const item = createElementWithSize('button', 40); + + manager.addItem({ element: item, id: 'a', priority: 1 }); + manager.observe(container); + manager.forceUpdate(); + + expect(lastDispatch(onUpdateOverflow).visibleItems.map(i => i.id)).toEqual(['a']); + + manager.removeItem('a'); + manager.forceUpdate(); + + const dispatch = lastDispatch(onUpdateOverflow); + expect(dispatch.visibleItems).toEqual([]); + expect(dispatch.invisibleItems).toEqual([]); + }); +}); diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index babf1954e60bd..4421b9d021330 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -11,11 +11,24 @@ import type { OverflowDividerEntry, } from './types'; +const DEFAULT_OPTIONS: Required = { + overflowAxis: 'horizontal', + overflowDirection: 'end', + padding: 10, + minimumVisible: 0, + hasHiddenItems: false, + onUpdateItemVisibility: () => undefined, + onUpdateOverflow: () => undefined, +}; + /** + * Creates an overflow manager instance for a single container. + * * @internal + * @param initialOptions - Initial observe options. Missing values are filled with defaults. * @returns overflow manager instance */ -export function createOverflowManager(): OverflowManager { +export function createOverflowManager(initialOptions: Partial = {}): OverflowManager { // calls to `offsetWidth or offsetHeight` can happen multiple times in an update // Use a cache to avoid causing too many recalcs and avoid scripting time to meausure sizes const sizeCache = new Map(); @@ -26,16 +39,7 @@ export function createOverflowManager(): OverflowManager { // If true, next update will dispatch to onUpdateOverflow even if queue top states don't change // Initially true to force dispatch on first mount let forceDispatch = true; - const options: Required = { - padding: 10, - overflowAxis: 'horizontal', - overflowDirection: 'end', - minimumVisible: 0, - onUpdateItemVisibility: () => undefined, - onUpdateOverflow: () => undefined, - hasHiddenItems: false, - }; - + const options: Required = { ...DEFAULT_OPTIONS, ...initialOptions }; const overflowItems: Record = {}; const overflowDividers: Record = {}; let disposeResizeObserver: () => void = () => null; @@ -205,21 +209,65 @@ export function createOverflowManager(): OverflowManager { const update: OverflowManager['update'] = debounce(forceUpdate); - const observe: OverflowManager['observe'] = (observedContainer, userOptions) => { - Object.assign(options, userOptions); - observing = true; - Object.values(overflowItems).forEach(item => visibleItemQueue.enqueue(item.id)); + const setOptions: OverflowManager['setOptions'] = nextOptions => { + if (options === nextOptions) { + return; + } + const previousAxis = options.overflowAxis; + const previousDirection = options.overflowDirection; + const previousPadding = options.padding; + const previousMinimumVisible = options.minimumVisible; + const previousHasHiddenItems = options.hasHiddenItems; + + Object.assign(options, nextOptions); + + if ( + previousAxis !== options.overflowAxis || + previousDirection !== options.overflowDirection || + previousPadding !== options.padding || + previousMinimumVisible !== options.minimumVisible || + previousHasHiddenItems !== options.hasHiddenItems + ) { + forceDispatch = true; + update(); + } + }; + + let observeCleanup: () => void = () => null; + + const observe: OverflowManager['observe'] = observedContainer => { + Object.values(overflowItems).forEach(item => { + if (!visibleItemQueue.contains(item.id) && !invisibleItemQueue.contains(item.id)) { + visibleItemQueue.enqueue(item.id); + } + }); container = observedContainer; + observing = true; disposeResizeObserver = observeResize(container, entries => { if (!entries[0] || !container) { return; } - update(); }); + + const cleanup = () => { + if (container !== observedContainer) { + return; + } + disposeResizeObserver(); + disposeResizeObserver = () => null; + container = undefined; + observing = false; + forceDispatch = true; + }; + + observeCleanup = cleanup; + return cleanup; }; + const disconnect: OverflowManager['disconnect'] = () => observeCleanup(); + const addItem: OverflowManager['addItem'] = item => { if (overflowItems[item.id]) { return; @@ -234,14 +282,13 @@ export function createOverflowManager(): OverflowManager { // force a dispatch on the next batched update forceDispatch = true; visibleItemQueue.enqueue(item.id); + update(); } if (item.groupId) { groupManager.addItem(item.id, item.groupId); item.element.setAttribute(DATA_OVERFLOW_GROUP, item.groupId); } - - update(); }; const addOverflowMenu: OverflowManager['addOverflowMenu'] = el => { @@ -294,22 +341,9 @@ export function createOverflowManager(): OverflowManager { sizeCache.delete(item.element); delete overflowItems[itemId]; - update(); - }; - - const disconnect: OverflowManager['disconnect'] = () => { - disposeResizeObserver(); - - // reset flags - container = undefined; - observing = false; - forceDispatch = true; - - // clear all entries - Object.keys(overflowItems).forEach(itemId => removeItem(itemId)); - Object.keys(overflowDividers).forEach(dividerId => removeDivider(dividerId)); - removeOverflowMenu(); - sizeCache.clear(); + if (observing) { + update(); + } }; return { @@ -323,6 +357,7 @@ export function createOverflowManager(): OverflowManager { removeOverflowMenu, addDivider, removeDivider, + setOptions, }; } diff --git a/packages/react-components/priority-overflow/src/types.ts b/packages/react-components/priority-overflow/src/types.ts index c3760795ca417..76382716a8469 100644 --- a/packages/react-components/priority-overflow/src/types.ts +++ b/packages/react-components/priority-overflow/src/types.ts @@ -1,21 +1,39 @@ +/** + * Direction where items are removed when overflow occurs. + */ export type OverflowDirection = 'start' | 'end'; + +/** + * Axis used to measure overflow. + */ export type OverflowAxis = 'horizontal' | 'vertical'; + +/** + * Visibility state for an overflow group. + */ export type OverflowGroupState = 'visible' | 'hidden' | 'overflow'; + +/** + * Tracked item in the overflow manager. + */ export interface OverflowItemEntry { /** - * HTML element that will be disappear when overflowed + * HTML element that disappears when the item overflows. */ element: HTMLElement; /** - * Lower priority items are invisible first when the container is overflowed + * Lower-priority items become invisible first when the container overflows. * @default 0 */ priority: number; /** - * Specific id, used to track visibility and provide updates to consumers + * Stable item id used to track visibility and emit updates. */ id: string; + /** + * Optional group id used to coordinate divider and grouped visibility states. + */ groupId?: string; /** @@ -26,125 +44,167 @@ export interface OverflowItemEntry { pinned?: boolean; } +/** + * Tracked divider in the overflow manager. + */ export interface OverflowDividerEntry { /** - * HTML element that will disappear when overflowed + * HTML element that disappears when its group overflows. */ element: HTMLElement; + /** + * Id of the group controlled by this divider. + */ groupId: string; } /** - * signature similar to standard event listeners, but typed to handle the custom event + * Signature similar to standard event listeners, typed for overflow updates. */ export type OnUpdateOverflow = (data: OverflowEventPayload) => void; +/** + * Callback invoked when a single item's visibility changes. + */ export type OnUpdateItemVisibility = (data: OnUpdateItemVisibilityPayload) => void; /** * Payload of the custom DOM event for overflow updates */ export interface OverflowEventPayload { + /** + * Items currently visible in the container. + */ visibleItems: OverflowItemEntry[]; + + /** + * Items currently moved to overflow. + */ invisibleItems: OverflowItemEntry[]; + + /** + * Current visibility state by group id. + */ groupVisibility: Record; } +/** + * Payload for item-level visibility updates. + */ export interface OnUpdateItemVisibilityPayload { + /** + * Item whose visibility changed. + */ item: OverflowItemEntry; + + /** + * Whether the item is now visible. + */ visible: boolean; } +/** + * Options used to initialize or reconfigure overflow observation. + */ export interface ObserveOptions { /** - * Padding (in px) at the end of the container before overflow occurs - * Useful to account for extra elements (i.e. dropdown menu) - * or to account for any kinds of margins between items which are hard to measure with JS + * Padding in pixels reserved at the end of the container before overflow occurs. + * Useful for accounting for extra elements (for example an overflow menu button) + * or margins between items that are difficult to measure in JavaScript. * @default 10 */ padding?: number; /** - * Direction where items are removed when overflow occurs + * Direction where items are removed when overflow occurs. * @default end */ overflowDirection?: OverflowDirection; /** - * Horizontal or vertical overflow + * Overflow axis used for size measurement. * @default horizontal */ overflowAxis?: OverflowAxis; /** - * The minimum number of visible items + * Minimum number of items that must remain visible. */ minimumVisible?: number; /** - * Callback when item visibility is updated + * Callback invoked when an individual item's visibility changes. */ onUpdateItemVisibility: OnUpdateItemVisibility; /** - * Callback when item visibility is updated + * Callback invoked after overflow state is recomputed. */ onUpdateOverflow: OnUpdateOverflow; /** - * When true, the overflow menu has default hidden items + * When true, reserve space as if the overflow menu were visible even with no overflowing items. * @default false */ hasHiddenItems?: boolean; } /** + * Internal manager contract used to observe and compute priority overflow. + * * @internal */ export interface OverflowManager { /** - * Starts observing the container and managing the overflow state + * Updates engine options without requiring full observation re-creation. */ - observe: (container: HTMLElement, options: ObserveOptions) => void; + setOptions: (options: Partial) => void; /** - * Stops observing the container + * Starts observing the container and managing overflow state. */ - disconnect: () => void; + observe: (container: HTMLElement) => () => void; /** - * Add overflow items + * Adds an item to overflow tracking. */ - addItem: (items: OverflowItemEntry) => void; + addItem: (item: OverflowItemEntry) => void; + /** - * Remove overflow item + * Removes an overflow item by id. */ removeItem: (itemId: string) => void; /** - * Manually update the overflow, updates are batched and async + * Schedules an asynchronous overflow recomputation. */ update: () => void; /** - * Manually update the overflow sync + * Forces an immediate synchronous overflow recomputation. */ forceUpdate: () => void; /** - * Adds an element that opens an overflow menu. This is used to calculate - * available space and check if additional items need to overflow + * Attaches the overflow menu element. + * This is used to calculate available space and determine whether more items should overflow. */ addOverflowMenu: (element: HTMLElement) => void; /** - * Add overflow divider + * Removes the overflow menu element. + */ + removeOverflowMenu: () => void; + + /** + * Adds a divider for the provided group. */ addDivider: (divider: OverflowDividerEntry) => void; /** - * Remove overflow divider + * Removes a divider by group id. */ removeDivider: (groupId: string) => void; /** - * Unsets the overflow menu element + * Disconnects all active observers. + * Equivalent to calling the latest cleanup function returned by `observe`. */ - removeOverflowMenu: () => void; + disconnect: () => void; } diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx similarity index 59% rename from packages/react-components/react-overflow/library/src/useOverflowContainer.test.ts rename to packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx index fdb02fb571166..1834065bd5e51 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx @@ -1,8 +1,9 @@ -import type * as React from 'react'; +import * as React from 'react'; import type { OverflowAxis, OverflowManager } from '@fluentui/priority-overflow'; import { createOverflowManager } from '@fluentui/priority-overflow'; -import { useOverflowContainer } from './useOverflowContainer'; +import { render } from '@testing-library/react'; import { renderHook } from '@testing-library/react-hooks'; +import { useOverflowContainer } from './useOverflowContainer'; jest.mock('@fluentui/priority-overflow'); @@ -12,12 +13,13 @@ const mockOverflowManager = (options: Partial = {}) => { addOverflowMenu: jest.fn(), disconnect: jest.fn(), forceUpdate: jest.fn(), - observe: jest.fn(), + observe: jest.fn(() => () => null), removeItem: jest.fn(), removeOverflowMenu: jest.fn(), update: jest.fn(), addDivider: jest.fn(), removeDivider: jest.fn(), + setOptions: jest.fn(), }; (createOverflowManager as jest.Mock).mockReturnValue({ ...defaultMock, @@ -26,10 +28,22 @@ const mockOverflowManager = (options: Partial = {}) => { }; describe('useOverflowContainer', () => { - it('should create overflow manager', () => { + beforeEach(() => { + (createOverflowManager as jest.Mock).mockReset(); + mockOverflowManager(); + }); + + it('should create overflow manager with initial options', () => { renderHook(() => useOverflowContainer(() => undefined, { onUpdateItemVisibility: () => undefined })); expect(createOverflowManager).toHaveBeenCalledTimes(1); + expect((createOverflowManager as jest.Mock).mock.calls[0][0]).toMatchObject({ + hasHiddenItems: false, + minimumVisible: 0, + overflowAxis: 'horizontal', + overflowDirection: 'end', + padding: 10, + }); }); it('should add to overflow manager when registering item', () => { @@ -61,30 +75,57 @@ describe('useOverflowContainer', () => { expect(removeItemMock).toHaveBeenCalledWith(overflowItem.id); }); - it('should call observe with default options', () => { - const observeMock = jest.fn(); + it('should call observe with the container element', () => { + const observeMock = jest.fn(() => () => null); mockOverflowManager({ observe: observeMock }); - const { result, rerender } = renderHook(() => { - return useOverflowContainer(() => undefined, { onUpdateItemVisibility: () => undefined }); - }); - // eslint-disable-next-line @typescript-eslint/no-deprecated - (result.current.containerRef as React.MutableRefObject).current = document.createElement('div'); - rerender(); + + const TestComponent: React.FC = () => { + const { containerRef } = useOverflowContainer(() => undefined, { + onUpdateItemVisibility: () => undefined, + }); + return
; + }; + + const { getByTestId } = render(); expect(observeMock).toHaveBeenCalledTimes(1); - expect(observeMock.mock.calls[0]).toMatchInlineSnapshot(` - Array [ -
, - Object { - "hasHiddenItems": false, - "minimumVisible": 0, - "onUpdateItemVisibility": [Function], - "onUpdateOverflow": [Function], - "overflowAxis": "horizontal", - "overflowDirection": "end", - "padding": 10, - }, - ] - `); + expect(observeMock).toHaveBeenCalledWith(getByTestId('container')); + }); + + it('should dispose observation on unmount', () => { + const disposeMock = jest.fn(); + const observeMock = jest.fn(() => disposeMock); + mockOverflowManager({ observe: observeMock }); + + const TestComponent: React.FC = () => { + const { containerRef } = useOverflowContainer(() => undefined, { + onUpdateItemVisibility: () => undefined, + }); + return
; + }; + + const { unmount } = render(); + expect(disposeMock).not.toHaveBeenCalled(); + unmount(); + expect(disposeMock).toHaveBeenCalledTimes(1); + }); + + it('should call setOptions when options change', () => { + const setOptionsMock = jest.fn(); + mockOverflowManager({ setOptions: setOptionsMock }); + + let overflowAxis: OverflowAxis = 'horizontal'; + const { rerender } = renderHook(() => + useOverflowContainer(() => undefined, { onUpdateItemVisibility: () => undefined, overflowAxis }), + ); + + setOptionsMock.mockClear(); + overflowAxis = 'vertical'; + rerender(); + + expect(setOptionsMock).toHaveBeenCalled(); + expect(setOptionsMock.mock.calls[setOptionsMock.mock.calls.length - 1][0]).toMatchObject({ + overflowAxis: 'vertical', + }); }); it('should invoke updateOverflow on overflow manager', () => { @@ -100,7 +141,6 @@ describe('useOverflowContainer', () => { }); it('should not re-render on first mount', () => { - mockOverflowManager(); let renderCount = 0; renderHook(() => { renderCount++; @@ -109,21 +149,4 @@ describe('useOverflowContainer', () => { expect(renderCount).toEqual(1); }); - - it('should re-render when option changes', () => { - let overflowAxis: OverflowAxis = 'horizontal'; - mockOverflowManager(); - let renderCount = 0; - const { rerender } = renderHook(() => { - renderCount++; - return useOverflowContainer(() => undefined, { onUpdateItemVisibility: () => undefined, overflowAxis }); - }); - - expect(renderCount).toEqual(1); - - overflowAxis = 'vertical'; - rerender(); - - expect(renderCount).toEqual(2); - }); }); diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index 071f51d2e7a4c..345269fd048dc 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -14,7 +14,7 @@ import type { OverflowManager, ObserveOptions, } from '@fluentui/priority-overflow'; -import { canUseDOM, useEventCallback, useFirstMount, useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; +import { canUseDOM, useEventCallback, useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; import type { UseOverflowContainerReturn } from './types'; import { DATA_OVERFLOWING, DATA_OVERFLOW_DIVIDER, DATA_OVERFLOW_ITEM, DATA_OVERFLOW_MENU } from './constants'; @@ -42,20 +42,21 @@ export const useOverflowContainer = ( } = options; const onUpdateOverflow = useEventCallback(update); + const onUpdateItemVisibilityCallback = useEventCallback(onUpdateItemVisibility); - const overflowOptions = React.useMemo( + const observeOptions: Required = React.useMemo( () => ({ overflowAxis, overflowDirection, padding, minimumVisible, - onUpdateItemVisibility, + onUpdateItemVisibility: onUpdateItemVisibilityCallback, onUpdateOverflow, hasHiddenItems, }), [ minimumVisible, - onUpdateItemVisibility, + onUpdateItemVisibilityCallback, overflowAxis, overflowDirection, padding, @@ -64,86 +65,79 @@ export const useOverflowContainer = ( ], ); - const firstMount = useFirstMount(); - - // DOM ref to the overflow container element const containerRef = React.useRef(null); + const overflowMenuRef = React.useRef(null); + const dividerElementsRef = React.useRef(new Map()); - const [overflowManager, setOverflowManager] = React.useState(() => - canUseDOM() ? createOverflowManager() : null, + const manager = React.useMemo( + () => (canUseDOM() ? createOverflowManager(observeOptions) : null), + // Manager is created once for the component's lifetime; option changes go through setOptions. + // eslint-disable-next-line react-hooks/exhaustive-deps + [], ); - // On first mount there is no need to create an overflow manager and re-render - useIsomorphicLayoutEffect(() => { - if (firstMount && containerRef.current) { - overflowManager?.observe(containerRef.current, overflowOptions); - } - }, [firstMount, overflowManager, overflowOptions]); - useIsomorphicLayoutEffect(() => { - if (!containerRef.current || !canUseDOM() || firstMount) { - return; + if (manager && containerRef.current) { + return manager.observe(containerRef.current); } + }, [manager]); - const newOverflowManager = createOverflowManager(); - newOverflowManager.observe(containerRef.current, overflowOptions); - setOverflowManager(newOverflowManager); - // We don't want to re-create the overflow manager when the first mount flag changes from true to false - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [overflowOptions]); - - /* Clean up overflow manager on unmount */ - React.useEffect( - () => () => { - overflowManager?.disconnect(); - }, - [overflowManager], - ); + React.useEffect(() => { + manager?.setOptions(observeOptions); + }, [observeOptions, manager]); const registerItem = React.useCallback( (item: OverflowItemEntry) => { - overflowManager?.addItem(item); + manager?.addItem(item); item.element.setAttribute(DATA_OVERFLOW_ITEM, ''); return () => { item.element.removeAttribute(DATA_OVERFLOWING); item.element.removeAttribute(DATA_OVERFLOW_ITEM); - overflowManager?.removeItem(item.id); + manager?.removeItem(item.id); }; }, - [overflowManager], + [manager], ); const registerDivider = React.useCallback( (divider: OverflowDividerEntry) => { const el = divider.element; - overflowManager?.addDivider(divider); + manager?.addDivider(divider); + dividerElementsRef.current.set(divider.groupId, el); el.setAttribute(DATA_OVERFLOW_DIVIDER, ''); return () => { - divider.groupId && overflowManager?.removeDivider(divider.groupId); + if (dividerElementsRef.current.get(divider.groupId) === el) { + manager?.removeDivider(divider.groupId); + dividerElementsRef.current.delete(divider.groupId); + } el.removeAttribute(DATA_OVERFLOW_DIVIDER); }; }, - [overflowManager], + [manager], ); const registerOverflowMenu = React.useCallback( (el: HTMLElement) => { - overflowManager?.addOverflowMenu(el); + manager?.addOverflowMenu(el); + overflowMenuRef.current = el; el.setAttribute(DATA_OVERFLOW_MENU, ''); return () => { - overflowManager?.removeOverflowMenu(); + if (overflowMenuRef.current === el) { + manager?.removeOverflowMenu(); + overflowMenuRef.current = null; + } el.removeAttribute(DATA_OVERFLOW_MENU); }; }, - [overflowManager], + [manager], ); const updateOverflow = React.useCallback(() => { - overflowManager?.update(); - }, [overflowManager]); + manager?.update(); + }, [manager]); return { registerItem, From ae93c45e0a7083db73c8d0b213e36124aab42342 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Thu, 28 May 2026 11:43:12 +0200 Subject: [PATCH 2/7] chore: add change files for PR1 strict-mode/lifecycle refactor Co-Authored-By: Claude Opus 4.7 (1M context) --- change/@fluentui-priority-overflow-pr1-strict-mode.json | 7 +++++++ change/@fluentui-react-overflow-pr1-strict-mode.json | 7 +++++++ 2 files changed, 14 insertions(+) create mode 100644 change/@fluentui-priority-overflow-pr1-strict-mode.json create mode 100644 change/@fluentui-react-overflow-pr1-strict-mode.json diff --git a/change/@fluentui-priority-overflow-pr1-strict-mode.json b/change/@fluentui-priority-overflow-pr1-strict-mode.json new file mode 100644 index 0000000000000..3bc5e5da77106 --- /dev/null +++ b/change/@fluentui-priority-overflow-pr1-strict-mode.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: createOverflowManager accepts initialOptions, new setOptions method, observe now returns its cleanup, and the OverflowManagerOptions type is exported.", + "packageName": "@fluentui/priority-overflow", + "email": "bsunderhus@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-overflow-pr1-strict-mode.json b/change/@fluentui-react-overflow-pr1-strict-mode.json new file mode 100644 index 0000000000000..77de58239f215 --- /dev/null +++ b/change/@fluentui-react-overflow-pr1-strict-mode.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: make Overflow container strict-mode safe by creating the manager via useMemo, starting observation in a single layout effect with colocated cleanup, propagating option changes through setOptions, and ref-guarding overflow menu and divider deregistration.", + "packageName": "@fluentui/react-overflow", + "email": "bsunderhus@microsoft.com", + "dependentChangeType": "patch" +} From be892d0254fed9744487b3f4302d38af744442f2 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Thu, 28 May 2026 17:14:57 +0200 Subject: [PATCH 3/7] chore(react-overflow,priority-overflow): address PR1 review feedback - useOverflowContainer: drop redundant destructure defaults; the manager's DEFAULT_OPTIONS is the single source of truth. Adjust the test accordingly. - useOverflowContainer: switch manager creation to a useRef lazy initializer with a localized react-hooks/refs disable. - useOverflowContainer: setOptions propagation moves to a layout effect so option changes land before paint. - useOverflowContainer: drop the ref-guarded cleanups on registerOverflowMenu and registerDivider; in normal React flow the previous cleanup always runs before the next registration. - DEFAULT_OPTIONS no-op callbacks use a commented block to satisfy no-empty-function. - Shorten the priority-overflow change-file comment. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...uentui-react-overflow-pr1-strict-mode.json | 2 +- .../priority-overflow/src/overflowManager.ts | 20 ++++++++--- .../library/src/useOverflowContainer.test.tsx | 12 +++---- .../library/src/useOverflowContainer.ts | 35 ++++++------------- 4 files changed, 31 insertions(+), 38 deletions(-) diff --git a/change/@fluentui-react-overflow-pr1-strict-mode.json b/change/@fluentui-react-overflow-pr1-strict-mode.json index 77de58239f215..d699834de9cd5 100644 --- a/change/@fluentui-react-overflow-pr1-strict-mode.json +++ b/change/@fluentui-react-overflow-pr1-strict-mode.json @@ -1,6 +1,6 @@ { "type": "patch", - "comment": "fix: make Overflow container strict-mode safe by creating the manager via useMemo, starting observation in a single layout effect with colocated cleanup, propagating option changes through setOptions, and ref-guarding overflow menu and divider deregistration.", + "comment": "fix: make Overflow container strict-mode safe", "packageName": "@fluentui/react-overflow", "email": "bsunderhus@microsoft.com", "dependentChangeType": "patch" diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index 4421b9d021330..4d192e69c5bb5 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -17,8 +17,12 @@ const DEFAULT_OPTIONS: Required = { padding: 10, minimumVisible: 0, hasHiddenItems: false, - onUpdateItemVisibility: () => undefined, - onUpdateOverflow: () => undefined, + onUpdateItemVisibility: () => { + /* noop */ + }, + onUpdateOverflow: () => { + /* noop */ + }, }; /** @@ -42,7 +46,9 @@ export function createOverflowManager(initialOptions: Partial = const options: Required = { ...DEFAULT_OPTIONS, ...initialOptions }; const overflowItems: Record = {}; const overflowDividers: Record = {}; - let disposeResizeObserver: () => void = () => null; + let disposeResizeObserver: () => void = () => { + /* noop */ + }; const getNextItem = (queueToDequeue: PriorityQueue, queueToEnqueue: PriorityQueue) => { const nextItem = queueToDequeue.dequeue(); @@ -233,7 +239,9 @@ export function createOverflowManager(initialOptions: Partial = } }; - let observeCleanup: () => void = () => null; + let observeCleanup: () => void = () => { + /* noop */ + }; const observe: OverflowManager['observe'] = observedContainer => { Object.values(overflowItems).forEach(item => { @@ -256,7 +264,9 @@ export function createOverflowManager(initialOptions: Partial = return; } disposeResizeObserver(); - disposeResizeObserver = () => null; + disposeResizeObserver = () => { + /* noop */ + }; container = undefined; observing = false; forceDispatch = true; diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx index 1834065bd5e51..d3492b90ac3b5 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx @@ -33,16 +33,14 @@ describe('useOverflowContainer', () => { mockOverflowManager(); }); - it('should create overflow manager with initial options', () => { - renderHook(() => useOverflowContainer(() => undefined, { onUpdateItemVisibility: () => undefined })); + it('should create overflow manager with the provided options', () => { + renderHook(() => useOverflowContainer(() => undefined, { onUpdateItemVisibility: () => undefined, padding: 25 })); expect(createOverflowManager).toHaveBeenCalledTimes(1); expect((createOverflowManager as jest.Mock).mock.calls[0][0]).toMatchObject({ - hasHiddenItems: false, - minimumVisible: 0, - overflowAxis: 'horizontal', - overflowDirection: 'end', - padding: 10, + padding: 25, + onUpdateItemVisibility: expect.any(Function), + onUpdateOverflow: expect.any(Function), }); }); diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index 345269fd048dc..c22cc4011c71f 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -18,8 +18,6 @@ import { canUseDOM, useEventCallback, useIsomorphicLayoutEffect } from '@fluentu import type { UseOverflowContainerReturn } from './types'; import { DATA_OVERFLOWING, DATA_OVERFLOW_DIVIDER, DATA_OVERFLOW_ITEM, DATA_OVERFLOW_MENU } from './constants'; -const noop = () => null; - /** * @internal * @param update - Callback when overflow state changes @@ -32,19 +30,12 @@ export const useOverflowContainer = ( ): UseOverflowContainerReturn => { 'use no memo'; - const { - overflowAxis = 'horizontal', - overflowDirection = 'end', - padding = 10, - minimumVisible = 0, - onUpdateItemVisibility = noop, - hasHiddenItems = false, - } = options; + const { overflowAxis, overflowDirection, padding, minimumVisible, onUpdateItemVisibility, hasHiddenItems } = options; const onUpdateOverflow = useEventCallback(update); - const onUpdateItemVisibilityCallback = useEventCallback(onUpdateItemVisibility); + const onUpdateItemVisibilityCallback = useEventCallback(onUpdateItemVisibility ?? noop); - const observeOptions: Required = React.useMemo( + const observeOptions: Partial = React.useMemo( () => ({ overflowAxis, overflowDirection, @@ -66,8 +57,6 @@ export const useOverflowContainer = ( ); const containerRef = React.useRef(null); - const overflowMenuRef = React.useRef(null); - const dividerElementsRef = React.useRef(new Map()); const manager = React.useMemo( () => (canUseDOM() ? createOverflowManager(observeOptions) : null), @@ -82,7 +71,7 @@ export const useOverflowContainer = ( } }, [manager]); - React.useEffect(() => { + useIsomorphicLayoutEffect(() => { manager?.setOptions(observeOptions); }, [observeOptions, manager]); @@ -104,14 +93,10 @@ export const useOverflowContainer = ( (divider: OverflowDividerEntry) => { const el = divider.element; manager?.addDivider(divider); - dividerElementsRef.current.set(divider.groupId, el); el.setAttribute(DATA_OVERFLOW_DIVIDER, ''); return () => { - if (dividerElementsRef.current.get(divider.groupId) === el) { - manager?.removeDivider(divider.groupId); - dividerElementsRef.current.delete(divider.groupId); - } + manager?.removeDivider(divider.groupId); el.removeAttribute(DATA_OVERFLOW_DIVIDER); }; }, @@ -121,14 +106,10 @@ export const useOverflowContainer = ( const registerOverflowMenu = React.useCallback( (el: HTMLElement) => { manager?.addOverflowMenu(el); - overflowMenuRef.current = el; el.setAttribute(DATA_OVERFLOW_MENU, ''); return () => { - if (overflowMenuRef.current === el) { - manager?.removeOverflowMenu(); - overflowMenuRef.current = null; - } + manager?.removeOverflowMenu(); el.removeAttribute(DATA_OVERFLOW_MENU); }; }, @@ -148,6 +129,10 @@ export const useOverflowContainer = ( }; }; +const noop = () => { + /* noop */ +}; + export const updateVisibilityAttribute: OnUpdateItemVisibility = ({ item, visible }) => { if (visible) { item.element.removeAttribute(DATA_OVERFLOWING); From 2053b3bb7209459c80087960e54d9678f1a8a868 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Thu, 28 May 2026 18:19:57 +0200 Subject: [PATCH 4/7] fix(react-overflow): restore default values for missing observe options Removing the destructure defaults in useOverflowContainer caused undefined fields to spread over the manager's DEFAULT_OPTIONS, leaving runtime values like options.padding as undefined. processOverflowItems then computed availableSize as NaN, so no items overflowed and the menu never rendered. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../library/src/useOverflowContainer.test.tsx | 12 +++++++----- .../library/src/useOverflowContainer.ts | 13 ++++++++++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx index d3492b90ac3b5..1834065bd5e51 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx @@ -33,14 +33,16 @@ describe('useOverflowContainer', () => { mockOverflowManager(); }); - it('should create overflow manager with the provided options', () => { - renderHook(() => useOverflowContainer(() => undefined, { onUpdateItemVisibility: () => undefined, padding: 25 })); + it('should create overflow manager with initial options', () => { + renderHook(() => useOverflowContainer(() => undefined, { onUpdateItemVisibility: () => undefined })); expect(createOverflowManager).toHaveBeenCalledTimes(1); expect((createOverflowManager as jest.Mock).mock.calls[0][0]).toMatchObject({ - padding: 25, - onUpdateItemVisibility: expect.any(Function), - onUpdateOverflow: expect.any(Function), + hasHiddenItems: false, + minimumVisible: 0, + overflowAxis: 'horizontal', + overflowDirection: 'end', + padding: 10, }); }); diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index c22cc4011c71f..1bc009cbd7a6c 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -30,12 +30,19 @@ export const useOverflowContainer = ( ): UseOverflowContainerReturn => { 'use no memo'; - const { overflowAxis, overflowDirection, padding, minimumVisible, onUpdateItemVisibility, hasHiddenItems } = options; + const { + overflowAxis = 'horizontal', + overflowDirection = 'end', + padding = 10, + minimumVisible = 0, + onUpdateItemVisibility = noop, + hasHiddenItems = false, + } = options; const onUpdateOverflow = useEventCallback(update); - const onUpdateItemVisibilityCallback = useEventCallback(onUpdateItemVisibility ?? noop); + const onUpdateItemVisibilityCallback = useEventCallback(onUpdateItemVisibility); - const observeOptions: Partial = React.useMemo( + const observeOptions: Required = React.useMemo( () => ({ overflowAxis, overflowDirection, From a5256a2551468d0281d459dde7adcea80c7d891d Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Thu, 28 May 2026 21:41:49 +0200 Subject: [PATCH 5/7] refactor(react-overflow,priority-overflow): preserve OverflowManager API surface Restore the original observe / disconnect contract on OverflowManager so the @internal interface keeps its existing signatures. observe's options parameter becomes optional (backward-compatible widening) so the React layer can drive options through createOverflowManager and setOptions instead of re-passing them at every observation. disconnect goes back to its full reset behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../etc/priority-overflow.api.md | 4 +- .../src/overflowManager.test.ts | 19 ++++--- .../priority-overflow/src/overflowManager.ts | 53 +++++++++---------- .../priority-overflow/src/types.ts | 41 +++++++------- .../library/src/useOverflowContainer.test.tsx | 16 +++--- .../library/src/useOverflowContainer.ts | 3 +- 6 files changed, 69 insertions(+), 67 deletions(-) diff --git a/packages/react-components/priority-overflow/etc/priority-overflow.api.md b/packages/react-components/priority-overflow/etc/priority-overflow.api.md index 73ac72034be45..2a720d068daf9 100644 --- a/packages/react-components/priority-overflow/etc/priority-overflow.api.md +++ b/packages/react-components/priority-overflow/etc/priority-overflow.api.md @@ -64,11 +64,11 @@ export interface OverflowItemEntry { // @internal export interface OverflowManager { addDivider: (divider: OverflowDividerEntry) => void; - addItem: (item: OverflowItemEntry) => void; + addItem: (items: OverflowItemEntry) => void; addOverflowMenu: (element: HTMLElement) => void; disconnect: () => void; forceUpdate: () => void; - observe: (container: HTMLElement) => () => void; + observe: (container: HTMLElement, options?: ObserveOptions) => void; removeDivider: (groupId: string) => void; removeItem: (itemId: string) => void; removeOverflowMenu: () => void; diff --git a/packages/react-components/priority-overflow/src/overflowManager.test.ts b/packages/react-components/priority-overflow/src/overflowManager.test.ts index 33caa56dc164d..9921498eddecc 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.test.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.test.ts @@ -50,7 +50,8 @@ describe('overflowManager', () => { it('should dispatch overflow update after forceUpdate', () => { const onUpdateOverflow = jest.fn(); - const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const options = createObserveOptions({ onUpdateOverflow }); + const manager = createOverflowManager(options); const container = createContainer(100); const itemA = createElementWithSize('button', 40); const itemB = createElementWithSize('button', 40); @@ -70,7 +71,8 @@ describe('overflowManager', () => { it('should re-dispatch when setOptions changes a relevant option', () => { const onUpdateOverflow = jest.fn(); - const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const options = createObserveOptions({ onUpdateOverflow }); + const manager = createOverflowManager(options); const container = createContainer(100); const itemA = createElementWithSize('button', 40); const itemB = createElementWithSize('button', 40); @@ -91,20 +93,22 @@ describe('overflowManager', () => { expect(dispatch.invisibleItems.map(item => item.id)).toEqual(['b']); }); - it('observe should return a cleanup that allows re-observation', () => { + it('disconnect stops observation and re-observe restarts dispatching', () => { const onUpdateOverflow = jest.fn(); - const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const options = createObserveOptions({ onUpdateOverflow }); + const manager = createOverflowManager(options); const container = createContainer(100); const item = createElementWithSize('button', 40); manager.addItem({ element: item, id: 'a', priority: 1 }); - const cleanup = manager.observe(container); + manager.observe(container); manager.forceUpdate(); expect(lastDispatch(onUpdateOverflow).visibleItems.map(i => i.id)).toEqual(['a']); - cleanup(); + manager.disconnect(); onUpdateOverflow.mockClear(); + manager.addItem({ element: item, id: 'a', priority: 1 }); manager.observe(container); manager.forceUpdate(); expect(onUpdateOverflow).toHaveBeenCalled(); @@ -113,7 +117,8 @@ describe('overflowManager', () => { it('should remove items through removeItem', () => { const onUpdateOverflow = jest.fn(); - const manager = createOverflowManager(createObserveOptions({ onUpdateOverflow })); + const options = createObserveOptions({ onUpdateOverflow }); + const manager = createOverflowManager(options); const container = createContainer(100); const item = createElementWithSize('button', 40); diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index 4d192e69c5bb5..bda98c4d4d519 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -239,11 +239,10 @@ export function createOverflowManager(initialOptions: Partial = } }; - let observeCleanup: () => void = () => { - /* noop */ - }; - - const observe: OverflowManager['observe'] = observedContainer => { + const observe: OverflowManager['observe'] = (observedContainer, userOptions) => { + if (userOptions) { + Object.assign(options, userOptions); + } Object.values(overflowItems).forEach(item => { if (!visibleItemQueue.contains(item.id) && !invisibleItemQueue.contains(item.id)) { visibleItemQueue.enqueue(item.id); @@ -258,32 +257,32 @@ export function createOverflowManager(initialOptions: Partial = } update(); }); + }; - const cleanup = () => { - if (container !== observedContainer) { - return; - } - disposeResizeObserver(); - disposeResizeObserver = () => { - /* noop */ - }; - container = undefined; - observing = false; - forceDispatch = true; + const disconnect: OverflowManager['disconnect'] = () => { + disposeResizeObserver(); + disposeResizeObserver = () => { + /* noop */ }; - observeCleanup = cleanup; - return cleanup; - }; + // reset flags + container = undefined; + observing = false; + forceDispatch = true; - const disconnect: OverflowManager['disconnect'] = () => observeCleanup(); + // clear all entries + Object.keys(overflowItems).forEach(itemId => removeItem(itemId)); + Object.keys(overflowDividers).forEach(dividerId => removeDivider(dividerId)); + removeOverflowMenu(); + sizeCache.clear(); + }; - const addItem: OverflowManager['addItem'] = item => { - if (overflowItems[item.id]) { + const addItem: OverflowManager['addItem'] = items => { + if (overflowItems[items.id]) { return; } - overflowItems[item.id] = item; + overflowItems[items.id] = items; // some options can affect priority which are only set on `observe` if (observing) { @@ -291,13 +290,13 @@ export function createOverflowManager(initialOptions: Partial = // i.e. new element is enqueued but the top of the queue stays the same // force a dispatch on the next batched update forceDispatch = true; - visibleItemQueue.enqueue(item.id); + visibleItemQueue.enqueue(items.id); update(); } - if (item.groupId) { - groupManager.addItem(item.id, item.groupId); - item.element.setAttribute(DATA_OVERFLOW_GROUP, item.groupId); + if (items.groupId) { + groupManager.addItem(items.id, items.groupId); + items.element.setAttribute(DATA_OVERFLOW_GROUP, items.groupId); } }; diff --git a/packages/react-components/priority-overflow/src/types.ts b/packages/react-components/priority-overflow/src/types.ts index 76382716a8469..51faa5a5ee4fd 100644 --- a/packages/react-components/priority-overflow/src/types.ts +++ b/packages/react-components/priority-overflow/src/types.ts @@ -156,55 +156,52 @@ export interface ObserveOptions { */ export interface OverflowManager { /** - * Updates engine options without requiring full observation re-creation. + * Starts observing the container and managing the overflow state */ - setOptions: (options: Partial) => void; + observe: (container: HTMLElement, options?: ObserveOptions) => void; /** - * Starts observing the container and managing overflow state. + * Stops observing the container */ - observe: (container: HTMLElement) => () => void; + disconnect: () => void; /** - * Adds an item to overflow tracking. + * Updates engine options without restarting observation. */ - addItem: (item: OverflowItemEntry) => void; - + setOptions: (options: Partial) => void; + /** + * Add overflow items + */ + addItem: (items: OverflowItemEntry) => void; /** - * Removes an overflow item by id. + * Remove overflow item */ removeItem: (itemId: string) => void; /** - * Schedules an asynchronous overflow recomputation. + * Manually update the overflow, updates are batched and async */ update: () => void; /** - * Forces an immediate synchronous overflow recomputation. + * Manually update the overflow sync */ forceUpdate: () => void; /** - * Attaches the overflow menu element. - * This is used to calculate available space and determine whether more items should overflow. + * Adds an element that opens an overflow menu. This is used to calculate + * available space and check if additional items need to overflow */ addOverflowMenu: (element: HTMLElement) => void; /** - * Removes the overflow menu element. - */ - removeOverflowMenu: () => void; - - /** - * Adds a divider for the provided group. + * Add overflow divider */ addDivider: (divider: OverflowDividerEntry) => void; /** - * Removes a divider by group id. + * Remove overflow divider */ removeDivider: (groupId: string) => void; /** - * Disconnects all active observers. - * Equivalent to calling the latest cleanup function returned by `observe`. + * Unsets the overflow menu element */ - disconnect: () => void; + removeOverflowMenu: () => void; } diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx index 1834065bd5e51..d6badfedd4d64 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.test.tsx @@ -13,7 +13,7 @@ const mockOverflowManager = (options: Partial = {}) => { addOverflowMenu: jest.fn(), disconnect: jest.fn(), forceUpdate: jest.fn(), - observe: jest.fn(() => () => null), + observe: jest.fn(), removeItem: jest.fn(), removeOverflowMenu: jest.fn(), update: jest.fn(), @@ -76,7 +76,7 @@ describe('useOverflowContainer', () => { }); it('should call observe with the container element', () => { - const observeMock = jest.fn(() => () => null); + const observeMock = jest.fn(); mockOverflowManager({ observe: observeMock }); const TestComponent: React.FC = () => { @@ -91,10 +91,10 @@ describe('useOverflowContainer', () => { expect(observeMock).toHaveBeenCalledWith(getByTestId('container')); }); - it('should dispose observation on unmount', () => { - const disposeMock = jest.fn(); - const observeMock = jest.fn(() => disposeMock); - mockOverflowManager({ observe: observeMock }); + it('should disconnect on unmount', () => { + const disconnectMock = jest.fn(); + const observeMock = jest.fn(); + mockOverflowManager({ observe: observeMock, disconnect: disconnectMock }); const TestComponent: React.FC = () => { const { containerRef } = useOverflowContainer(() => undefined, { @@ -104,9 +104,9 @@ describe('useOverflowContainer', () => { }; const { unmount } = render(); - expect(disposeMock).not.toHaveBeenCalled(); + expect(disconnectMock).not.toHaveBeenCalled(); unmount(); - expect(disposeMock).toHaveBeenCalledTimes(1); + expect(disconnectMock).toHaveBeenCalledTimes(1); }); it('should call setOptions when options change', () => { diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index 1bc009cbd7a6c..a652cf1692ff7 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -74,7 +74,8 @@ export const useOverflowContainer = ( useIsomorphicLayoutEffect(() => { if (manager && containerRef.current) { - return manager.observe(containerRef.current); + manager.observe(containerRef.current); + return () => manager.disconnect(); } }, [manager]); From d2a62d60b24e5f7809f10b990afdb7188d41a849 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Fri, 29 May 2026 04:52:22 +0200 Subject: [PATCH 6/7] refactor(react-overflow): use ref-based lazy initializer for the overflow manager Switch useOverflowContainer's manager storage from a useMemo with empty dependencies to a useRef with an in-render lazy initializer. The ref pattern is the canonical "create once per instance" idiom documented by React for this exact use case, and it lets every dependent useCallback / useIsomorphicLayoutEffect drop manager from its deps array. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../library/src/useOverflowContainer.ts | 100 ++++++++---------- 1 file changed, 45 insertions(+), 55 deletions(-) diff --git a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts index a652cf1692ff7..54fcd01abe93a 100644 --- a/packages/react-components/react-overflow/library/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/library/src/useOverflowContainer.ts @@ -65,68 +65,58 @@ export const useOverflowContainer = ( const containerRef = React.useRef(null); - const manager = React.useMemo( - () => (canUseDOM() ? createOverflowManager(observeOptions) : null), - // Manager is created once for the component's lifetime; option changes go through setOptions. - // eslint-disable-next-line react-hooks/exhaustive-deps - [], - ); + const managerRef = React.useRef(null); + + if (managerRef.current === null) { + managerRef.current = canUseDOM() ? createOverflowManager(observeOptions) : null; + } useIsomorphicLayoutEffect(() => { - if (manager && containerRef.current) { - manager.observe(containerRef.current); - return () => manager.disconnect(); + if (managerRef.current && containerRef.current) { + managerRef.current.observe(containerRef.current); + return () => managerRef.current?.disconnect(); } - }, [manager]); + }, []); useIsomorphicLayoutEffect(() => { - manager?.setOptions(observeOptions); - }, [observeOptions, manager]); - - const registerItem = React.useCallback( - (item: OverflowItemEntry) => { - manager?.addItem(item); - item.element.setAttribute(DATA_OVERFLOW_ITEM, ''); - - return () => { - item.element.removeAttribute(DATA_OVERFLOWING); - item.element.removeAttribute(DATA_OVERFLOW_ITEM); - manager?.removeItem(item.id); - }; - }, - [manager], - ); - - const registerDivider = React.useCallback( - (divider: OverflowDividerEntry) => { - const el = divider.element; - manager?.addDivider(divider); - el.setAttribute(DATA_OVERFLOW_DIVIDER, ''); - - return () => { - manager?.removeDivider(divider.groupId); - el.removeAttribute(DATA_OVERFLOW_DIVIDER); - }; - }, - [manager], - ); - - const registerOverflowMenu = React.useCallback( - (el: HTMLElement) => { - manager?.addOverflowMenu(el); - el.setAttribute(DATA_OVERFLOW_MENU, ''); - - return () => { - manager?.removeOverflowMenu(); - el.removeAttribute(DATA_OVERFLOW_MENU); - }; - }, - [manager], - ); + managerRef.current?.setOptions(observeOptions); + }, [observeOptions]); + + const registerItem = React.useCallback((item: OverflowItemEntry) => { + managerRef.current?.addItem(item); + item.element.setAttribute(DATA_OVERFLOW_ITEM, ''); + + return () => { + item.element.removeAttribute(DATA_OVERFLOWING); + item.element.removeAttribute(DATA_OVERFLOW_ITEM); + managerRef.current?.removeItem(item.id); + }; + }, []); + + const registerDivider = React.useCallback((divider: OverflowDividerEntry) => { + const el = divider.element; + managerRef.current?.addDivider(divider); + el.setAttribute(DATA_OVERFLOW_DIVIDER, ''); + + return () => { + managerRef.current?.removeDivider(divider.groupId); + el.removeAttribute(DATA_OVERFLOW_DIVIDER); + }; + }, []); + + const registerOverflowMenu = React.useCallback((el: HTMLElement) => { + managerRef.current?.addOverflowMenu(el); + el.setAttribute(DATA_OVERFLOW_MENU, ''); + + return () => { + managerRef.current?.removeOverflowMenu(); + el.removeAttribute(DATA_OVERFLOW_MENU); + }; + }, []); const updateOverflow = React.useCallback(() => { - manager?.update(); - }, [manager]); + managerRef.current?.update(); + }, []); return { registerItem, From b14ca914c577998efed9c6b0937c7304209c1487 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Fri, 29 May 2026 06:06:56 +0200 Subject: [PATCH 7/7] test(priority-overflow): guard setOptions partial input against false-positive dispatch Adds a regression test covering setOptions called with a partial whose values match the manager's current state. The previous-capture comparison continues to short-circuit correctly; this test pins that behaviour so a future refactor that compares the live options against the partial directly cannot silently regress into spurious dispatches. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/overflowManager.test.ts | 17 ++++++++++++++++ .../priority-overflow/src/overflowManager.ts | 20 ++++++++----------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/react-components/priority-overflow/src/overflowManager.test.ts b/packages/react-components/priority-overflow/src/overflowManager.test.ts index 9921498eddecc..d937f4615f9ff 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.test.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.test.ts @@ -93,6 +93,23 @@ describe('overflowManager', () => { expect(dispatch.invisibleItems.map(item => item.id)).toEqual(['b']); }); + it('should not re-dispatch when setOptions is called with a partial that does not change anything', () => { + const onUpdateOverflow = jest.fn(); + const options = createObserveOptions({ onUpdateOverflow }); + const manager = createOverflowManager(options); + const container = createContainer(100); + const itemA = createElementWithSize('button', 40); + + manager.addItem({ element: itemA, id: 'a', priority: 1 }); + manager.observe(container, options); + manager.forceUpdate(); + + onUpdateOverflow.mockClear(); + manager.setOptions({ padding: 10 }); // padding is already 10; no real change + + expect(onUpdateOverflow).not.toHaveBeenCalled(); + }); + it('disconnect stops observation and re-observe restarts dispatching', () => { const onUpdateOverflow = jest.fn(); const options = createObserveOptions({ onUpdateOverflow }); diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index bda98c4d4d519..c41f43617cb09 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -219,21 +219,17 @@ export function createOverflowManager(initialOptions: Partial = if (options === nextOptions) { return; } - const previousAxis = options.overflowAxis; - const previousDirection = options.overflowDirection; - const previousPadding = options.padding; - const previousMinimumVisible = options.minimumVisible; - const previousHasHiddenItems = options.hasHiddenItems; + + const shouldTriggerUpdate = + (nextOptions.overflowAxis && options.overflowAxis !== nextOptions.overflowAxis) || + (nextOptions.overflowDirection && options.overflowDirection !== nextOptions.overflowDirection) || + (nextOptions.padding && options.padding !== nextOptions.padding) || + (nextOptions.minimumVisible && options.minimumVisible !== nextOptions.minimumVisible) || + (nextOptions.hasHiddenItems && options.hasHiddenItems !== nextOptions.hasHiddenItems); Object.assign(options, nextOptions); - if ( - previousAxis !== options.overflowAxis || - previousDirection !== options.overflowDirection || - previousPadding !== options.padding || - previousMinimumVisible !== options.minimumVisible || - previousHasHiddenItems !== options.hasHiddenItems - ) { + if (shouldTriggerUpdate) { forceDispatch = true; update(); }