Feat: browser max element size#44
Conversation
Detects the browser's per-element height ceiling by binary-searching an offscreen probe div. Result is cached after the first call, with an SSR fallback when document is unavailable.
…l offset mapping When the virtual total (count * itemSize) exceeds the probed per-element height ceiling, the spacer is clamped and DOM scroll positions are linearly mapped onto the virtual range. Item offsets are translated to render relative to the current scroll so they remain visible inside the clamped spacer.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for massive lists in FixedLayoutStrategy by probing the browser's maximum element size and mapping virtual offsets to a clamped scrollable area. The implementation includes a new utility, getMaxElementSize, which uses binary search to determine the height limit. Review feedback correctly identified a potential 32-bit integer overflow when using bitwise operators for truncation on large lists, a possible null reference error if document.body is unavailable during probing, and an opportunity to optimize performance by caching redundant calculations in the item offset logic.
|
|
||
| const startIndex = clamp( | ||
| 0, | ||
| (virtualOffset / this.#itemSize) | 0, |
There was a problem hiding this comment.
Using the bitwise OR operator (| 0) for truncation is problematic for very large lists. In JavaScript, bitwise operations operate on 32-bit signed integers, meaning any value exceeding ~2.1 billion (2^31 - 1) will overflow and result in an incorrect (potentially negative) index. Since this PR specifically aims to support massive lists that exceed browser element size limits, it is highly likely that virtualOffset will exceed this range. Using Math.floor() is safe for all double-precision numbers up to Number.MAX_SAFE_INTEGER.
| (virtualOffset / this.#itemSize) | 0, | |
| Math.floor(virtualOffset / this.#itemSize), |
|0 coerces to int32 and overflows above 2^31, producing negative indices on lists where count * itemSize exceeds ~2.1B px — exactly the regime this strategy is meant to support. Math.floor is correct up to Number.MAX_SAFE_INTEGER.
When the library is imported by a script in <head>, document exists but document.body has not been parsed yet — appendChild would throw. Fall back to the SSR constant in that case without caching, so a later call after the DOM is ready still performs the real probe.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to handle lists that exceed the browser's maximum element size by probing the environment's limits and clamping the layout strategy's total size. The FixedLayoutStrategy was updated to calculate virtual offsets and map them to the clamped scrollable area. Feedback highlights that the strategy has become stateful, which may prevent sharing instances across multiple virtualizers. Additionally, the change in getItemOffset to return relative coordinates could break features like 'scroll to index,' and a bug was identified where getTotalSize fails to update the internal item count state.
| getTotalSize(count: number): number { | ||
| return count * this.#itemSize; | ||
| return this.#clampedTotalSize(count * this.#itemSize); | ||
| } |
There was a problem hiding this comment.
The #lastCount state is not updated within getTotalSize. If getItemOffset is called after the item count changes but before the next getVisibleRange call (which can happen during data updates), it will use a stale count for its virtual total and clamping calculations. This may lead to incorrect item positioning during the re-render cycle.
| getTotalSize(count: number): number { | |
| return count * this.#itemSize; | |
| return this.#clampedTotalSize(count * this.#itemSize); | |
| } | |
| getTotalSize(count: number): number { | |
| this.#lastCount = count; | |
| return this.#clampedTotalSize(count * this.#itemSize); | |
| } |
…t/browser-max-element-size
…lation to Virtualizer Restores the original contract: - getItemOffset(index) returns absolute virtual coordinates (index * itemSize), stable across calls and safe to share a single strategy instance across multiple Virtualizers. - getTotalSize(count) returns the (possibly clamped) spacer size. - New optional getVirtualSize(count) on LayoutStrategy reports the unclamped virtual total when it differs. The Virtualizer compares the two and, when clamping is active, translates each item's virtual offset into render coordinates relative to the current scroll position. Clamped layouts skip the per-render-range memoization because item positions then depend on scrollOffset. Addresses three review notes: - statefulness blocking instance sharing - getItemOffset semantics changing with scroll - stale lastCount during data updates
|
/gemini review |
📊 Test Coverage Report (vitest)
|
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to handle extremely large lists that exceed the browser's maximum element size limit. It adds a utility to probe the browser's height limit and updates the FixedLayoutStrategy and Virtualizer to clamp the physical container size while mapping scroll offsets to a larger virtual range. The review feedback suggests addressing a potential precision loss in the virtual offset calculation, simplifying the item positioning logic in the Virtualizer, and deduplicating the offset mapping function into a shared utility to improve maintainability.
| virtualOffset = | ||
| scrollable > 0 ? (scrollOffset * virtualScrollable) / scrollable : 0; |
There was a problem hiding this comment.
The calculation (scrollOffset * virtualScrollable) / scrollable can lead to precision loss or overflow when dealing with extremely large lists, as the intermediate product may exceed Number.MAX_SAFE_INTEGER. Calculating the ratio first is safer.
| virtualOffset = | |
| scrollable > 0 ? (scrollOffset * virtualScrollable) / scrollable : 0; | |
| virtualOffset = | |
| scrollable > 0 ? (scrollOffset / scrollable) * virtualScrollable : 0; |
| const start = isClamped | ||
| ? scrollOffset + (virtualStart - virtualOffset) | ||
| : virtualStart; |
There was a problem hiding this comment.
Since virtualOffset is initialized to scrollOffset when not clamped, the formula scrollOffset + (virtualStart - virtualOffset) is mathematically equivalent to virtualStart in the non-clamped case. You can simplify the loop by using this formula unconditionally.
const start = scrollOffset + (virtualStart - virtualOffset);| export function mapToVirtualOffset( | ||
| scrollOffset: number, | ||
| viewportSize: number, | ||
| virtualTotal: number, | ||
| clampedTotal: number | ||
| ): number { | ||
| if (virtualTotal <= clampedTotal) return scrollOffset; | ||
| const scrollable = clampedTotal - viewportSize; | ||
| if (scrollable <= 0) return 0; | ||
| const ratio = scrollOffset / scrollable; | ||
| return ratio * (virtualTotal - viewportSize); | ||
| } |
There was a problem hiding this comment.
This mapping logic is duplicated in Virtualizer.ts. Consider moving mapToVirtualOffset to a shared utility file in src/utils to improve maintainability and ensure consistency across different layout strategies.
…rflow - Move mapToVirtualOffset out of FixedLayoutStrategy into src/utils so the Virtualizer and any future LayoutStrategy share one implementation. - Compute the ratio (scrollOffset / scrollable) before multiplying by the virtual scrollable range. The previous Virtualizer formula multiplied first and could overflow Number.MAX_SAFE_INTEGER on extreme lists. - Drop the if-isClamped branch in the item loop. When virtualOffset defaults to scrollOffset (non-clamped case), the formula scrollOffset + (virtualStart - virtualOffset) reduces to virtualStart, so a single expression covers both paths.
|
/gemini review |
📊 Test Coverage Report (vitest)
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for rendering extremely large lists by implementing a clamping mechanism to bypass browser-specific element size limits. It adds a getMaxElementSize utility to probe the environment's maximum height and a mapToVirtualOffset helper to translate physical scroll positions into a larger virtual range. The FixedLayoutStrategy and Virtualizer have been updated to utilize these utilities, ensuring correct item positioning and range calculation for huge datasets. Feedback was provided to enhance the robustness of the probe element's styling to prevent interference from global CSS.
1 participant
fixes #33