From f2c2244f3bfa04285b0019e06038d09933b739d2 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 8 Jun 2022 23:25:30 +0200 Subject: [PATCH] Revert "Avoid unnecessary router state changes (#37431)" This reverts commit 1dad75109cb05372b2d9a3c546bde71197e1a167. --- packages/next/shared/lib/router/router.ts | 96 ++++++------------- .../rewrites-client-rerender/next.config.js | 10 -- .../rewrites-client-rerender/pages/index.js | 13 --- .../test/index.test.js | 55 ----------- 4 files changed, 28 insertions(+), 146 deletions(-) delete mode 100644 test/integration/rewrites-client-rerender/next.config.js delete mode 100644 test/integration/rewrites-client-rerender/pages/index.js delete mode 100644 test/integration/rewrites-client-rerender/test/index.test.js diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 90838d4fb5248..17586b1b1f297 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -81,37 +81,6 @@ function buildCancellationError() { }) } -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 -} - /** * Detects whether a given url is routable by the Next.js router (browser only). */ @@ -1343,47 +1312,38 @@ export default class Router implements BaseRouter { const shouldScroll = options.scroll ?? !isValidShallowRoute const resetScroll = shouldScroll ? { x: 0, y: 0 } : null - const nextScroll = forcedScroll ?? resetScroll - const mergedNextState = { - ...nextState, - route, - pathname, - query, - asPath: cleanedAs, - isFallback: false, - } - - // 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 && - !nextScroll && - compareRouterStates(mergedNextState, this.state) - - if (!canSkipUpdating) { - await this.set(mergedNextState, routeInfo, nextScroll).catch((e) => { - if (e.cancelled) error = error || e - else throw e - }) + 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 + }) - if (error) { - Router.events.emit('routeChangeError', error, cleanedAs, routeProps) - throw error - } + if (error) { + Router.events.emit('routeChangeError', error, cleanedAs, routeProps) + 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 } - Router.events.emit('routeChangeComplete', as, routeProps) + } + 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/test/integration/rewrites-client-rerender/next.config.js b/test/integration/rewrites-client-rerender/next.config.js deleted file mode 100644 index 111b8e2c548ff..0000000000000 --- a/test/integration/rewrites-client-rerender/next.config.js +++ /dev/null @@ -1,10 +0,0 @@ -module.exports = { - rewrites() { - return [ - { - source: '/rewrite', - destination: '/?foo=bar', - }, - ] - }, -} diff --git a/test/integration/rewrites-client-rerender/pages/index.js b/test/integration/rewrites-client-rerender/pages/index.js deleted file mode 100644 index 59b807ab1e958..0000000000000 --- a/test/integration/rewrites-client-rerender/pages/index.js +++ /dev/null @@ -1,13 +0,0 @@ -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/rewrites-client-rerender/test/index.test.js b/test/integration/rewrites-client-rerender/test/index.test.js deleted file mode 100644 index 72412a90aabb6..0000000000000 --- a/test/integration/rewrites-client-rerender/test/index.test.js +++ /dev/null @@ -1,55 +0,0 @@ -/* 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 = () => { - it('should not trigger unncessary rerenders', async () => { - const browser = await webdriver(appPort, '/') - await new Promise((resolve) => setTimeout(resolve, 100)) - - expect(await browser.eval('window.__renders')).toEqual([undefined]) - }) - - it('should rerender with the correct query parameter if present', async () => { - const browser = await webdriver(appPort, '/rewrite') - await new Promise((resolve) => setTimeout(resolve, 100)) - - expect(await browser.eval('window.__renders')).toEqual([undefined, 'bar']) - }) -} - -describe('rewrites client 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() - }) -})