Failing test: popstate causes flash back to PPR state

Will be fixed in the next commit
vercel · Jan 12, 2024 · 464c718 · 464c718
1 parent 4726f3e
commit 464c718
Show file tree

Hide file tree

Showing 7 changed files with 235 additions and 0 deletions.
diff --git a/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/layout.tsx b/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/layout.tsx
@@ -0,0 +1,11 @@
+import { ShouldFallbackThrowContainer } from './some-page/client'
+
+export default function Root({ children }) {
+  return (
+    <html>
+      <body>
+        <ShouldFallbackThrowContainer>{children}</ShouldFallbackThrowContainer>
+      </body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/page.tsx b/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/page.tsx
@@ -0,0 +1,9 @@
+import Link from 'next/link'
+
+export default function Page() {
+  return (
+    <div>
+      <Link href="/some-page">Some page</Link>
+    </div>
+  )
+}
diff --git a/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/some-page/client.tsx b/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/some-page/client.tsx
@@ -0,0 +1,45 @@
+'use client'
+
+import { Component, createContext, use, useState } from 'react'
+
+const ShouldFallbackThrowContext = createContext(false)
+
+export function ShouldFallbackThrowContainer({ children }) {
+  const [shouldFallbackThrow, setShouldFallbackThrow] = useState(false)
+  return (
+    <>
+      <label>
+        Throw if fallback appears
+        <input
+          id="should-fallback-throw"
+          type="checkbox"
+          checked={shouldFallbackThrow}
+          onChange={(e) => setShouldFallbackThrow(e.target.checked)}
+        />
+      </label>
+      <ShouldFallbackThrowContext.Provider value={shouldFallbackThrow}>
+        <ErrorBoundary>{children}</ErrorBoundary>
+      </ShouldFallbackThrowContext.Provider>
+    </>
+  )
+}
+
+export function Fallback({ children }) {
+  if (use(ShouldFallbackThrowContext)) {
+    throw new Error('Unexpected fallback')
+  }
+  return children
+}
+
+class ErrorBoundary extends Component<{ children: React.ReactNode }> {
+  state = { error: null }
+  static getDerivedStateFromError(error) {
+    return { error }
+  }
+  render() {
+    if (this.state.error) {
+      return <div id="error">{this.state.error.message}</div>
+    }
+    return this.props.children
+  }
+}
diff --git a/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/some-page/page.tsx b/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/some-page/page.tsx
@@ -0,0 +1,24 @@
+import React, { Suspense } from 'react'
+import { getDynamicTestData, getStaticTestData } from '../test-data-service'
+import { Fallback } from './client'
+
+async function Dynamic() {
+  return <div id="dynamic">{await getDynamicTestData('Dynamic')}</div>
+}
+
+async function Static() {
+  return <div id="static">{await getStaticTestData('Static')}</div>
+}
+
+export default async function Page() {
+  return (
+    <div id="container">
+      <Suspense fallback={<Fallback>Loading dynamic...</Fallback>}>
+        <Dynamic />
+      </Suspense>
+      <Suspense fallback={<Fallback>Loading static...</Fallback>}>
+        <Static />
+      </Suspense>
+    </div>
+  )
+}
diff --git a/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/test-data-service.ts b/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/app/test-data-service.ts
@@ -0,0 +1,47 @@
+import 'server-only'
+
+import { unstable_noStore } from 'next/cache'
+
+// NOTE: I've intentionally not yet moved these helpers into a shared module, to
+// avoid early abstraction. I will if/when we start using them for other tests.
+// They are based on the testing patterns we use all over the React codebase, so
+// I'm reasonably confident in them.
+const TEST_DATA_SERVICE_URL = process.env.TEST_DATA_SERVICE_URL
+const ARTIFICIAL_DELAY = 3000
+
+async function getTestData(key: string, isStatic: boolean): Promise<string> {
+  const searchParams = new URLSearchParams({
+    key,
+  })
+  if (!TEST_DATA_SERVICE_URL) {
+    // If environment variable is not set, resolve automatically after a delay.
+    // This is so you can run the test app locally without spinning up a
+    // data server.
+    await new Promise<void>((resolve) =>
+      setTimeout(() => resolve(), ARTIFICIAL_DELAY)
+    )
+    if (!isStatic) {
+      unstable_noStore()
+    }
+    return key
+  }
+  const response = await fetch(
+    TEST_DATA_SERVICE_URL + '?' + searchParams.toString(),
+    {
+      cache: isStatic ? 'force-cache' : 'no-store',
+    }
+  )
+  const text = await response.text()
+  if (response.status !== 200) {
+    throw new Error(text)
+  }
+  return text
+}
+
+export async function getStaticTestData(key: string): Promise<string> {
+  return getTestData(key, true)
+}
+
+export async function getDynamicTestData(key: string): Promise<string> {
+  return getTestData(key, false)
+}
diff --git a/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/avoid-popstate-flash.test.ts b/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/avoid-popstate-flash.test.ts
@@ -0,0 +1,89 @@
+import { createNext } from 'e2e-utils'
+import { findPort } from 'next-test-utils'
+import { createTestDataServer } from 'test-data-service/writer'
+import { createTestLog } from 'test-log'
+
+describe('avoid-popstate-flash', () => {
+  if ((global as any).isNextDev) {
+    test('ppr is disabled in dev', () => {})
+    return
+  }
+
+  let server
+  let next
+  afterEach(async () => {
+    await next?.destroy()
+    server?.close()
+  })
+
+  test('does not flash back to partial PPR data during back/forward navigation', async () => {
+    const TestLog = createTestLog()
+    let autoresolveRequests = true
+    let pendingRequests = new Map()
+    server = createTestDataServer(async (key, res) => {
+      TestLog.log('REQUEST: ' + key)
+      if (autoresolveRequests) {
+        res.resolve()
+        return
+      }
+      if (pendingRequests.has(key)) {
+        throw new Error('Request already pending for ' + key)
+      }
+      pendingRequests.set(key, res)
+    })
+    const port = await findPort()
+    server.listen(port)
+    next = await createNext({
+      files: __dirname,
+      env: { TEST_DATA_SERVICE_URL: `http://localhost:${port}` },
+    })
+    TestLog.assert(['REQUEST: Static'])
+    autoresolveRequests = false
+
+    const browser = await next.browser('/')
+
+    // Navigate to the target page.
+    const link = await browser.elementByCss('a[href="/some-page"]')
+    await link.click()
+
+    // The static UI appears immediately because it was prerendered at
+    // build time.
+    const staticContainer = await browser.elementById('static')
+    expect(await staticContainer.innerText()).toBe('Static')
+
+    await TestLog.waitFor(['REQUEST: Dynamic'])
+    pendingRequests.get('Dynamic').resolve()
+
+    // Now the dynamic data appears.
+    const dynamic = await browser.elementById('dynamic')
+    expect(await dynamic.innerText()).toBe('Dynamic')
+
+    // At this point all the data has been loaded into the cache. We're going
+    // to test what happens during a back/forward navigation.
+
+    // Set a global state that causes Suspense fallbacks to throw.
+    const checkbox = await browser.elementById('should-fallback-throw')
+    await checkbox.click()
+    const checked = await checkbox.getProperty('checked')
+    expect(await checked.jsonValue()).toBe(true)
+
+    // Navigate using back/forward using the browser's history stack. This
+    // should not trigger a fresh navigation, nor any network requests. We
+    // should read the data from the cache. And we should not render the
+    // partially complete PPR data that was used during the initial navigation.
+    //
+    // If the data is not read from cache, or if partial data is shown, it will
+    // trigger a fallback, which will throw an error because of the state we
+    // set above.
+    await browser.back()
+    await browser.forward()
+
+    // Confirm that the dynamic data is visible. This implies that the fallback
+    // did not throw.
+    const dynamic2 = await browser.elementById('dynamic')
+    expect(await dynamic2.innerText()).toBe('Dynamic')
+
+    // There should have been no additional requests.
+    TestLog.assert([])
+  })
+})
diff --git a/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/next.config.js b/test/e2e/app-dir/ppr-navigations/avoid-popstate-flash/next.config.js
@@ -0,0 +1,10 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {
+  experimental: {
+    ppr: true,
+  },
+}
+
+module.exports = nextConfig