refactor(react-overflow,priority-overflow): subscribe model removes intermediate state from <Overflow>#36263
Open
bsunderhus wants to merge 3 commits into
Open
Conversation
5 tasks
📊 Bundle size reportUnchanged fixtures
|
|
Pull request demo site: URL |
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Charts-DonutChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Charts-DonutChart.Dynamic.default.chromium.png | 33361 | Changed |
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 615 | Changed |
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 749 | Changed |
vr-tests-react-components/ProgressBar converged 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - High Contrast.default.chromium.png | 41 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png | 121 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - Dark Mode.default.chromium.png | 49 | Changed |
vr-tests-react-components/TagPicker 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - RTL.chromium.png | 635 | Changed |
| vr-tests-react-components/TagPicker.disabled.chromium.png | 677 | Changed |
There were 2 duplicate changes discarded. Check the build logs for more information.
61974e9 to
ef9fd48
Compare
7 tasks
…ntermediate state from Overflow container The Overflow container previously held the overflow snapshot in component state and re-rendered every consumer through context selector when it changed. This forced consumers to opt out via memoization and made first-paint sequencing fragile. Move the snapshot into the manager and let each hook subscribe directly. priority-overflow: - the manager caches the latest snapshot and exposes getSnapshot/subscribe - dispatchOverflowUpdate goes through takeSnapshot, which fans out to listeners - the observe cleanup resets the snapshot so subscribers see an empty state react-overflow: - OverflowContextValue exposes getSnapshot/subscribe directly from the manager - the context drops react-context-selector and uses plain React.createContext; useOverflowContext keeps the selector overload for backward compatibility - the existing itemVisibility, groupVisibility and hasOverflow fields on the context are kept but marked deprecated — they are now always empty - new useOverflowSnapshot hook subscribes via useState + useIsomorphicLayoutEffect - useOverflowCount, useIsOverflowItemVisible, useIsOverflowGroupVisible and useOverflowVisibility are rewritten on top of useOverflowSnapshot - Overflow.tsx no longer keeps a useState; onOverflowChange callers receive the same OnOverflowChangeData shape derived from the snapshot - drop the @fluentui/react-context-selector dependency from the package Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- overflowContext.ts: replace the inline `() => () => null` / `() => null` default no-ops on the context value with references to a local `noop` const (block-form body). - useOverflowContainer.ts: `defaultSubscribe` now returns the same shared `noop` rather than re-allocating a fresh placeholder per call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ef9fd48 to
de5968f
Compare
bsunderhus
commented
May 29, 2026
|
|
||
| // @internal (undocumented) | ||
| export interface UseOverflowContainerReturn<TElement extends HTMLElement> extends Pick<OverflowContextValue, 'registerItem' | 'updateOverflow' | 'registerOverflowMenu' | 'registerDivider'> { | ||
| export interface UseOverflowContainerReturn<TElement extends HTMLElement> extends Pick<OverflowContextValue, 'registerItem' | 'updateOverflow' | 'registerOverflowMenu' | 'registerDivider' | 'getSnapshot' | 'subscribe'> { |
bsunderhus
commented
May 29, 2026
Comment on lines
+71
to
+77
| getSnapshot: () => OverflowEventPayload; | ||
| observe: (container: HTMLElement, options?: ObserveOptions) => void; | ||
| removeDivider: (groupId: string) => void; | ||
| removeItem: (itemId: string) => void; | ||
| removeOverflowMenu: () => void; | ||
| setOptions: (options: Partial<ObserveOptions>) => void; | ||
| subscribe: (listener: () => void) => () => void; |
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
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 2 of 3. Depends on PR 1 (#36262,
react-overflow/pr1-strict-mode-lifecycle). Do not merge until PR 1 lands — the "Files changed" view here will show PR 1's diff cumulatively until then; GitHub narrows it automatically once PR 1 merges.Moves the overflow snapshot out of the
<Overflow>container and into the manager itself, so each auxiliary hook subscribes directly to the manager without forcing intermediate React state.Stack
react-overflow/pr1-strict-mode-lifecycle(refactor(react-overflow,priority-overflow): pure manager + strict-mode-safe lifecycle #36262) — strict-mode-safe lifecycle.react-overflow/pr3-first-paint— first-paint correctness.priority-overflow
getSnapshot/subscribe.dispatchOverflowUpdategoes through a singletakeSnapshothelper that updates the cache and fans out to listeners.react-overflow
OverflowContextValueexposesgetSnapshot/subscribedirectly from the manager.@fluentui/react-context-selectorand uses plainReact.createContext;useOverflowContextkeeps the selector overload for backward compatibility.itemVisibility,groupVisibility, andhasOverflowon the context are kept as deprecated, always-empty placeholders so existing external consumers still type-check.useOverflowSnapshotsubscribes viauseState+useIsomorphicLayoutEffect.useOverflowCount,useIsOverflowItemVisible,useIsOverflowGroupVisible, anduseOverflowVisibilityare rewritten on top ofuseOverflowSnapshot.<Overflow>no longer holds auseState;onOverflowChangecallers receive the sameOnOverflowChangeDatashape, now derived from the snapshot.@fluentui/react-context-selectorremoved from package dependencies.Test plan
yarn nx run priority-overflow:test/react-overflow:testyarn nx run priority-overflow:lint/react-overflow:lintyarn nx run react-overflow:type-check