fix crash on windows when resize browser to specific width #96170
+95
−46
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,59 +1,102 @@ | ||
| diff --git a/node_modules/@shopify/flash-list/dist/recyclerview/layout-managers/LinearLayoutManager.js b/node_modules/@shopify/flash-list/dist/recyclerview/layout-managers/LinearLayoutManager.js | ||
| index 12375d9..581068e 100644 | ||
| @@ -1,4 +1,13 @@ | ||
| +import { Platform } from "react-native"; | ||
| import { RVLayoutManager, } from "./LayoutManager"; | ||
| +// Recent (rounded) cross-axis sizes kept for scrollbar oscillation detection. | ||
| +const BOUNDED_SIZE_HISTORY_LENGTH = 8; | ||
| +// Number of times the size must flip between the two values before it counts as an oscillation. | ||
| +// A resize drag sweeps through fresh values and never returns to a previous one, so it never flips. | ||
| +const MIN_OSCILLATION_FLIPS = 2; | ||
| +// Max cross-axis delta (px) treated as a scrollbar-induced oscillation rather than a real resize. | ||
| +// Classic desktop scrollbars are ~15-17px; the headroom covers thicker/zoomed scrollbars. | ||
| +const SCROLLBAR_OSCILLATION_TOLERANCE = 25; | ||
| /** | ||
| * LinearLayoutManager implementation that arranges items in a single row or column. | ||
| * Supports both horizontal and vertical layouts with dynamic item sizing. | ||
| @@ -23,9 +32,17 @@ | ||
| const prevHorizontal = this.horizontal; | ||
| super.updateLayoutParams(params); | ||
| const oldBoundedSize = this.boundedSize; | ||
| - this.boundedSize = this.horizontal | ||
| + const measuredBoundedSize = this.horizontal | ||
| ? params.windowSize.height | ||
| : params.windowSize.width; | ||
| + // On web a classic (non-overlay) scrollbar appearing/disappearing changes the measured | ||
| + // cross-axis size by the scrollbar width, which feeds back into layout and toggles the | ||
| + // scrollbar again: an infinite layout loop that crashes with "Maximum update depth exceeded". | ||
| + // Native uses overlay scrollbars and never oscillates, so this only runs on web. | ||
| + this.boundedSize = | ||
| + Platform.OS === "web" | ||
| + ? this.settleScrollbarOscillation(measuredBoundedSize) | ||
| + : measuredBoundedSize; | ||
| if (oldBoundedSize !== this.boundedSize || | ||
| prevHorizontal !== this.horizontal) { | ||
| if (this.layouts.length > 0) { | ||
| @@ -36,6 +53,66 @@ | ||
| } | ||
| } | ||
| /** | ||
| + * Web only. Breaks the scrollbar <-> layout feedback loop by pinning the cross-axis size once the | ||
| + * measured size is seen bouncing between two scrollbar-width-apart values. | ||
| + * | ||
| + * The measured cross-axis size is the scroll viewport's *client* width, which excludes a classic | ||
| + * scrollbar. So while the loop runs it can only ever report two values: with-scrollbar and | ||
| + * without. We wait until the history shows exactly those two values, no further apart than a | ||
| + * scrollbar, having flipped at least twice, and then settle on the smaller one - it already | ||
| + * accounts for the scrollbar, so items never overflow the client width. | ||
| + * | ||
| + * Detection deliberately does not assume the flips alternate on every single pass: the scrollbar | ||
| + * only toggles once the re-rendered items have been re-measured, which can lag the size change by | ||
| + * a pass, producing runs like A,A,B,B,A,A. Requiring strict A,B,A,B alternation misses those and | ||
| + * lets the loop run to the crash. | ||
| + * | ||
| + * A real resize is never misread: dragging sweeps through many distinct sizes (more than two), so | ||
| + * it cannot match. Once settled, any size outside the oscillating pair means a genuine layout | ||
| + * change, which releases the pin and restarts detection. | ||
| + * @param measuredBoundedSize Cross-axis size just measured from the DOM | ||
| + * @returns The size to lay out with | ||
| + */ | ||
| + settleScrollbarOscillation(measuredBoundedSize) { | ||
| + var _a; | ||
| + // Rounded, so the comparisons below are robust against subpixel drift. | ||
| + const size = Math.round(measuredBoundedSize); | ||
| + const settledPair = this.scrollbarOscillationPair; | ||
| + if (settledPair) { | ||
| + if (size === settledPair[0] || size === settledPair[1]) { | ||
| + return settledPair[0]; | ||
|
Comment on lines
+65
to
+67
There was a problem hiding this comment.
In web classic-scrollbar lists after an oscillation is detected, this branch returns the smaller width for both members of the latched pair. If the list contents later shrink or row wrapping changes so the scrollbar legitimately disappears without a window resize, the DOM will keep measuring the larger member of the pair, but Useful? React with 👍 / 👎. |
||
| + } | ||
| + // Outside the pair: a real layout change. Forget the oscillation and start over. | ||
| + this.scrollbarOscillationPair = undefined; | ||
| + this.recentBoundedSizes = undefined; | ||
| + } | ||
| + const history = ((_a = this.recentBoundedSizes) !== null && _a !== void 0 ? _a : (this.recentBoundedSizes = [])); | ||
| + history.push(size); | ||
| + if (history.length > BOUNDED_SIZE_HISTORY_LENGTH) { | ||
| + history.shift(); | ||
| + } | ||
| + const distinctSizes = Array.from(new Set(history)); | ||
| + if (distinctSizes.length !== 2 || | ||
| + Math.abs(distinctSizes[0] - distinctSizes[1]) > SCROLLBAR_OSCILLATION_TOLERANCE) { | ||
| + return measuredBoundedSize; | ||
| + } | ||
| + let flips = 0; | ||
| + for (let i = 1; i < history.length; i++) { | ||
| + if (history[i] !== history[i - 1]) { | ||
| + flips++; | ||
| + } | ||
| + } | ||
| + if (flips < MIN_OSCILLATION_FLIPS) { | ||
| + return measuredBoundedSize; | ||
| + } | ||
| + const smaller = Math.min(distinctSizes[0], distinctSizes[1]); | ||
| + this.scrollbarOscillationPair = [ | ||
| + smaller, | ||
| + Math.max(distinctSizes[0], distinctSizes[1]), | ||
| + ]; | ||
| + return smaller; | ||
| + } | ||
| + /** | ||
| * Processes layout information for items, updating their dimensions. | ||
| * For horizontal layouts, also normalizes heights of items. | ||
| * @param layoutInfo Array of layout information for items | ||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With
MIN_OSCILLATION_FLIPS = 2, the detector latches after a singleA -> B -> Aexcursion. On web with classic scrollbars, a transient content/filter/loading change can make the scrollbar appear once and then disappear without any oscillation; this now createsscrollbarOscillationPairand immediately pins the current no-scrollbar measurement to the smaller scrollbar width until some third width is observed. Requiring at least one more flip, or otherwise distinguishing one-off scrollbar changes from repeated ping-pong, would avoid leaving these lists laid out as if a scrollbar still exists.Useful? React with 👍 / 👎.