refactor: share pendingRequests between connector and controller by DiegoCardoso · Pull Request #9238 · vaadin/flow-components

DiegoCardoso · 2026-05-07T06:58:06Z

The combo-box connector tracked pending page-load callbacks in a private pageCallbacks map, separate from the data-provider controller's own rootCache.pendingRequests. The two diverged in lifetime — pageCallbacks accumulated entries until cleared explicitly, while the controller's map shrank as responses arrived.

A scroll-to-index call (e.g. via the new scrollToIndex web-component API or any programmatic jump) lands on a page index that's non-contiguous with whatever pages already populated pageCallbacks, which the connector's "active range" check classifies as a stale gap and clears instead of issuing the fetch. Subsequent lazy callbacks never resolve and loading stays true.

The connector now reads pendingRequests directly from the controller (single source of truth). On top of that, the request path is rewritten to mirror gridConnector:

requestPage derives the fetch range from the virtualizer's visible viewport ± pageSize, so a single-page scroll keeps the previously-loaded page in the server's active range and a fast scroll past several pages only issues one request for the final viewport.
lastRequestedRange deduplicates back-to-back requests for the same range.
Filter changes go through a 500 ms debouncer on comboBox._filterDebouncer (with needsDataCommunicatorReset to force a re-emit on A→B→A cycles within the debounce window). The filter guard in $connector.set / $connector.confirm compares against the latest lastFilter so a stale-filter response is dropped immediately rather than briefly painting old data.
$connector.clear stamps placeholders into filteredItems for the cleared positions. When the user scrolls back into a range the server has evicted, the virtualizer re-fetches via the placeholders rather than rendering stale items (this is required for ComponentRenderer items, which carry nodeId references that the server invalidates when items leave the active range).
When dataProvider is called for a page whose data is already in the connector-local cache (from a buffer prefetch), the callback is committed synchronously — no server round-trip.
getViewportRange defaults the virtualizer indices to 0 when undefined/NaN, and the size clamp falls back to +Infinity when comboBox.size is undefined. Without these guards the very first request after openPopup sent length=null to the server, the server returned empty, the virtualizer re-fired dataProvider once the size arrived, and the user saw two server round-trips per action and a brief loading-spinner flicker.

Warning

Behavior change: the connector's overflow-eviction heuristic (active range * pageSize > 500) no longer fires, because the shared pendingRequests only holds in-flight pages instead of growing with every loaded page. Loaded pages stay cached for the lifetime of the active range; the server still drops items outside that range, which the connector now propagates to filteredItems as placeholders so the virtualizer re-fetches on scroll-back. Memory grows with how far the user scrolls (~50 items per page), matching Grid's behavior.

Tests:

combo-box-connector.test.ts: new pending requests describe block (4 tests) covering the shared map, the post-debounce callback, and the stale-callback replacement on filter change. shared.ts extended with the matching connector-side typings.
LazyLoadingIT.openPopup_setDisabled_removeDisabledAttribute_noNewDataLoaded: expected loaded count updated from 50 to 100. Initial open now prefetches one extra page beyond the viewport.
LazyLoadingIT.setComponentRenderer_scrollDown_itemsRendered: minor formatting changes; the scroll-up step still verifies the new renderer is applied at the top after server-side eviction + re-fetch.
ItemCountUnknownComboBoxIT: RangeLog indices and a couple of ranges updated to match the new single-request-per-scroll fetch sequence; one assertion split to reflect that revisiting a previously-visited page no longer re-fetches it when it's still inside the active range.
AbstractItemCountComboBoxIT.verifyFetchForUndefinedItemCountCallback: wraps the log-element lookup in a try/catch so a missing log produces a clear failure message instead of NoSuchElementException.

🤖 Generated with Claude Code

DiegoCardoso · 2026-05-13T15:19:32Z

+            try {
+                WebElement log = findElement(By.id("log-" + index));
+                Assert.assertEquals("Invalid range for index " + index,
+                        index + ":" + rangeLog.getRange().toString(),
+                        log.getText());
+            } catch (NoSuchElementException e) {
+                Assert.fail("Log element not found for index " + index
+                        + ", expected range: "
+                        + rangeLog.getRange().toString());
+            }


This is purely to give a better feedback in case the log-{N} element is not found.

The combo-box connector tracked pending page-load callbacks in a private pageCallbacks map, separate from the data-provider controller's own rootCache.pendingRequests. The two diverged in lifetime — pageCallbacks accumulated entries until cleared explicitly, while the controller's map shrank as responses arrived. Splits across these two maps caused the connector's "non-sequential range" check to misclassify a fresh single-page fetch as a stale gap and trigger clearPageCallbacks instead of serverFacade.requestData, leaving lazy callbacks unresolved on later scrolls. Replace pageCallbacks with getPendingRequests() reading the controller's own map. The connector and controller now share a single source of truth for in-flight page requests. Behavior change: the connector's overflow-eviction heuristic (active range * pageSize > 500) no longer fires, because the shared pendingRequests is small (only in-flight) instead of growing with every loaded page. Loaded pages stay cached for the lifetime of the combo-box session. Memory grows with how far the user scrolls (~50 items per page), matching Grid's behavior. Trade-off: scrolling back no longer triggers a re-fetch (spinner-free), at the cost of some retained items. Tests: - combo-box-connector.test.ts: new "pending requests" describe block exercising the shared map (4 tests). - ComboBoxClientSideDataRangeIT: rewritten to drop the eviction assertions; verifies pages load cumulatively. - ItemCountUnknownComboBoxIT: updated RangeLog indices to match the new fetch sequence (each scroll requests just the target page rather than a wide range). - ComponentRendererIT.multiplePagesOfItems_scrollDown_close_*: now asserts items are immediately rendered on reopen (cache hit) rather than blank during a fresh fetch. - LazyLoadingIT.setComponentRenderer_scrollDown_*: dropped the scroll-up step and renamed; the original test relied on eviction-driven scroll reset to verify the new renderer applies at the top. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The exact order/index of intermediate fetches the connector issues during a scroll-to-N call varies with viewport height and scroll timing across environments (intermediate prefetches appear or not depending on the browser). Asserting log-N == specific-range was flaky in CI. Verify the expected ranges appear somewhere in the server fetch log instead. The test purpose (the right pages were fetched in response to user scrolls) is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Use DataProviderController type in shared.ts instead of an inline literal mirroring its shape. - Drop the redundant delete in clearPageCallbacks: invoking the callback already removes the entry from pendingRequests via the controller's own callback. - Simplify $connector.confirm with Object.entries().forEach. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Tracks active pages in the connector with a `Set` separate from the controller's `pendingRequests` (which is reset on commit) so the eviction heuristic survives committed pages. Without it, scrolling away from a page leaves the server's per-item Flow components passivated while the client still holds the items, and on scroll-back the renderer follows stale `nodeId` references that point at destroyed components. Also reverts the test updates that assumed no eviction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…back" This reverts commit 2a53de9.

…improved clarity and functionality

…drain The dataProvider had its `_clientSideFilter` short-circuit removed during the refactor, breaking small-dataset client-side filtering — every typed filter now hit the 500ms server debounce and ended up wiping the cache. Restored the short-circuit so subsequent dataProvider calls filter `cache[0]` locally without server roundtrip. `$connector.clear` also stamped placeholders into `filteredItems` for every cleared page, breaking the connector's cumulative caching: scroll forward + back showed only the latest page rather than the full history ComboBoxClientSideDataRangeIT expects. Reverted to plain `delete cache[i]`, keeping `filteredItems` intact across server-driven drops. Filter-change drain now runs without calling stale callbacks (the controller's own `#loadCachePage` already overwrites pendingRequests with the new wrapper). WTR test updated accordingly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The previous fix removed placeholder-stamping in $connector.clear, which broke LazyLoadingIT.setComponentRenderer_scrollDown_itemsRendered: items out of the server's active range kept stale Flow nodeId references and re-rendered with the new ComponentRenderer pointed at destroyed components. Re-stamp placeholders on server-emitted clear so the virtualizer triggers a fresh fetch on scroll-back (the ensureFlatIndexLoaded path only fires on placeholders). The 150ms request debouncer also masked the in-flight setViewportRange RPC from TestBench's waitForVaadin: the test asserts after one requestAnimationFrame, before the timer macrotask runs. The debouncer isn't load-bearing for combo-box scrolling (smaller viewport than grid) so call requestPage directly. Drop hasData while there. Widen the prefetch buffer to at least pageSize so a single-page scroll keeps the previously-loaded page within the server's active range — otherwise the server emits clear() for it and placeholder-stamping trims loaded items count back to one viewport's worth. Update ComboBoxClientSideDataRangeIT to the bounded-caching shape introduced by the reverted b908b4f (matches the new prefetch eviction semantics). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

At the first dataProvider call after openPopup, comboBox.size can be undefined and the virtualizer's firstVisibleIndex/lastVisibleIndex can be NaN (set before the first row renders). Math.min(50, undefined) is NaN, which propagated through the range math and made setViewportRange receive length=null. The server treated it as 0 length, returned no items, and the virtualizer triggered a second dataProvider call once items rendered — visible to the user as the loading spinner flickering (appear → quickly disappear → appear again) and every interaction taking two server round-trips instead of one. Default size to +Infinity when not finite, and substitute 0 for NaN/undefined virtualizer indices. Each open or filter change now makes a single setViewportRange call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

When buffer-prefetch fetches multiple pages per server call, only the page the controller explicitly requested gets a pendingRequests entry and is committed via confirm(). Pages beyond that are stored as "orphan" entries in the connector-local cache. When the user later scrolls into one of those pages, the new dataProvider call would find the same lastRequestedRange and skip the server roundtrip, leaving the page-pending callback unresolved and the items rendered as placeholders. If cache already has the requested page, commit it synchronously and skip the server call. This makes scroll-driven reveals of prefetched pages instantaneous without extra server traffic. Tests updated: - ComboBoxClientSideDataRangeIT: replace the bounded-cumulative assertions with simpler "items render + count stays within buffer" checks. The connector caches viewport ± pageSize, not full history. - ComponentRendererIT.multiplePagesOfItems_scrollDown_close_*: reopening now re-fetches the visible page (placeholder-stamping on server-driven clear), so wait for items to render before asserting rather than reading the first DOM node immediately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Mirror gridConnector by computing the viewport buffer from the scroller's rendered children (overscan window) instead of the visible-only virtualizer indices, and drop the pageSize floor on the buffer. The previous floor inflated the active range to 3+ pages for large pageSize. Update ITs to match: - ComboBoxClientSideDataRangeIT: replace cumulative-loading assertions with a bound-only check (loaded count stays within ~2 pages). - LazyLoadingIT: restore main's expectation after scrolling a disabled ComboBox (50, not 100). - ItemCountUnknownComboBoxIT: shift RangeLog indices back closer to main now that fewer fetches happen per scroll. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

After the connector stopped writing to pendingRequests in dataProvider (912faae), the existing tests asserted state that only the controller's loadCachePage populates. The failing assertions caused chai's formatter to choke on sinon spy values, so mocha never reported and WTR hit its 120s testsFinishTimeout. Rewrite the pending requests describe to drive loadFirstPage and assert on observable controller state, and stub confirmUpdate / resetDataCommunicator on the $server spy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Without the truthy guard around the placeholder write, clear would unconditionally set comboBox.filteredItems[index] = placeHolder for the full requested range, extending the array beyond cache.size. After a filter that shrinks size, that left phantom placeholders past the new size, which the virtualizer kept requesting — creating a pending entry the server never resolved, so the dropdown stayed in loading forever. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

In $connector.confirm, the client-side-filter branch gated performClientSideFilter on page === 0, but Object.entries returns string keys — so the comparison was always false and the callback fired with empty data. Filtering a small dataset (CustomValueIT) silently dropped all matches. Swap the gate for an items-presence check, which also guards performClientSideFilter against undefined input. Update FilteringIT.verifyFilteredItems and LazyLoadingIT to assert a bounded loaded-items count instead of the pre-buffer-fix cumulative total — the active range now stays within ~2 pages by design. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

sonarqubecloud · 2026-05-13T18:13:20Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

DiegoCardoso mentioned this pull request May 7, 2026

feat: add setFocusSelectedItem to ComboBox #9239

Open

vursen reviewed May 7, 2026

View reviewed changes

Comment thread vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/test/shared.ts Outdated

vursen reviewed May 7, 2026

View reviewed changes

Comment thread ...nt/vaadin-combo-box-flow/src/main/resources/META-INF/resources/frontend/comboBoxConnector.js Outdated

vursen reviewed May 7, 2026

View reviewed changes

Comment thread ...nt/vaadin-combo-box-flow/src/main/resources/META-INF/resources/frontend/comboBoxConnector.js Outdated

vursen reviewed May 7, 2026

View reviewed changes

Comment thread ...c/test/java/com/vaadin/flow/component/combobox/test/dataview/ItemCountUnknownComboBoxIT.java Outdated

DiegoCardoso force-pushed the refactor/combo-box-connector-pending-requests branch from b467eff to d2e2d62 Compare May 7, 2026 13:37

vaadin deleted a comment from github-actions Bot May 7, 2026

DiegoCardoso commented May 7, 2026

View reviewed changes

Comment thread ...w-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/LazyLoadingIT.java Outdated

DiegoCardoso force-pushed the refactor/combo-box-connector-pending-requests branch from 76cbd53 to 5cbf58f Compare May 11, 2026 15:29

vaadin deleted a comment from github-actions Bot May 12, 2026

DiegoCardoso force-pushed the refactor/combo-box-connector-pending-requests branch from 27dfc71 to 44d8577 Compare May 12, 2026 15:44

vaadin deleted a comment from github-actions Bot May 12, 2026

DiegoCardoso commented May 13, 2026

View reviewed changes

DiegoCardoso force-pushed the refactor/combo-box-connector-pending-requests branch 2 times, most recently from 4af3d8c to be5e60c Compare May 13, 2026 15:24

DiegoCardoso and others added 15 commits May 13, 2026 18:39

test: reduce changes in ItemCountUnknownComboBoxIT file

65ca780

Revert "fix: preserve cache eviction to avoid stale nodeId on scroll-…

95c7d33

…back" This reverts commit 2a53de9.

refactor: make dataProvider work similar to gridConnector's

5c1994b

test: revert changes in LazyLoadingIT

3b22a1c

refactor: replace filterDebouncer with comboBox._filterDebouncer for …

17064d4

…improved clarity and functionality

chore: spotless apply

abcc97d

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test: adjust assertions

4308933

DiegoCardoso and others added 5 commits May 13, 2026 18:39

test: restore some assertions in ItemCountUnknownComboBoxIT

06463d0

test: undo changes in ComponentRendererIT file

bb6330d

refactor: simplify pending requests handling in combo-box connector

9f9707d

DiegoCardoso force-pushed the refactor/combo-box-connector-pending-requests branch from 490f9e6 to 43ca860 Compare May 13, 2026 16:40

test: revert changes in LazyLoadingIT

fea69b9

DiegoCardoso force-pushed the refactor/combo-box-connector-pending-requests branch from 43ca860 to fea69b9 Compare May 13, 2026 16:41

DiegoCardoso and others added 2 commits May 13, 2026 19:11

This was referenced May 14, 2026

feat: add focusSelectedItem flag to combo-box #9194

Closed

fix: preserve committed pages in connector range management in combo-box #9220

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: share pendingRequests between connector and controller#9238

refactor: share pendingRequests between connector and controller#9238
DiegoCardoso wants to merge 23 commits into
mainfrom
refactor/combo-box-connector-pending-requests

DiegoCardoso commented May 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DiegoCardoso May 13, 2026

Uh oh!

sonarqubecloud Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

DiegoCardoso commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DiegoCardoso May 13, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented May 13, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DiegoCardoso commented May 7, 2026 •

edited

Loading