fix: increase pool only after a few elements have been rendered

[vursen]

Description

This PR limits the scope of the __preventElementUpdates flag during size updates to only when scroll restoration is needed (the scroll position is not at the top). Before, when a non-zero size was set for the first time, the virtualizer created an initial batch of elements, but since the flag prevented their content from rendering, their heights didn't reflect the final values and instead defaulted to the minimum height set via CSS. As a result, the virtualizer could overestimate how many more elements it needed to add to fill the remaining viewport.

This change improves Aura performance in the grid benchmark by 15%, narrowing the gap with Lumo to just 9%:

+ mixed-rendertime-chrome: performance improved by 15%

And surprisingly, it also has a positive impact on Lumo itself:

+ mixed-rendertime-chrome: performance improved by 10%

[tomivirkki]

Hmm...does this test change kind of reveal a regression that renderer calls are doubled?

Should there be a (virtualizer) test that validates the fix for "it caused the virtualizer to create more virtual elements than needed"?

[vursen]

Hmm...does this test change kind of reveal a regression that renderer calls are doubled?

Yes, it's a small regression caused by the way iron-list is implemented: it always re-renders all rows after the physical count is increased. With the proposed change, the first increase happens in itemsChanged and the second in the subsequent scrollToIndex. We can't avoid two increases because we need the sizes of the initial batch of elements to estimate how many more are needed to fill the viewport. Optimizing the second increase to not re-render the already measured elements would require more changes, and I didn't want to go there yet :)

Should there be a (virtualizer) test that validates the fix for "it caused the virtualizer to create more virtual elements than needed"?

Yeah, I think I could create a virtualizer test.

[vursen]

Yeah, I think I could create a virtualizer test.

Improved the existing test to catch the issue.

[tomivirkki]

I traced down the PR which added __preventElementUpdates flag and it's #2801. The change was needed to support infinite scrolling in Flow Grid.

The PR also included two tests but for some reason they no longer exist in virtualizer.test.js (!)

I adapted the test from the PR for the current virtualizer logic and it passes on main:

it('should not request a different set of items on size increase', async () => {
  const updateElement = sinon.spy((el, index) => {
    el.textContent = `item-${index}`;
  });
  init({ size: 100, updateElement });

  // Scroll halfway down the list
  updateElement.resetHistory();
  virtualizer.scrollToIndex(50);
  await aTimeout(100);
  const updatedIndexes = updateElement.getCalls().map((call) => call.args[1]);

  // Increase the size so it shouldn't affect the current viewport items
  updateElement.resetHistory();
  virtualizer.size = 200;
  const postResizeUpdatedIndexes = updateElement.getCalls().map((call) => call.args[1]);

  if (postResizeUpdatedIndexes.length > 0) {
    expect(postResizeUpdatedIndexes).to.eql(updatedIndexes);
  }
  expect(postResizeUpdatedIndexes).not.to.include(0);
});

But with the changes of this PR, virtualizer would again render elements at unexpected indexed on size increase and the test would fail.

[vursen]

Thanks! I've updated the solution so that it now focuses on initial render only.

[tomivirkki]

The test case mentioned in the other comment was used for quick local testing and isn't really polished. For example it has await aTimeout(100); which makes it flaky.

We should probably instead restore this newer version that got removed in https://github.com/vaadin/web-components/pull/7205/changes#diff-17e7a20b2bf83191f4deeb3c213c8d0023dd076e4adb3da00f3d78ad00dcb3cfL187

it('should not request item updates on size increase', () => {
  const updateElement = sinon.spy((el, index) => {
    el.textContent = `item-${index}`;
  });
  init({ size: 100, updateElement });

  // Scroll halfway down the list
  virtualizer.scrollToIndex(50);
  virtualizer.flush();
  updateElement.resetHistory();

  // Increase the size so it shouldn't affect the current viewport items
  virtualizer.size = 200;
  virtualizer.flush();

  expect(updateElement.called).to.be.false;
});

[vursen]

Done.

[tomivirkki]

This causes a minor regression in that it may lose the scroll position when for example the grid's size increases

Before:

Kapture.2026-02-27.at.12.51.00.mp4

After:

Kapture.2026-02-27.at.12.52.31.mp4

[tomivirkki]

If we're only targeting the initial render, maybe we could use some condition other than fvi > 0, maybe isClientFull

[vursen]

Right... I've added a check for fviOffsetBefore < 0, and this seems to work now

[tomivirkki]

I guess we only need to care about scrollTop? const shouldRestoreScrollPosition = this._scrollTop > 0;

[vursen]

Good point! Done.

[sonarqubecloud]

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

[vaadin-bot]

Hi @vursen and @vursen, when i performed cherry-pick to this commit to 24.9, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 2e96227
error: could not apply 2e96227... fix: increase pool only after a few elements have been rendered (#11196)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

[vaadin-bot]

This ticket/PR has been released with Vaadin 25.1.0-beta2.