refactor(react-overflow,priority-overflow): pure manager + strict-mode-safe lifecycle#36262
Merged
bsunderhus merged 7 commits intoMay 29, 2026
Conversation
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…e 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>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bsunderhus
force-pushed
the
react-overflow/pr1-strict-mode-lifecycle
branch
from
538ecc0 to
ae93c45
Compare
- 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
…flow 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) <noreply@anthropic.com>
…-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) <noreply@anthropic.com>
layershifter
approved these changes
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 1 of 3 in a stack that refactors the priority-overflow manager and the react-overflow container hook for strict-mode safety, moves the overflow snapshot out of the
<Overflow>container, and fixes the overflow snapshot on the first painted frame.Stack
react-overflow/pr2-subscribe-model— manager exposesgetSnapshot/subscribe; auxiliary hooks subscribe directly;<Overflow>drops itsuseState.react-overflow/pr3-first-paint— overflow snapshot is correct on the first painted frame.priority-overflow — additions only, no API modifications
OverflowManager's diff vs master is exclusively additions:createOverflowManager(initialOptions?: Partial<ObserveOptions>)— new optional parameter. Existing zero-arg callers still compile.observe(container, options?)—optionswidened from required to optional. Existing 2-arg callers still compile; 1-arg call now allowed for consumers that already pre-populated options.setOptions: (options: Partial<ObserveOptions>) => void— new method that updates engine options without rebuilding the manager.All other methods (
addItem,removeItem,addOverflowMenu,disconnect,forceUpdate, …) keep their master signatures and runtime behaviour, including parameter names.react-overflow
useReflazy initializer so it's instantiated once per<Overflow>instance and React renders stay pure — nouseMemo([])+eslint-disable react-hooks/exhaustive-deps.useIsomorphicLayoutEffect; the effect cleanup callsmanager.disconnect().manager.setOptions(observeOptions)in a separate layout effect, instead of rebuilding the manager.Test plan
yarn nx run priority-overflow:testyarn nx run priority-overflow:lintyarn nx run priority-overflow:type-checkyarn nx run react-overflow:testyarn nx run react-overflow:lintyarn nx run react-overflow:type-check