refactor: trigger range fetching before items reach viewport#9480
Merged
Conversation
vursen
changed the title
vursen
changed the title
vursen
changed the title
vursen
force-pushed
the
fix/grid-viewport-range-page-alignment
branch
2 times, most recently
from
a8fc44a to
f034d3e
Compare
vursen
force-pushed
the
fix/grid-viewport-range-page-alignment
branch
3 times, most recently
from
d606457 to
6b8796d
Compare
vursen
force-pushed
the
fix/grid-viewport-range-page-alignment
branch
3 times, most recently
from
84ffad6 to
b85a8d5
Compare
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`_updateScrollerItem` ran the fetch-range load whenever a column tree existed, triggering loads during non-scroll updates. Gate it on the scrolling state (`overscrollBehavior === 'auto'`) so the load only happens while the user is actually scrolling.
The web component renamed `_updateScrollerItem` to `__updateVirtualizerElement` and no longer toggles the `overscrollBehavior` style while scrolling. Hook the new method and use the scroller's `scrolling` attribute instead.
The test currently fails because the connector hooks `__updateVirtualizerElement`, which is only available in unreleased @vaadin/grid (vaadin/web-components#11914). It should pass once the dependency is updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vursen
force-pushed
the
fix/grid-viewport-range-page-alignment
branch
from
b85a8d5 to
de3c551
Compare
These tests covered behavior removed by the viewport range fix; the connector no longer reloads a range based on which part was cleared.
vursen
commented
Jun 17, 2026
Comment on lines
-191
to
-235
| describe('last requested range is not in viewport', () => { | ||
| beforeEach(async () => { | ||
| // Request a range of items further down | ||
| clear(grid.$connector, 50, 50); | ||
| grid.scrollToIndex(50); | ||
| await aTimeout(GRID_CONNECTOR_ROOT_REQUEST_DELAY); | ||
| expect(grid.$server.setViewportRange).to.have.been.calledOnceWith(30, 50); | ||
| setRootItems(grid.$connector, items, 30, 50); | ||
| grid.$server.setViewportRange.resetHistory(); | ||
| }); | ||
|
|
||
| it('should request for items if part of the last range was cleared', async () => { | ||
| // Simulate preloading of items when scrolling to top programmatically on server-side, which may also partially clear the last requested range: | ||
| // - Scroll to top | ||
| // - Clear last requested range partially | ||
| // - Preload first two pages so that grid doesn't need to request a new range yet | ||
| grid.scrollToIndex(0); | ||
| clear(grid.$connector, 40, grid.pageSize); | ||
| setRootItems(grid.$connector, items, 0, 30); | ||
| await aTimeout(GRID_CONNECTOR_ROOT_REQUEST_DELAY); | ||
| expect(grid.$server.setViewportRange).to.not.have.been.called; | ||
|
|
||
| // Scroll down again, should reload the range because part of it was cleared | ||
| grid.scrollToIndex(50); | ||
| await aTimeout(GRID_CONNECTOR_ROOT_REQUEST_DELAY); | ||
| expect(grid.$server.setViewportRange).to.have.been.calledOnceWith(30, 50); | ||
| }); | ||
|
|
||
| it('should not request for items if data outside of the last range was cleared', async () => { | ||
| // Simulate preloading of items when scrolling to top programmatically on server-side, which may also partially clear the last requested range: | ||
| // - Scroll to top | ||
| // - Clear data outside the requested range | ||
| // - Preload first two pages so that grid doesn't need to request a new range yet | ||
| grid.scrollToIndex(0); | ||
| clear(grid.$connector, 70, grid.pageSize); | ||
| grid.$connector.confirm(-1); | ||
| await aTimeout(GRID_CONNECTOR_ROOT_REQUEST_DELAY); | ||
| expect(grid.$server.setViewportRange).to.not.have.been.called; | ||
|
|
||
| // Scroll down again, should not reload the range because nothing from it was cleared | ||
| grid.scrollToIndex(50); | ||
| await aTimeout(GRID_CONNECTOR_ROOT_REQUEST_DELAY); | ||
| expect(grid.$server.setViewportRange).to.not.have.been.called; | ||
| }); | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
These tests can be removed.
They were added in #6010 to cover the sanitizeLastRequestedRange logic, which decided whether to reload a range based on which part of it had been cleared. That logic was later removed in #7896, so these tests no longer exercise any real connector behavior.
The connector now loads the cached fetch range (rendered rows + buffer) while scrolling, and that behavior is already covered in grid-connector-data-range.test.ts.
vursen
requested review from
sissbruecker and
tomivirkki
and removed request for
sissbruecker
Contributor
Author
|
@copilot resolve the merge conflicts in this pull request |
…ge-page-alignment # Conflicts: # vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/test/shared.ts
Contributor
|
sissbruecker
approved these changes
Jun 18, 2026
tomivirkki
approved these changes
Jun 18, 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
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.
The grid web component fires a page request to the data provider only when a row from that page gets rendered in the DOM. Combined with the limited size of the viewport buffer, this often leads to the grid showing blank rows even when scrolling at a relatively slow speed, since pages can't load fast enough to keep up. Then, when the response arrives, many rendered rows get updated at once, triggering a heavy synchronous DOM update that blocks the event loop and has a negative impact on FPS. This effect has become even more noticeable after vaadin/web-components#11196, which reduced the number of rendered rows outside the viewport. So while render time metrics improved after that fix, scroll frame time metrics regressed.
This PR makes the connector request data ahead of rendering while the user scrolls. On every virtualizer row update during scrolling, the connector computes the fetch range (the rendered range expanded by one viewport of buffer in both directions and aligned to page boundaries) and ensures its first and last indexes are requested from the data provider. If a boundary page is missing from the cache, this in turn triggers a server request that fetches the whole computed range. As a result, buffer pages start loading before their rows get rendered, so rows entering the viewport are much more likely to already have data, and row updates arrive in smaller batches. When the fetch range is already cached, the calls do nothing.
The prefetch only runs while the scroller has the
scrollingattribute. During other row updates (initial render, server-side data changes), requests driven by rendered rows are enough, and prefetching there would only produce redundant requests for pages the server chose not to send.Before:
Before.mov
After:
After.mov
Depends on