Skip to content

Commit

Permalink
Ignore popstate with invalid state (#37110)
Browse files Browse the repository at this point in the history
* Ignore popstate with invalid state

* Make tests pass

* i18n case fixed

* Fix lint error

* Unhandled Promise Rejection

* Revert "Unhandled Promise Rejection"

This reverts commit ac4fde7.

* fix type check

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
Hannes Bornö and ijjk committed May 29, 2022
1 parent 88b1f7d commit 5739edc
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 2 deletions.
15 changes: 14 additions & 1 deletion packages/next/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ type PreflightEffect =
| { type: 'refresh' }
| { type: 'next' }

type HistoryState =
export type HistoryState =
| null
| { __N: false }
| ({ __N: true; key: string } & NextHistoryState)
Expand Down Expand Up @@ -642,6 +642,7 @@ export default class Router implements BaseRouter {
domainLocales?: DomainLocale[] | undefined
isReady: boolean
isLocaleDomain: boolean
isFirstPopStateEvent = true

private state: Readonly<{
route: string
Expand Down Expand Up @@ -797,6 +798,9 @@ export default class Router implements BaseRouter {
}

onPopState = (e: PopStateEvent): void => {
const { isFirstPopStateEvent } = this
this.isFirstPopStateEvent = false

const state = e.state as HistoryState

if (!state) {
Expand All @@ -822,6 +826,15 @@ export default class Router implements BaseRouter {
return
}

// Safari fires popstateevent when reopening the browser.
if (
isFirstPopStateEvent &&
this.locale === state.options.locale &&
state.as === this.asPath
) {
return
}

let forcedScroll: { x: number; y: number } | undefined
const { url, as, options, key } = state
if (process.env.__NEXT_SCROLL_RESTORATION) {
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/ignore-invalid-popstateevent/app/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
i18n: {
locales: ['en', 'sv'],
defaultLocale: 'en',
},
}
3 changes: 3 additions & 0 deletions test/e2e/ignore-invalid-popstateevent/app/pages/[dynamic].js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function DynamicPage() {
return <p id="page-type">dynamic</p>
}
3 changes: 3 additions & 0 deletions test/e2e/ignore-invalid-popstateevent/app/pages/static.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function StaticPage() {
return <p id="page-type">static</p>
}
94 changes: 94 additions & 0 deletions test/e2e/ignore-invalid-popstateevent/with-i18n.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { join } from 'path'
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { check, waitFor } from 'next-test-utils'
import webdriver from 'next-webdriver'

import type { HistoryState } from '../../../packages/next/shared/lib/router/router'
import { BrowserInterface } from 'test/lib/browsers/base'

const emitPopsStateEvent = (browser: BrowserInterface, state: HistoryState) =>
browser.eval(
`window.dispatchEvent(new PopStateEvent("popstate", { state: ${JSON.stringify(
state
)} }))`
)

describe('i18n: Event with stale state - static route previously was dynamic', () => {
let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: {
pages: new FileRef(join(__dirname, 'app/pages')),
'next.config.js': new FileRef(join(__dirname, 'app/next.config.js')),
},
dependencies: {},
})
})
afterAll(() => next.destroy())

test('Ignore event without query param', async () => {
const browser = await webdriver(next.url, '/sv/static')
browser.close()

const state: HistoryState = {
url: '/[dynamic]?',
as: '/static',
options: { locale: 'sv' },
__N: true,
key: '',
}

expect(await browser.elementByCss('#page-type').text()).toBe('static')

// 1st event is ignored
await emitPopsStateEvent(browser, state)
await waitFor(1000)
expect(await browser.elementByCss('#page-type').text()).toBe('static')

// 2nd event isn't ignored
await emitPopsStateEvent(browser, state)
await check(() => browser.elementByCss('#page-type').text(), 'dynamic')
})

test('Ignore event with query param', async () => {
const browser = await webdriver(next.url, '/sv/static?param=1')

const state: HistoryState = {
url: '/[dynamic]?param=1',
as: '/static?param=1',
options: { locale: 'sv' },
__N: true,
key: '',
}

expect(await browser.elementByCss('#page-type').text()).toBe('static')

// 1st event is ignored
await emitPopsStateEvent(browser, state)
await waitFor(1000)
expect(await browser.elementByCss('#page-type').text()).toBe('static')

// 2nd event isn't ignored
await emitPopsStateEvent(browser, state)
await check(() => browser.elementByCss('#page-type').text(), 'dynamic')
})

test("Don't ignore event with different locale", async () => {
const browser = await webdriver(next.url, '/sv/static?param=1')

const state: HistoryState = {
url: '/[dynamic]?param=1',
as: '/static?param=1',
options: { locale: 'en' },
__N: true,
key: '',
}

expect(await browser.elementByCss('#page-type').text()).toBe('static')

await emitPopsStateEvent(browser, state)
await check(() => browser.elementByCss('#page-type').text(), 'dynamic')
})
})
77 changes: 77 additions & 0 deletions test/e2e/ignore-invalid-popstateevent/without-i18n.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { join } from 'path'
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { check, waitFor } from 'next-test-utils'
import webdriver from 'next-webdriver'

import type { HistoryState } from '../../../packages/next/shared/lib/router/router'
import { BrowserInterface } from 'test/lib/browsers/base'

const emitPopsStateEvent = (browser: BrowserInterface, state: HistoryState) =>
browser.eval(
`window.dispatchEvent(new PopStateEvent("popstate", { state: ${JSON.stringify(
state
)} }))`
)

describe('Event with stale state - static route previously was dynamic', () => {
let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: {
pages: new FileRef(join(__dirname, 'app/pages')),
// Don't use next.config.js to avoid getting i18n
},
dependencies: {},
})
})
afterAll(() => next.destroy())

test('Ignore event without query param', async () => {
const browser = await webdriver(next.url, '/static')
browser.close()

const state: HistoryState = {
url: '/[dynamic]?',
as: '/static',
options: {},
__N: true,
key: '',
}

expect(await browser.elementByCss('#page-type').text()).toBe('static')

// 1st event is ignored
await emitPopsStateEvent(browser, state)
await waitFor(1000)
expect(await browser.elementByCss('#page-type').text()).toBe('static')

// 2nd event isn't ignored
await emitPopsStateEvent(browser, state)
await check(() => browser.elementByCss('#page-type').text(), 'dynamic')
})

test('Ignore event with query param', async () => {
const browser = await webdriver(next.url, '/static?param=1')

const state: HistoryState = {
url: '/[dynamic]?param=1',
as: '/static?param=1',
options: {},
__N: true,
key: '',
}

expect(await browser.elementByCss('#page-type').text()).toBe('static')

// 1st event is ignored
await emitPopsStateEvent(browser, state)
await waitFor(1000)
expect(await browser.elementByCss('#page-type').text()).toBe('static')

// 2nd event isn't ignored
await emitPopsStateEvent(browser, state)
await check(() => browser.elementByCss('#page-type').text(), 'dynamic')
})
})
7 changes: 6 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,10 @@
"e2e-utils": ["test/lib/e2e-utils"]
}
},
"include": ["test/**/*.test.ts", "test/**/*.test.tsx", "test/**/test/*"]
"include": [
"test/**/*.test.ts",
"test/**/*.test.tsx",
"test/**/test/*",
"packages/next/types/webpack.d.ts"
]
}

0 comments on commit 5739edc

Please sign in to comment.