Skip to content

fix(router-core): preserve pending scroll reset - #8245

Open
ousamabenyounes wants to merge 1 commit into
TanStack:mainfrom
ousamabenyounes:fix/issue-8028
Open

fix(router-core): preserve pending scroll reset#8245
ousamabenyounes wants to merge 1 commit into
TanStack:mainfrom
ousamabenyounes:fix/issue-8028

Conversation

@ousamabenyounes

@ousamabenyounes ousamabenyounes commented Sep 5, 2026

Copy link
Copy Markdown

🎯 Changes

Fixes #8028.

  • Preserve a pending page scroll reset when the destination route performs a same-path URL patch with resetScroll: false before the navigation has rendered.
  • Store the effective reset decision on the committed history state and read it during scroll restoration.
  • Keep the internal reset-scroll state out of user history-state comparisons.

Tests run:

  • RED: CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache -- tests/scroll-restoration.test.ts -t "preserves a pending page reset"
  • GREEN: CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache -- tests/scroll-restoration.test.ts -t "pending"
  • CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache -- tests/scroll-restoration.test.ts
  • CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache -- tests/build-location.test.ts
  • CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:types --outputStyle=stream --skipRemoteCache
  • CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache
  • CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:eslint --outputStyle=stream --skipRemoteCache
  • CI=1 NX_DAEMON=false pnpm test:eslint
  • CI=1 NX_DAEMON=false pnpm test:types
  • CI=1 NX_DAEMON=false pnpm test:unit

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested code changes locally with the relevant test commands, or tests do not apply to this pull request.
  • I fully understand the code in this pull request, including any code generated with AI assistance.

🚀 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

  • Bug Fixes
    • Improved scroll restoration during navigation.
    • Pending page scroll resets are now preserved when a same-path URL update occurs before the destination renders.
    • Navigation options that replace the current URL now correctly retain an existing pending scroll reset, including when the navigation does not explicitly request a reset.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: bd768b03-0028-45ca-abc0-42c079b413ed

📥 Commits

Reviewing files that changed from the base of the PR and between c18e690 and a63594e.

📒 Files selected for processing (6)
  • .changeset/pending-scroll-reset.md
  • packages/router-core/src/history.ts
  • packages/router-core/src/router.ts
  • packages/router-core/src/scroll-restoration.ts
  • packages/router-core/tests/build-location.test.ts
  • packages/router-core/tests/scroll-restoration.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The router now preserves pending scroll-reset intent across same-path URL patches. It stores the intent in history state, tracks pending restoration, applies the destination marker during rendering, and adds regression tests.

Changes

Pending scroll reset preservation

Layer / File(s) Summary
Reset-scroll state contract
packages/router-core/src/history.ts, packages/router-core/src/router.ts
Adds the resetScrollStateKey history marker, excludes it from user history state, and tracks pending scroll restoration.
Commit-location propagation
packages/router-core/src/router.ts
commitLocation inherits pending reset intent for same-path navigation, stores it in history state, and updates scroll state.
Rendered restoration and validation
packages/router-core/src/scroll-restoration.ts, packages/router-core/tests/*, .changeset/pending-scroll-reset.md
Rendered navigation reads the destination reset marker and clears pending state. Tests cover same-path patches and volatile state filtering. The changeset documents the fix.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to a6359

Pending page scroll resets are now retained through same-path URL updates without exposing router bookkeeping in user history state. Regression coverage covers the affected navigation sequence, and no merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant Navigation
  participant RouterCore
  participant History
  participant ScrollRestoration
  participant Window
  Navigation->>RouterCore: commit same-path URL patch
  RouterCore->>History: store resetScrollStateKey
  RouterCore->>ScrollRestoration: mark pending reset
  ScrollRestoration->>ScrollRestoration: read destination reset marker
  ScrollRestoration->>Window: scroll to top
Loading

Suggested reviewers: schiller-manuel, sheraff

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: preserving a pending scroll reset in router-core.
Description check ✅ Passed The description follows the repository template, explains the change and motivation, lists tests, completes the checklist, and documents the release impact and changeset.
Linked Issues check ✅ Passed The implementation addresses issue [#8028] by preserving the pending reset decision across same-path URL patches, storing the effective decision in history state, restoring it during rendering, and ex…
Out of Scope Changes check ✅ Passed The changeset, router-core implementation, scroll-restoration logic, and tests directly support the linked issue and stated objectives. No unrelated code changes are present.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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.

resetScroll from a navigation is lost when the destination navigates while it is still settling

1 participant