Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useSearchParams - bailout to client rendering during static generation #43603

Merged
merged 23 commits into from Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6396f16
Add error for bailing out to client rendering while SSG:ing
Dec 1, 2022
0ebd27f
Return true when it's forceStatic, same pattern as SSG bailout
Dec 1, 2022
acf5a12
Replace staticGenerationBailout with bailoutToClientRendering in useS…
Dec 1, 2022
2a68fc7
Add handling for bailout error
Dec 1, 2022
649894f
Return empty [200~ReadonlyURLSearchParams~ on force-static
Dec 1, 2022
c85f63e
Send forceStatic to app router, return empty search params when true
Dec 1, 2022
c20dc49
Update tests
Dec 1, 2022
87d9010
Merge branch 'canary' of https://github.com/hanneslund/next.js into u…
Dec 1, 2022
ad8a4c6
Skip server response tests in dev
Dec 1, 2022
3578702
Merge branch 'canary' into use-search-params-bailout
hanneslund Dec 1, 2022
ae3a6e6
Merge branch 'canary' into use-search-params-bailout
hanneslund Dec 1, 2022
343f91d
Merge branch 'canary' into use-search-params-bailout
hanneslund Dec 1, 2022
0f3e30f
Merge branch 'canary' into use-search-params-bailout
hanneslund Dec 2, 2022
c406aa3
Merge branch 'canary' of https://github.com/hanneslund/next.js into u…
Dec 5, 2022
c8a9039
Fix return types
Dec 5, 2022
2df8694
Merge branch 'canary' of https://github.com/hanneslund/next.js into u…
Dec 8, 2022
040795c
Add failing test for navigating with force-static
Dec 8, 2022
8e96a8f
Remove bad force-static handling from client
Dec 8, 2022
f249bac
Reuse dynamic-no-ssr suspense
Dec 8, 2022
2675cbf
Skip force-static tests
Dec 8, 2022
c12de2e
Merge branch 'canary' into use-search-params-bailout
hanneslund Dec 8, 2022
ba12175
Merge branch 'canary' into use-search-params-bailout
hanneslund Dec 9, 2022
758324f
Merge branch 'canary' into use-search-params-bailout
hanneslund Dec 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions packages/next/client/components/bailout-to-client-rendering.ts
@@ -0,0 +1,19 @@
import { suspense } from '../../shared/lib/dynamic-no-ssr'
import { staticGenerationAsyncStorage } from './static-generation-async-storage'

export function bailoutToClientRendering(): boolean | never {
const staticGenerationStore =
staticGenerationAsyncStorage && 'getStore' in staticGenerationAsyncStorage
? staticGenerationAsyncStorage?.getStore()
: staticGenerationAsyncStorage

if (staticGenerationStore?.forceStatic) {
return true
}

if (staticGenerationStore?.isStaticGeneration) {
suspense()
}

return false
}
5 changes: 3 additions & 2 deletions packages/next/client/components/navigation.ts
Expand Up @@ -12,7 +12,7 @@ import {
PathnameContext,
// LayoutSegmentsContext,
} from '../../shared/lib/hooks-client-context'
import { staticGenerationBailout } from './static-generation-bailout'
import { bailoutToClientRendering } from './bailout-to-client-rendering'

const INTERNAL_URLSEARCHPARAMS_INSTANCE = Symbol(
'internal for urlsearchparams readonly'
Expand Down Expand Up @@ -76,7 +76,8 @@ export function useSearchParams() {
return new ReadonlyURLSearchParams(searchParams || new URLSearchParams())
}, [searchParams])

if (staticGenerationBailout('useSearchParams')) {
if (bailoutToClientRendering()) {
// TODO-APP: handle dynamic = 'force-static' here and on the client
return readonlySearchParams
}

Expand Down
@@ -1,7 +1,7 @@
import { DynamicServerError } from './hooks-server-context'
import { staticGenerationAsyncStorage } from './static-generation-async-storage'

export function staticGenerationBailout(reason: string) {
export function staticGenerationBailout(reason: string): boolean | never {
const staticGenerationStore =
staticGenerationAsyncStorage && 'getStore' in staticGenerationAsyncStorage
? staticGenerationAsyncStorage?.getStore()
Expand All @@ -17,4 +17,6 @@ export function staticGenerationBailout(reason: string) {
}
throw new DynamicServerError(reason)
}

return false
}
186 changes: 138 additions & 48 deletions test/e2e/app-dir/app-static.test.ts
Expand Up @@ -65,7 +65,15 @@ describe('app-dir static/dynamic handling', () => {
'hooks/use-pathname/[slug]/page.js',
'hooks/use-pathname/slug.html',
'hooks/use-pathname/slug.rsc',
'hooks/use-search-params/[slug]/page.js',
'hooks/use-search-params.html',
'hooks/use-search-params.rsc',
'hooks/use-search-params/force-static.html',
'hooks/use-search-params/force-static.rsc',
'hooks/use-search-params/force-static/page.js',
'hooks/use-search-params/page.js',
'hooks/use-search-params/with-suspense.html',
'hooks/use-search-params/with-suspense.rsc',
'hooks/use-search-params/with-suspense/page.js',
'ssg-preview.html',
'ssg-preview.rsc',
'ssg-preview/[[...route]]/page.js',
Expand Down Expand Up @@ -146,6 +154,21 @@ describe('app-dir static/dynamic handling', () => {
initialRevalidateSeconds: false,
srcRoute: '/hooks/use-pathname/[slug]',
},
'/hooks/use-search-params': {
dataRoute: '/hooks/use-search-params.rsc',
initialRevalidateSeconds: false,
srcRoute: '/hooks/use-search-params',
},
'/hooks/use-search-params/force-static': {
dataRoute: '/hooks/use-search-params/force-static.rsc',
initialRevalidateSeconds: false,
srcRoute: '/hooks/use-search-params/force-static',
},
'/hooks/use-search-params/with-suspense': {
dataRoute: '/hooks/use-search-params/with-suspense.rsc',
initialRevalidateSeconds: false,
srcRoute: '/hooks/use-search-params/with-suspense',
},
'/force-static/first': {
dataRoute: '/force-static/first.rsc',
initialRevalidateSeconds: false,
Expand Down Expand Up @@ -526,23 +549,28 @@ describe('app-dir static/dynamic handling', () => {
expect(secondDate).not.toBe(initialDate)
})

describe('hooks', () => {
describe('useSearchParams', () => {
if (isDev) {
it('should bail out to client rendering during SSG', async () => {
const res = await fetchViaHTTP(
next.url,
'/hooks/use-search-params/slug'
)
const html = await res.text()
expect(html).toInclude('<html id="__next_error__">')
})
}
describe('useSearchParams', () => {
describe('client', () => {
it('should bailout to client rendering - without suspense boundary', async () => {
const browser = await webdriver(
next.url,
'/hooks/use-search-params?first=value&second=other&third'
)

it('should have the correct values', async () => {
expect(await browser.elementByCss('#params-first').text()).toBe('value')
expect(await browser.elementByCss('#params-second').text()).toBe(
'other'
)
expect(await browser.elementByCss('#params-third').text()).toBe('')
expect(await browser.elementByCss('#params-not-real').text()).toBe(
'N/A'
)
})

it('should bailout to client rendering - with suspense boundary', async () => {
const browser = await webdriver(
next.url,
'/hooks/use-search-params/slug?first=value&second=other&third'
'/hooks/use-search-params/with-suspense?first=value&second=other&third'
)

expect(await browser.elementByCss('#params-first').text()).toBe('value')
Expand All @@ -555,6 +583,31 @@ describe('app-dir static/dynamic handling', () => {
)
})

it.skip('should have empty search params on force-static', async () => {
const browser = await webdriver(
next.url,
'/hooks/use-search-params/force-static?first=value&second=other&third'
)

expect(await browser.elementByCss('#params-first').text()).toBe('N/A')
expect(await browser.elementByCss('#params-second').text()).toBe('N/A')
expect(await browser.elementByCss('#params-third').text()).toBe('N/A')
expect(await browser.elementByCss('#params-not-real').text()).toBe(
'N/A'
)

await browser.elementById('to-use-search-params').click()
await browser.waitForElementByCss('#hooks-use-search-params')

// Should not be empty after navigating to another page with useSearchParams
expect(await browser.elementByCss('#params-first').text()).toBe('1')
expect(await browser.elementByCss('#params-second').text()).toBe('2')
expect(await browser.elementByCss('#params-third').text()).toBe('3')
expect(await browser.elementByCss('#params-not-real').text()).toBe(
'N/A'
)
})

// TODO-APP: re-enable after investigating rewrite params
if (!(global as any).isNextDeploy) {
it('should have values from canonical url on rewrite', async () => {
Expand All @@ -572,52 +625,89 @@ describe('app-dir static/dynamic handling', () => {
})
}
})

// TODO: needs updating as usePathname should not bail
describe.skip('usePathname', () => {
if (isDev) {
it('should bail out to client rendering during SSG', async () => {
const res = await fetchViaHTTP(next.url, '/hooks/use-pathname/slug')
// Don't run these tests in dev mode since they won't be statically generated
if (!isDev) {
describe('server response', () => {
it('should bailout to client rendering - without suspense boundary', async () => {
const res = await fetchViaHTTP(next.url, '/hooks/use-search-params')
const html = await res.text()
expect(html).toInclude('<html id="__next_error__">')
})
}

it('should have the correct values', async () => {
const browser = await webdriver(next.url, '/hooks/use-pathname/slug')
it('should bailout to client rendering - with suspense boundary', async () => {
const res = await fetchViaHTTP(
next.url,
'/hooks/use-search-params/with-suspense'
)
const html = await res.text()
expect(html).toInclude('<p>search params suspense</p>')
})

expect(await browser.elementByCss('#pathname').text()).toBe(
'/hooks/use-pathname/slug'
)
})
it.skip('should have empty search params on force-static', async () => {
const res = await fetchViaHTTP(
next.url,
'/hooks/use-search-params/force-static?first=value&second=other&third'
)
const html = await res.text()

it('should have values from canonical url on rewrite', async () => {
const browser = await webdriver(next.url, '/rewritten-use-pathname')
// Shouild not bail out to client rendering
expect(html).not.toInclude('<p>search params suspense</p>')

expect(await browser.elementByCss('#pathname').text()).toBe(
'/rewritten-use-pathname'
)
// Use empty search params instead
const $ = cheerio.load(html)
expect($('#params-first').text()).toBe('N/A')
expect($('#params-second').text()).toBe('N/A')
expect($('#params-third').text()).toBe('N/A')
expect($('#params-not-real').text()).toBe('N/A')
})
})
})
}
})

if (!(global as any).isNextDeploy) {
it('should show a message to leave feedback for `appDir`', async () => {
expect(next.cliOutput).toContain(
`Thank you for testing \`appDir\` please leave your feedback at https://nextjs.link/app-feedback`
)
// TODO: needs updating as usePathname should not bail
describe.skip('usePathname', () => {
if (isDev) {
it('should bail out to client rendering during SSG', async () => {
const res = await fetchViaHTTP(next.url, '/hooks/use-pathname/slug')
const html = await res.text()
expect(html).toInclude('<html id="__next_error__">')
})
}

it('should keep querystring on static page', async () => {
const browser = await webdriver(next.url, '/blog/tim?message=hello-world')
const checkUrl = async () =>
expect(await browser.url()).toBe(
next.url + '/blog/tim?message=hello-world'
)
it('should have the correct values', async () => {
const browser = await webdriver(next.url, '/hooks/use-pathname/slug')

expect(await browser.elementByCss('#pathname').text()).toBe(
'/hooks/use-pathname/slug'
)
})

it('should have values from canonical url on rewrite', async () => {
const browser = await webdriver(next.url, '/rewritten-use-pathname')

checkUrl()
await waitFor(1000)
checkUrl()
expect(await browser.elementByCss('#pathname').text()).toBe(
'/rewritten-use-pathname'
)
})
})

if (!(global as any).isNextDeploy) {
it('should show a message to leave feedback for `appDir`', async () => {
expect(next.cliOutput).toContain(
`Thank you for testing \`appDir\` please leave your feedback at https://nextjs.link/app-feedback`
)
})
}

it('should keep querystring on static page', async () => {
const browser = await webdriver(next.url, '/blog/tim?message=hello-world')
const checkUrl = async () =>
expect(await browser.url()).toBe(
next.url + '/blog/tim?message=hello-world'
)

checkUrl()
await waitFor(1000)
checkUrl()
})
})

This file was deleted.

@@ -0,0 +1,19 @@
export const dynamic = 'force-static'

import Link from 'next/link'
import { Suspense } from 'react'
import UseSearchParams from '../search-params'

export default function Page() {
return (
<Suspense fallback={<p>search params suspense</p>}>
<UseSearchParams />
<Link
id="to-use-search-params"
href="/hooks/use-search-params?first=1&second=2&third=3"
>
To /
</Link>
</Suspense>
)
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/app-static/app/hooks/use-search-params/page.js
@@ -0,0 +1,10 @@
import UseSearchParams from './search-params'

export default function Page() {
return (
<>
<p id="hooks-use-search-params" />
<UseSearchParams />
</>
)
}
@@ -1,11 +1,7 @@
'use client'
import { useSearchParams } from 'next/navigation'

export const config = {
dynamicParams: false,
}

export default function Page() {
export default function UseSearchParams() {
const params = useSearchParams()

return (
Expand Down
@@ -0,0 +1,10 @@
import { Suspense } from 'react'
import UseSearchParams from '../search-params'

export default function Page() {
return (
<Suspense fallback={<p>search params suspense</p>}>
<UseSearchParams />
</Suspense>
)
}
3 changes: 1 addition & 2 deletions test/e2e/app-dir/app-static/next.config.js
Expand Up @@ -14,8 +14,7 @@ module.exports = {
},
{
source: '/rewritten-use-search-params',
destination:
'/hooks/use-search-params/slug?first=value&second=other%20value&third',
destination: '/hooks/use-search-params',
},
{
source: '/rewritten-use-pathname',
Expand Down