feat: scrollRestorationBehavior per navigation #6415
Conversation
📝 WalkthroughWalkthroughAdds an optional per-navigation Changes
Sequence Diagram(s)mermaid Client->>Router: navigate({ ..., scrollRestorationBehavior }) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 1m 13s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-20 01:18:39 UTC
|
needs #6398 to be merged first |
schiller-manuel
force-pushed
the
feat/scrollRestorationBehavior-per-navigation
branch
from
7beceed to
9f549da
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@e2e/solid-router/scroll-restoration-sandbox-vite/src/routes/index.tsx`:
- Around line 1-2: There are duplicate imports from '@tanstack/solid-router'
(createFileRoute and separately Link, linkOptions, useNavigate); consolidate
them into a single import statement by combining createFileRoute, Link,
linkOptions, and useNavigate into one import from '@tanstack/solid-router'
(update the import that currently declares createFileRoute and the other import
that declares Link, linkOptions, useNavigate to a single merged import).
In `@e2e/vue-router/scroll-restoration-sandbox-vite/src/routes/index.tsx`:
- Around line 1-2: The file imports from '@tanstack/vue-router' twice;
consolidate them into a single import that includes createFileRoute, Link,
linkOptions, and useNavigate together by replacing the two separate import
statements with one combined import listing those symbols so all uses of
createFileRoute, Link, linkOptions, and useNavigate remain unchanged.
🧹 Nitpick comments (3)
e2e/react-router/scroll-restoration-sandbox-vite/src/routes/index.tsx (1)
1-3: Consolidate duplicate imports from@tanstack/react-router.ESLint correctly flags that the module is imported twice. Merge the imports into a single statement.
Suggested fix
-import { createFileRoute } from '@tanstack/react-router' import * as React from 'react' -import { Link, linkOptions, useNavigate } from '@tanstack/react-router' +import { + createFileRoute, + Link, + linkOptions, + useNavigate, +} from '@tanstack/react-router'e2e/react-router/scroll-restoration-sandbox-vite/tests/app.spec.ts (1)
71-78: Consider extracting the scrollTo interception into a reusable helper.This same interception pattern is repeated four times across the test file. While acceptable for e2e tests, extracting it would reduce duplication.
Regarding the ESLint
any[]warnings: these occur insideaddInitScriptwhich runs in the browser context where TypeScript types don't apply at runtime, so the impact is minimal.Optional: Extract helper function
async function setupScrollToInterception(page: Page) { await page.addInitScript(() => { ;(window as any).__scrollToCalls = [] const originalScrollTo = window.scrollTo.bind(window) window.scrollTo = ((...args: Array<any>) => { ;(window as any).__scrollToCalls.push(args) return originalScrollTo(...args) }) as typeof window.scrollTo }) } async function clearScrollToCalls(page: Page) { await page.evaluate(() => { ;(window as any).__scrollToCalls = [] }) } async function getScrollToCalls(page: Page): Promise<Array<any>> { return page.evaluate(() => (window as any).__scrollToCalls) }e2e/vue-router/scroll-restoration-sandbox-vite/tests/app.spec.ts (1)
57-64: Consider extracting the scrollTo interception setup.The
window.scrollTointerception pattern is duplicated across four tests. You could extract this to a helper function or use a Playwright fixture to reduce duplication.Additionally, ESLint prefers
Array<any>overany[]per project rules, though this is minor in test context.Example helper extraction
async function interceptScrollTo(page: Page) { await page.addInitScript(() => { ;(window as any).__scrollToCalls = [] const originalScrollTo = window.scrollTo.bind(window) window.scrollTo = ((...args: Array<any>) => { ;(window as any).__scrollToCalls.push(args) return originalScrollTo(...args) }) as typeof window.scrollTo }) } async function getScrollToCalls(page: Page): Promise<Array<any>> { return page.evaluate(() => (window as any).__scrollToCalls) } async function clearScrollToCalls(page: Page) { await page.evaluate(() => { ;(window as any).__scrollToCalls = [] }) }Also applies to: 103-110, 167-174, 213-220
| import { createFileRoute } from '@tanstack/solid-router' | ||
| import { Link, linkOptions } from '@tanstack/solid-router' | ||
| import { Link, linkOptions, useNavigate } from '@tanstack/solid-router' |
There was a problem hiding this comment.
Consolidate duplicate imports to satisfy linting.
ESLint import/no-duplicates will fail with two imports from the same module. Merge them into a single import.
🛠️ Proposed fix
-import { createFileRoute } from '@tanstack/solid-router'
-import { Link, linkOptions, useNavigate } from '@tanstack/solid-router'
+import { createFileRoute, Link, linkOptions, useNavigate } from '@tanstack/solid-router'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { createFileRoute } from '@tanstack/solid-router' | |
| import { Link, linkOptions } from '@tanstack/solid-router' | |
| import { Link, linkOptions, useNavigate } from '@tanstack/solid-router' | |
| import { createFileRoute, Link, linkOptions, useNavigate } from '@tanstack/solid-router' |
🧰 Tools
🪛 ESLint
[error] 1-1: '@tanstack/solid-router' imported multiple times.
(import/no-duplicates)
[error] 2-2: '@tanstack/solid-router' imported multiple times.
(import/no-duplicates)
🤖 Prompt for AI Agents
In `@e2e/solid-router/scroll-restoration-sandbox-vite/src/routes/index.tsx` around
lines 1 - 2, There are duplicate imports from '@tanstack/solid-router'
(createFileRoute and separately Link, linkOptions, useNavigate); consolidate
them into a single import statement by combining createFileRoute, Link,
linkOptions, and useNavigate into one import from '@tanstack/solid-router'
(update the import that currently declares createFileRoute and the other import
that declares Link, linkOptions, useNavigate to a single merged import).
| import { createFileRoute } from '@tanstack/vue-router' | ||
| import { Link, linkOptions } from '@tanstack/vue-router' | ||
| import { Link, linkOptions, useNavigate } from '@tanstack/vue-router' |
There was a problem hiding this comment.
Consolidate duplicate imports from the same module.
ESLint correctly flags that @tanstack/vue-router is imported twice. Merge these into a single import statement.
Proposed fix
-import { createFileRoute } from '@tanstack/vue-router'
-import { Link, linkOptions, useNavigate } from '@tanstack/vue-router'
+import {
+ createFileRoute,
+ Link,
+ linkOptions,
+ useNavigate,
+} from '@tanstack/vue-router'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { createFileRoute } from '@tanstack/vue-router' | |
| import { Link, linkOptions } from '@tanstack/vue-router' | |
| import { Link, linkOptions, useNavigate } from '@tanstack/vue-router' | |
| import { | |
| createFileRoute, | |
| Link, | |
| linkOptions, | |
| useNavigate, | |
| } from '@tanstack/vue-router' |
🧰 Tools
🪛 ESLint
[error] 1-1: '@tanstack/vue-router' imported multiple times.
(import/no-duplicates)
[error] 2-2: '@tanstack/vue-router' imported multiple times.
(import/no-duplicates)
🤖 Prompt for AI Agents
In `@e2e/vue-router/scroll-restoration-sandbox-vite/src/routes/index.tsx` around
lines 1 - 2, The file imports from '@tanstack/vue-router' twice; consolidate
them into a single import that includes createFileRoute, Link, linkOptions, and
useNavigate together by replacing the two separate import statements with one
combined import listing those symbols so all uses of createFileRoute, Link,
linkOptions, and useNavigate remain unchanged.
SeanCassiere
left a comment
SeanCassiere
left a comment
There was a problem hiding this comment.
Will need to confirm, but I think you'll need to make sure this prop doesn't get printed to the DOM.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.