diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index c9b279a666f4..6fc99336b2b1 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -46,6 +46,7 @@ import { addBasePath } from '../../../client/add-base-path' import { hasBasePath } from '../../../client/has-base-path' import { getNextPathnameInfo } from './utils/get-next-pathname-info' import { formatNextPathnameInfo } from './utils/format-next-pathname-info' +import { compareRouterStates } from './utils/compare-states' declare global { interface Window { @@ -990,6 +991,7 @@ export default class Router implements BaseRouter { // for static pages with query params in the URL we delay // marking the router ready until after the query is updated // or a navigation has occurred + const readyStateChange = this.isReady !== true this.isReady = true const isSsr = this.isSsr @@ -1127,7 +1129,7 @@ export default class Router implements BaseRouter { ) this._inFlightRoute = as - let localeChange = prevLocale !== nextState.locale + const localeChange = prevLocale !== nextState.locale // If the url change is only related to a hash change // We should not proceed. We should only change the state. @@ -1481,46 +1483,63 @@ export default class Router implements BaseRouter { const isValidShallowRoute = options.shallow && nextState.route === (routeInfo.route ?? route) - const shouldScroll = options.scroll ?? !isValidShallowRoute + const shouldScroll = + options.scroll ?? (!(options as any)._h && !isValidShallowRoute) const resetScroll = shouldScroll ? { x: 0, y: 0 } : null - await this.set( - { - ...nextState, - route, - pathname, - query, - asPath: cleanedAs, - isFallback: false, - }, - routeInfo, - forcedScroll ?? resetScroll - ).catch((e) => { - if (e.cancelled) error = error || e - else throw e - }) + // the new state that the router gonna set + const upcomingRouterState = { + ...nextState, + route, + pathname, + query, + asPath: cleanedAs, + isFallback: false, + } + const upcomingScrollState = forcedScroll ?? resetScroll + + // for query updates we can skip it if the state is unchanged and we don't + // need to scroll + // https://github.com/vercel/next.js/issues/37139 + const canSkipUpdating = + (options as any)._h && + !upcomingScrollState && + !readyStateChange && + !localeChange && + compareRouterStates(upcomingRouterState, this.state) + + if (!canSkipUpdating) { + await this.set( + upcomingRouterState, + routeInfo, + upcomingScrollState + ).catch((e) => { + if (e.cancelled) error = error || e + else throw e + }) - if (error) { - if (!isQueryUpdating) { - Router.events.emit('routeChangeError', error, cleanedAs, routeProps) + if (error) { + if (!isQueryUpdating) { + Router.events.emit('routeChangeError', error, cleanedAs, routeProps) + } + throw error } - throw error - } - if (process.env.__NEXT_I18N_SUPPORT) { - if (nextState.locale) { - document.documentElement.lang = nextState.locale + if (process.env.__NEXT_I18N_SUPPORT) { + if (nextState.locale) { + document.documentElement.lang = nextState.locale + } } - } - if (!isQueryUpdating) { - Router.events.emit('routeChangeComplete', as, routeProps) - } + if (!isQueryUpdating) { + Router.events.emit('routeChangeComplete', as, routeProps) + } - // A hash mark # is the optional last part of a URL - const hashRegex = /#.+$/ - if (shouldScroll && hashRegex.test(as)) { - this.scrollToHash(as) + // A hash mark # is the optional last part of a URL + const hashRegex = /#.+$/ + if (shouldScroll && hashRegex.test(as)) { + this.scrollToHash(as) + } } return true diff --git a/packages/next/shared/lib/router/utils/compare-states.ts b/packages/next/shared/lib/router/utils/compare-states.ts new file mode 100644 index 000000000000..5066ac5c55f3 --- /dev/null +++ b/packages/next/shared/lib/router/utils/compare-states.ts @@ -0,0 +1,32 @@ +import type { default as Router } from '../router' + +export function compareRouterStates(a: Router['state'], b: Router['state']) { + const stateKeys = Object.keys(a) + if (stateKeys.length !== Object.keys(b).length) return false + + for (let i = stateKeys.length; i--; ) { + const key = stateKeys[i] + if (key === 'query') { + const queryKeys = Object.keys(a.query) + if (queryKeys.length !== Object.keys(b.query).length) { + return false + } + for (let j = queryKeys.length; j--; ) { + const queryKey = queryKeys[j] + if ( + !b.query.hasOwnProperty(queryKey) || + a.query[queryKey] !== b.query[queryKey] + ) { + return false + } + } + } else if ( + !b.hasOwnProperty(key) || + a[key as keyof Router['state']] !== b[key as keyof Router['state']] + ) { + return false + } + } + + return true +} diff --git a/test/integration/router-rerender/middleware.js b/test/integration/router-rerender/middleware.js new file mode 100644 index 000000000000..a2089b0349d3 --- /dev/null +++ b/test/integration/router-rerender/middleware.js @@ -0,0 +1,5 @@ +import { NextResponse } from 'next/server' + +export function middleware() { + return NextResponse.next() +} diff --git a/test/integration/router-rerender/next.config.js b/test/integration/router-rerender/next.config.js new file mode 100644 index 000000000000..01d2bc9cff93 --- /dev/null +++ b/test/integration/router-rerender/next.config.js @@ -0,0 +1,10 @@ +module.exports = { + // rewrites() { + // return [ + // { + // source: '/rewrite', + // destination: '/?foo=bar', + // }, + // ] + // }, +} diff --git a/test/integration/router-rerender/pages/index.js b/test/integration/router-rerender/pages/index.js new file mode 100644 index 000000000000..59b807ab1e95 --- /dev/null +++ b/test/integration/router-rerender/pages/index.js @@ -0,0 +1,13 @@ +import { useEffect } from 'react' +import { useRouter } from 'next/router' + +export default function Index() { + const { query } = useRouter() + + useEffect(() => { + window.__renders = window.__renders || [] + window.__renders.push(query.foo) + }) + + return

A page should not be rerendered if unnecessary.

+} diff --git a/test/integration/router-rerender/test/index.test.js b/test/integration/router-rerender/test/index.test.js new file mode 100644 index 000000000000..7985da9d65ca --- /dev/null +++ b/test/integration/router-rerender/test/index.test.js @@ -0,0 +1,56 @@ +/* eslint-env jest */ + +import { join } from 'path' +import { + findPort, + killApp, + launchApp, + nextBuild, + nextStart, +} from 'next-test-utils' +import webdriver from 'next-webdriver' + +const appDir = join(__dirname, '../') + +let appPort +let app + +const runTests = () => { + describe('with middleware', () => { + it('should not trigger unncessary rerenders when middleware is used', async () => { + const browser = await webdriver(appPort, '/') + await new Promise((resolve) => setTimeout(resolve, 100)) + + expect(await browser.eval('window.__renders')).toEqual([undefined]) + }) + }) + + describe('with rewrites', () => { + // TODO: Figure out the `isReady` issue. + it.skip('should not trigger unncessary rerenders when rewrites are used', async () => {}) + it.skip('should rerender with the correct query parameter if present with rewrites', async () => {}) + }) +} + +describe('router rerender', () => { + describe('dev mode', () => { + beforeAll(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + }) + afterAll(() => killApp(app)) + + runTests() + }) + + describe('production mode', () => { + beforeAll(async () => { + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterAll(() => killApp(app)) + + runTests() + }) +})