Skip to content

Commit

Permalink
Re-enable scroll restoration behind flag (#14046)
Browse files Browse the repository at this point in the history
This updates the scroll position saving to occur as the scroll position changes instead of trying to do it when the navigation is changing since the `popState` event doesn't allow us to update the leaving history state once the `popState` has occurred. 

The order of events that was previously attempted to save scroll position on a `popState` event (back/forward navigation)

1. history.state is already updated with state from `popState`
2. we replace state with the currently rendered page adding scroll info
3. we replace state again with the `popState` event state overriding scroll info

Using this approach the above event order is no longer in conflict since we don't attempt to populate the state with scroll position while it's leaving the state and instead do it while it is still the active state in history

This approach resembles existing solutions:
https://www.npmjs.com/package/scroll-behavior
https://twitter.com/ryanflorence/status/1029121580855488512

Fixes: #13990
Fixes: #12530
x-ref: #14075
  • Loading branch information
ijjk authored Jul 6, 2020
1 parent fdc4c17 commit 38bd1a0
Show file tree
Hide file tree
Showing 12 changed files with 361 additions and 1 deletion.
3 changes: 3 additions & 0 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,9 @@ export default async function getBaseWebpackConfig(
'process.env.__NEXT_REACT_MODE': JSON.stringify(
config.experimental.reactMode
),
'process.env.__NEXT_SCROLL_RESTORATION': JSON.stringify(
config.experimental.scrollRestoration
),
'process.env.__NEXT_ROUTER_BASEPATH': JSON.stringify(config.basePath),
...(isServer
? {
Expand Down
40 changes: 40 additions & 0 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ type ComponentLoadCancel = (() => void) | null

type HistoryMethod = 'replaceState' | 'pushState'

const manualScrollRestoration =
process.env.__NEXT_SCROLL_RESTORATION &&
typeof window !== 'undefined' &&
'scrollRestoration' in window.history

function fetchNextData(
dataHref: string,
isServerRender: boolean,
Expand Down Expand Up @@ -250,6 +255,35 @@ export default class Router implements BaseRouter {
}

window.addEventListener('popstate', this.onPopState)

// enable custom scroll restoration handling when available
// otherwise fallback to browser's default handling
if (process.env.__NEXT_SCROLL_RESTORATION) {
if (manualScrollRestoration) {
window.history.scrollRestoration = 'manual'

let scrollDebounceTimeout: undefined | NodeJS.Timeout

const debouncedScrollSave = () => {
if (scrollDebounceTimeout) clearTimeout(scrollDebounceTimeout)

scrollDebounceTimeout = setTimeout(() => {
const { url, as: curAs, options } = history.state
this.changeState(
'replaceState',
url,
curAs,
Object.assign({}, options, {
_N_X: window.scrollX,
_N_Y: window.scrollY,
})
)
}, 10)
}

window.addEventListener('scroll', debouncedScrollSave)
}
}
}
}

Expand Down Expand Up @@ -503,7 +537,13 @@ export default class Router implements BaseRouter {
throw error
}

if (process.env.__NEXT_SCROLL_RESTORATION) {
if (manualScrollRestoration && '_N_X' in options) {
window.scrollTo(options._N_X, options._N_Y)
}
}
Router.events.emit('routeChangeComplete', as)

return resolve(true)
})
},
Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const defaultConfig: { [key: string]: any } = {
workerThreads: false,
pageEnv: false,
productionBrowserSourceMaps: false,
scrollRestoration: false,
},
future: {
excludeDefaultMomentLocales: false,
Expand Down
5 changes: 5 additions & 0 deletions test/integration/scroll-back-restoration/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
experimental: {
scrollRestoration: true,
},
}
10 changes: 10 additions & 0 deletions test/integration/scroll-back-restoration/pages/another.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Link from 'next/link'

export default () => (
<>
<p id="another">hi from another</p>
<Link href="/">
<a id="to-index">to index</a>
</Link>
</>
)
50 changes: 50 additions & 0 deletions test/integration/scroll-back-restoration/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import Link from 'next/link'

const Page = ({ loaded }) => {
const link = (
<Link href="/another">
<a
id="to-another"
style={{
marginLeft: 5000,
width: 95000,
display: 'block',
}}
>
to another
</a>
</Link>
)

if (typeof window !== 'undefined') {
window.didHydrate = true
}

if (loaded) {
return (
<>
<div
style={{
width: 10000,
height: 10000,
background: 'orange',
}}
/>
{link}
<div id="end-el">the end</div>
</>
)
}

return link
}

export default Page

export const getServerSideProps = () => {
return {
props: {
loaded: true,
},
}
}
89 changes: 89 additions & 0 deletions test/integration/scroll-back-restoration/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/* eslint-env jest */

import { join } from 'path'
import webdriver from 'next-webdriver'
import {
killApp,
findPort,
launchApp,
nextStart,
nextBuild,
check,
} from 'next-test-utils'

jest.setTimeout(1000 * 60 * 2)

const appDir = join(__dirname, '../')
let appPort
let app

const runTests = () => {
it('should restore the scroll position on navigating back', async () => {
const browser = await webdriver(appPort, '/')
await browser.eval(() =>
document.querySelector('#to-another').scrollIntoView()
)
const scrollRestoration = await browser.eval(
() => window.history.scrollRestoration
)

expect(scrollRestoration).toBe('manual')

const scrollX = Math.floor(await browser.eval(() => window.scrollX))
const scrollY = Math.floor(await browser.eval(() => window.scrollY))

expect(scrollX).not.toBe(0)
expect(scrollY).not.toBe(0)

await browser.eval(() => window.next.router.push('/another'))

await check(
() => browser.eval(() => document.documentElement.innerHTML),
/hi from another/
)
await browser.eval(() => (window.didHydrate = false))

await browser.eval(() => window.history.back())
await check(() => browser.eval(() => window.didHydrate), {
test(content) {
return content
},
})

const newScrollX = Math.floor(await browser.eval(() => window.scrollX))
const newScrollY = Math.floor(await browser.eval(() => window.scrollY))

console.log({
scrollX,
scrollY,
newScrollX,
newScrollY,
})

expect(scrollX).toBe(newScrollX)
expect(scrollY).toBe(newScrollY)
})
}

describe('Scroll Restoration Support', () => {
describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(() => killApp(app))

runTests()
})

describe('server mode', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(() => killApp(app))

runTests()
})
})
5 changes: 5 additions & 0 deletions test/integration/scroll-forward-restoration/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
experimental: {
scrollRestoration: true,
},
}
10 changes: 10 additions & 0 deletions test/integration/scroll-forward-restoration/pages/another.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Link from 'next/link'

export default () => (
<>
<p id="another">hi from another</p>
<Link href="/">
<a id="to-index">to index</a>
</Link>
</>
)
50 changes: 50 additions & 0 deletions test/integration/scroll-forward-restoration/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import Link from 'next/link'

const Page = ({ loaded }) => {
const link = (
<Link href="/another">
<a
id="to-another"
style={{
marginLeft: 5000,
width: 95000,
display: 'block',
}}
>
to another
</a>
</Link>
)

if (typeof window !== 'undefined') {
window.didHydrate = true
}

if (loaded) {
return (
<>
<div
style={{
width: 10000,
height: 10000,
background: 'orange',
}}
/>
{link}
<div id="end-el">the end</div>
</>
)
}

return link
}

export default Page

export const getServerSideProps = () => {
return {
props: {
loaded: true,
},
}
}
Loading

1 comment on commit 38bd1a0

@abdelrhman-adel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this still experimental?

Please sign in to comment.