Skip to content

fix(virtual-core): read shouldAdjustScrollPositionOnItemSizeChange from options#1232

Closed
xianjianlf2 wants to merge 1 commit into
TanStack:mainfrom
xianjianlf2:fix/should-adjust-option-1227
Closed

fix(virtual-core): read shouldAdjustScrollPositionOnItemSizeChange from options#1232
xianjianlf2 wants to merge 1 commit into
TanStack:mainfrom
xianjianlf2:fix/should-adjust-option-1227

Conversation

@xianjianlf2

@xianjianlf2 xianjianlf2 commented Jul 18, 2026

Copy link
Copy Markdown

🎯 Changes

Closes #1227.

Problem: The documented option shouldAdjustScrollPositionOnItemSizeChange is dead code. Passing it to a virtualizer has no effect: resizeItem never sees it.

Root cause: resizeItem reads this.shouldAdjustScrollPositionOnItemSizeChange — an instance field that setOptions never assigns — and the option is missing from VirtualizerOptions entirely. The CHANGELOG (around line 110) documents the option as public API, so this is a wiring bug rather than a design change.

Fix: Adds shouldAdjustScrollPositionOnItemSizeChange to VirtualizerOptions, and makes resizeItem read it options-first with a fallback to the instance field for backward compatibility with code that assigned the field directly.

Testing: 3 new tests — overriding the default backward-scroll adjustment, the predicate receiving (item, delta, instance), and returning false suppressing the default adjustment. 122/122 vitest passing; tsc, eslint and prettier clean.

Scope note: The issue also asks to revisit the backward-skip default behavior (part 2). This PR deliberately does not touch that — it only wires the documented option so users can opt out today. The default-behavior discussion can proceed separately.

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested this code locally with pnpm run test:pr.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • New Features

    • Added a configuration option to control whether scroll position adjusts when item sizes change.
    • The callback receives the affected item, size difference, and virtualizer instance for custom behavior.
  • Bug Fixes

    • Fixed the documented scroll-adjustment option so it now takes effect.
    • Preserved existing behavior when the new option is not provided.

…om options

The predicate was documented as the escape hatch to restore pre-3.14
scroll compensation behavior, but resizeItem read it from an instance
field that setOptions never assigned, so supplying it through options
silently did nothing and there was no working way to opt out of the
backward-scroll compensation skip.

Read it from options first (and expose it on VirtualizerOptions),
falling back to a directly-assigned instance field for back-compat.

Closes TanStack#1227
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b5683389-a6c3-4b0b-a7cb-06a1ad64d546

📥 Commits

Reviewing files that changed from the base of the PR and between 1e3b908 and 2dc9532.

📒 Files selected for processing (3)
  • .changeset/wire-should-adjust-scroll-position-option.md
  • packages/virtual-core/src/index.ts
  • packages/virtual-core/tests/index.test.ts

📝 Walkthrough

Walkthrough

VirtualizerOptions now accepts shouldAdjustScrollPositionOnItemSizeChange. resizeItem reads this option with an instance-field fallback, and tests cover predicate wiring, callback arguments, override behavior, and compensation suppression.

Changes

Scroll adjustment option

Layer / File(s) Summary
Option contract and resize handling
packages/virtual-core/src/index.ts, .changeset/wire-should-adjust-scroll-position-option.md
Adds the optional predicate to VirtualizerOptions and uses it in resizeItem, falling back to the existing instance field.
Option wiring regression tests
packages/virtual-core/tests/index.test.ts
Verifies option overrides, callback arguments, and suppression of above-viewport compensation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant VirtualizerOptions
  participant resizeItem
  participant Predicate
  participant applyScrollAdjustment
  VirtualizerOptions->>resizeItem: configured predicate
  resizeItem->>Predicate: item, delta, virtualizer
  Predicate-->>resizeItem: adjustment decision
  resizeItem->>applyScrollAdjustment: delta when enabled
Loading

Possibly related PRs

Suggested reviewers: piecyk, 2wheeh

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: wiring the option from virtual-core options.
Description check ✅ Passed The PR description follows the template and includes changes, checklist, and release-impact details.
Linked Issues check ✅ Passed The change satisfies the directly relevant part of #1227 by exposing the option on VirtualizerOptions and reading it from options.
Out of Scope Changes check ✅ Passed The added tests and changeset support the main fix and do not introduce unrelated scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/virtual-core/tests/index.test.ts

Parsing error: "parserOptions.project" has been provided for @typescript-eslint/parser.
The file was not found in any of the provided project(s): packages/virtual-core/tests/index.test.ts


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@piecyk

piecyk commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

@xianjianlf2 Thanks for the PR and the thorough write-up!

One correction on framing: this isn't dead code. shouldAdjustScrollPositionOnItemSizeChange is by design a property on the virtualizer instance, not a VirtualizerOptions field — it's documented under Virtualizer Instance and is meant to be assigned imperatively

The intent behind making it an instance field (rather than an option) is that it's a rarely-used escape hatch that's convenient to set or clear imperatively from an event handler, without threading it through the options object on every render.

So the reported problem is really that passing it via options doesn't work — which is expected, since it was never an option. I'll grant that the CHANGELOG's "supply" wording is ambiguous and reads like an option, so I understand how this happened.

Given that, I'd lean toward fixing this as a documentation clarification rather than expanding the API surface.

@piecyk piecyk closed this Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Up-scroll jank from uncompensated above-viewport resizes

2 participants