Skip to content

Commit

Permalink
useSearchParams - bailout to client rendering during static generation (
Browse files Browse the repository at this point in the history
#43603)

Currently `useSearchParams` bails out of static generation altogether, forcing the page to be dynamic. This behaviour is wrong. Instead it should still be statically generated, but `useSearchParams` should only run on the client.

This is achieved by throwing a "bailout to client rendering" error. If there's no suspense boundary the whole page will bailout to be rendered on the client. If there is a suspense boundary it will only bailout from that point.

~This PR also adds handling for `export const dynamic = 'force-static'` combined with `useSearchParams`. If it is enabled it will return an empty `ReadonlyURLSearchParams` and skip the bailout to client rendering. Since the `staticGenerationAsyncStorage` only is available on the server - `forceStatic` is sent to the `app-router` to enable sending an empty `URLSearchParams` to match the server response.~
#43603 (comment) the implementation was wrong, added skipped tests and todo comment for now.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
hanneslund committed Dec 12, 2022
1 parent d2c23bb commit b3dfa03
Show file tree
Hide file tree
Showing 10 changed files with 204 additions and 65 deletions.
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
4 changes: 3 additions & 1 deletion packages/next/client/components/static-generation-bailout.ts
@@ -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

0 comments on commit b3dfa03

Please sign in to comment.