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

feat(app-router): introduce experimental.missingSuspenseWithCSRBailout flag #57642

Merged
merged 46 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
3477c76
throw throw
feedthejim Oct 27, 2023
7471322
update copy
feedthejim Oct 30, 2023
1a4c1ee
Merge remote-tracking branch 'origin/canary' into feedthejim/forbid-n…
feedthejim Nov 1, 2023
c561de7
update test
feedthejim Nov 1, 2023
380b5ee
update test
feedthejim Nov 1, 2023
570b3e5
Merge remote-tracking branch 'origin/canary' into feedthejim/forbid-n…
feedthejim Nov 1, 2023
edd6c57
cleanup
feedthejim Nov 1, 2023
c6e3cae
cleanup
feedthejim Nov 1, 2023
8f9c91f
fix test
feedthejim Nov 1, 2023
2aa2dd9
fix test
feedthejim Nov 1, 2023
424e989
Merge branch 'canary' into feedthejim/forbid-no-suspense
feedthejim Nov 9, 2023
ccda2d4
update snapshot
feedthejim Nov 9, 2023
8e87709
Merge branch 'canary' into feedthejim/forbid-no-suspense
balazsorban44 Jan 4, 2024
f43c5dd
add experimental flag
balazsorban44 Jan 4, 2024
fe5493b
refactor: unify naming of bailout to client rendering error
balazsorban44 Jan 4, 2024
e2a1f9b
improve types
balazsorban44 Jan 4, 2024
0305f84
fix config types, pass flag
balazsorban44 Jan 4, 2024
efb256b
update test
balazsorban44 Jan 4, 2024
1d2caa7
update test
balazsorban44 Jan 4, 2024
63cd1ff
update test
balazsorban44 Jan 4, 2024
907ea98
fix test
balazsorban44 Jan 4, 2024
9657d17
Merge branch 'canary' into feedthejim/forbid-no-suspense
balazsorban44 Jan 4, 2024
5b27bf3
fix types
balazsorban44 Jan 4, 2024
7468cff
update snapshot
balazsorban44 Jan 4, 2024
7b7a682
drop .only from test
balazsorban44 Jan 4, 2024
6671b1f
update more snapshots
balazsorban44 Jan 4, 2024
8b0bce5
make bailout to csr error/message generic
balazsorban44 Jan 4, 2024
7989a55
refactor test
balazsorban44 Jan 4, 2024
5c5d0a1
Merge branch 'canary' into feedthejim/forbid-no-suspense
balazsorban44 Jan 4, 2024
8a26e56
cleanup loadable
balazsorban44 Jan 4, 2024
cce3e7b
Merge branch 'canary' into feedthejim/forbid-no-suspense
balazsorban44 Jan 4, 2024
6303f79
merge canary
balazsorban44 Jan 5, 2024
bf8f54f
revert some changes
balazsorban44 Jan 5, 2024
62c1c2d
revert
balazsorban44 Jan 5, 2024
4cc219a
better type loadable
balazsorban44 Jan 5, 2024
b0decbc
fix type
balazsorban44 Jan 5, 2024
6e92419
don't print bailtout to csr error during prerender
balazsorban44 Jan 5, 2024
f53bcc0
tweak log formatting
balazsorban44 Jan 5, 2024
4413664
fix lint
balazsorban44 Jan 5, 2024
e29f219
add error page
balazsorban44 Jan 5, 2024
3a515d5
fix: switch to native fs, skip next start for test
wyattjoh Jan 5, 2024
6b5ef5d
Merge branch 'canary' into feedthejim/forbid-no-suspense
balazsorban44 Jan 8, 2024
faa9faf
fix test
balazsorban44 Jan 8, 2024
d042d48
add comment to address review
balazsorban44 Jan 9, 2024
bbbcf7c
Merge branch 'canary' into feedthejim/forbid-no-suspense
balazsorban44 Jan 9, 2024
2c08aa2
fix build
balazsorban44 Jan 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions errors/missing-suspense-with-csr-bailout.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
title: Missing Suspense with CSR Bailout
---

#### Why This Error Occurred

Certain methods like `useSearchParams()` opt Next.js into client-side rendering. Without a suspense boundary, this will opt the entire page into client-side rendering, which is likely not intended.

#### Possible Ways to Fix It

Make sure that the method is wrapped in a suspense boundary. This way Next.js will only opt the component into client-side rendering up to the suspense boundary.

### Useful Links

- [`useSearchParams`](https://nextjs.org/docs/app/api-reference/functions/use-search-params)
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import { throwWithNoSSR } from '../../shared/lib/lazy-dynamic/no-ssr-error'
import { BailoutToCSRError } from '../../shared/lib/lazy-dynamic/bailout-to-csr'
import { staticGenerationAsyncStorage } from './static-generation-async-storage.external'

export function bailoutToClientRendering(): void | never {
export function bailoutToClientRendering(reason: string): void | never {
const staticGenerationStore = staticGenerationAsyncStorage.getStore()

if (staticGenerationStore?.forceStatic) {
return
}
if (staticGenerationStore?.forceStatic) return

if (staticGenerationStore?.isStaticGeneration) {
throwWithNoSSR()
}
if (staticGenerationStore?.isStaticGeneration)
throw new BailoutToCSRError(reason)
}
2 changes: 1 addition & 1 deletion packages/next/src/client/components/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export function useSearchParams(): ReadonlyURLSearchParams {
const { bailoutToClientRendering } =
require('./bailout-to-client-rendering') as typeof import('./bailout-to-client-rendering')
// TODO-APP: handle dynamic = 'force-static' here and on the client
bailoutToClientRendering()
bailoutToClientRendering('useSearchParams()')
}

return readonlySearchParams
Expand Down
6 changes: 3 additions & 3 deletions packages/next/src/client/on-recoverable-error.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { isBailoutCSRError } from '../shared/lib/lazy-dynamic/no-ssr-error'
import { isBailoutToCSRError } from '../shared/lib/lazy-dynamic/bailout-to-csr'

export default function onRecoverableError(err: any) {
export default function onRecoverableError(err: unknown) {
// Using default react onRecoverableError
// x-ref: https://github.com/facebook/react/blob/d4bc16a7d69eb2ea38a88c8ac0b461d5f72cdcab/packages/react-dom/src/client/ReactDOMRoot.js#L83
const defaultOnRecoverableError =
Expand All @@ -13,7 +13,7 @@ export default function onRecoverableError(err: any) {
}

// Skip certain custom errors which are not expected to be reported on client
if (isBailoutCSRError(err)) return
if (isBailoutToCSRError(err)) return

defaultOnRecoverableError(err)
}
2 changes: 0 additions & 2 deletions packages/next/src/export/helpers/is-dynamic-usage-error.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { DYNAMIC_ERROR_CODE } from '../../client/components/hooks-server-context'
import { isNotFoundError } from '../../client/components/not-found'
import { isRedirectError } from '../../client/components/redirect'
import { isBailoutCSRError } from '../../shared/lib/lazy-dynamic/no-ssr-error'

export const isDynamicUsageError = (err: any) =>
err.digest === DYNAMIC_ERROR_CODE ||
isNotFoundError(err) ||
isBailoutCSRError(err) ||
isRedirectError(err)
6 changes: 5 additions & 1 deletion packages/next/src/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,11 @@ export async function exportAppImpl(
: {}),
strictNextHead: !!nextConfig.experimental.strictNextHead,
deploymentId: nextConfig.experimental.deploymentId,
experimental: { ppr: nextConfig.experimental.ppr === true },
experimental: {
ppr: nextConfig.experimental.ppr === true,
missingSuspenseWithCSRBailout:
nextConfig.experimental.missingSuspenseWithCSRBailout,
},
}

const { serverRuntimeConfig, publicRuntimeConfig } = nextConfig
Expand Down
14 changes: 5 additions & 9 deletions packages/next/src/export/routes/pages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
NEXT_DATA_SUFFIX,
SERVER_PROPS_EXPORT_ERROR,
} from '../../lib/constants'
import { isBailoutCSRError } from '../../shared/lib/lazy-dynamic/no-ssr-error'
import { isBailoutToCSRError } from '../../shared/lib/lazy-dynamic/bailout-to-csr'
import AmpHtmlValidator from 'next/dist/compiled/amphtml-validator'
import { FileType, fileExists } from '../../lib/file-exists'
import { lazyRenderPagesPage } from '../../server/future/route-modules/pages/module.render'
Expand Down Expand Up @@ -105,10 +105,8 @@ export async function exportPages(
query,
renderOpts
)
} catch (err: any) {
if (!isBailoutCSRError(err)) {
throw err
}
} catch (err) {
if (!isBailoutToCSRError(err)) throw err
}
}

Expand Down Expand Up @@ -163,10 +161,8 @@ export async function exportPages(
{ ...query, amp: '1' },
renderOpts
)
} catch (err: any) {
if (!isBailoutCSRError(err)) {
throw err
}
} catch (err) {
if (!isBailoutToCSRError(err)) throw err
}

const ampHtml =
Expand Down
7 changes: 5 additions & 2 deletions packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { createIncrementalCache } from './helpers/create-incremental-cache'
import { isPostpone } from '../server/lib/router-utils/is-postpone'
import { isMissingPostponeDataError } from '../server/app-render/is-missing-postpone-error'
import { isDynamicUsageError } from './helpers/is-dynamic-usage-error'
import { isBailoutToCSRError } from '../shared/lib/lazy-dynamic/bailout-to-csr'

const envConfig = require('../shared/lib/runtime-config.external')

Expand Down Expand Up @@ -318,9 +319,11 @@ async function exportPageImpl(
// if this is a postpone error, it's logged elsewhere, so no need to log it again here
if (!isMissingPostponeDataError(err)) {
console.error(
`\nError occurred prerendering page "${path}". Read more: https://nextjs.org/docs/messages/prerender-error\n` +
(isError(err) && err.stack ? err.stack : err)
`\nError occurred prerendering page "${path}". Read more: https://nextjs.org/docs/messages/prerender-error\n`
)
if (!isBailoutToCSRError(err)) {
console.error(isError(err) && err.stack ? err.stack : err)
}
}

return { error: true }
Expand Down
19 changes: 13 additions & 6 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ import { parseAndValidateFlightRouterState } from './parse-and-validate-flight-r
import { validateURL } from './validate-url'
import { createFlightRouterStateFromLoaderTree } from './create-flight-router-state-from-loader-tree'
import { handleAction } from './action-handler'
import { isBailoutCSRError } from '../../shared/lib/lazy-dynamic/no-ssr-error'
import { isBailoutToCSRError } from '../../shared/lib/lazy-dynamic/bailout-to-csr'
import { warn, error } from '../../build/output/log'
import { appendMutableCookies } from '../web/spec-extension/adapters/request-cookies'
import { createServerInsertedHTML } from './server-inserted-html'
Expand Down Expand Up @@ -996,12 +996,19 @@ async function renderToHTMLOrFlightImpl(
throw err
}

// True if this error was a bailout to client side rendering error.
const shouldBailoutToCSR = isBailoutCSRError(err)
/** True if this error was a bailout to client side rendering error. */
const shouldBailoutToCSR = isBailoutToCSRError(err)
if (shouldBailoutToCSR) {
console.log()

if (renderOpts.experimental.missingSuspenseWithCSRBailout) {
error(
`${err.message} should be wrapped in a suspense boundary at page "${pagePath}". https://nextjs.org/docs/messages/missing-suspense-with-csr-bailout`
)
throw err
}
warn(
`Entire page ${pagePath} deopted into client-side rendering. https://nextjs.org/docs/messages/deopted-into-client-rendering`,
pagePath
`Entire page "${pagePath}" deopted into client-side rendering. https://nextjs.org/docs/messages/deopted-into-client-rendering`
)
}

Expand Down Expand Up @@ -1212,7 +1219,7 @@ async function renderToHTMLOrFlightImpl(
renderOpts.experimental.ppr &&
staticGenerationStore.postponeWasTriggered &&
!metadata.postponed &&
(!response.err || !isBailoutCSRError(response.err))
(!response.err || !isBailoutToCSRError(response.err))
) {
// a call to postpone was made but was caught and not detected by Next.js. We should fail the build immediately
// as we won't be able to generate the static part
Expand Down
8 changes: 5 additions & 3 deletions packages/next/src/server/app-render/create-error-handler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { formatServerError } from '../../lib/format-server-error'
import { SpanStatusCode, getTracer } from '../lib/trace/tracer'
import { isAbortError } from '../pipe-readable'
import { isDynamicUsageError } from '../../export/helpers/is-dynamic-usage-error'
import { isBailoutToCSRError } from '../../shared/lib/lazy-dynamic/bailout-to-csr'

export type ErrorHandler = (err: any) => string | undefined

Expand Down Expand Up @@ -34,11 +35,12 @@ export function createErrorHandler({
return (err) => {
if (allCapturedErrors) allCapturedErrors.push(err)

// A formatted error is already logged for this type of error
if (isBailoutToCSRError(err)) return

// These errors are expected. We return the digest
// so that they can be properly handled.
if (isDynamicUsageError(err)) {
return err.digest
}
if (isDynamicUsageError(err)) return err.digest

// If the response was closed, we don't need to log the error.
if (isAbortError(err)) return
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/app-render/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export interface RenderOptsPartial {
}
params?: ParsedUrlQuery
isPrefetch?: boolean
experimental: { ppr: boolean }
experimental: { ppr: boolean; missingSuspenseWithCSRBailout?: boolean }
postponed?: string
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export type StaticGenerationContext = {
isDraftMode?: boolean
isServerAction?: boolean
waitUntil?: Promise<any>
experimental: { ppr: boolean }
experimental: { ppr: boolean; missingSuspenseWithCSRBailout?: boolean }

/**
* A hack around accessing the store value outside the context of the
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/server/config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ export const configSchema: zod.ZodType<NextConfig> = z.lazy(() =>
staticWorkerRequestDeduping: z.boolean().optional(),
useWasmBinary: z.boolean().optional(),
useLightningcss: z.boolean().optional(),
missingSuspenseWithCSRBailout: z.boolean().optional(),
})
.optional(),
exportPathMap: z
Expand Down
11 changes: 11 additions & 0 deletions packages/next/src/server/config-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ export interface ExperimentalConfig {
* Use lightningcss instead of swc_css
*/
useLightningcss?: boolean

/**
* Certain methods calls like `useSearchParams()` can bail out of server-side rendering of **entire** pages to client-side rendering,
* if they are not wrapped in a suspense boundary.
*
* When this flag is set to `true`, Next.js will break the build instead of warning, to force the developer to add a suspense boundary above the method call.
*
* @default false
*/
missingSuspenseWithCSRBailout?: boolean
}

export type ExportPathMap = {
Expand Down Expand Up @@ -811,6 +821,7 @@ export const defaultConfig: NextConfig = {
? true
: false,
webpackBuildWorker: undefined,
missingSuspenseWithCSRBailout: false,
},
}

Expand Down
14 changes: 14 additions & 0 deletions packages/next/src/shared/lib/lazy-dynamic/bailout-to-csr.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// This has to be a shared module which is shared between client component error boundary and dynamic component

const BAILOUT_TO_CSR = 'BAILOUT_TO_CLIENT_SIDE_RENDERING'

/** An error that should be thrown when we want to bail out to client-side rendering. */
export class BailoutToCSRError extends Error {
digest: typeof BAILOUT_TO_CSR = BAILOUT_TO_CSR
}

/** Checks if a passed argument is an error that is thrown if we want to bail out to client-side rendering. */
export function isBailoutToCSRError(err: unknown): err is BailoutToCSRError {
if (typeof err !== 'object' || err === null) return false
return 'digest' in err && err.digest === BAILOUT_TO_CSR
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use client'

import type { ReactElement } from 'react'
import { BailoutToCSRError } from './bailout-to-csr'

interface BailoutToCSRProps {
reason: string
children: ReactElement
}

/**
* If rendered on the server, this component throws an error
* to signal Next.js that it should bail out to client-side rendering instead.
*/
export function BailoutToCSR({ reason, children }: BailoutToCSRProps) {
balazsorban44 marked this conversation as resolved.
Show resolved Hide resolved
if (typeof window === 'undefined') {
throw new BailoutToCSRError(reason)
}

return children
}
14 changes: 0 additions & 14 deletions packages/next/src/shared/lib/lazy-dynamic/dynamic-no-ssr.tsx

This file was deleted.

46 changes: 24 additions & 22 deletions packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
Original file line number Diff line number Diff line change
@@ -1,43 +1,45 @@
import { Suspense, lazy, Fragment } from 'react'
import { NoSSR } from './dynamic-no-ssr'
import { Suspense, lazy } from 'react'
import { BailoutToCSR } from './dynamic-bailout-to-csr'
import type { ComponentModule } from './types'

// Normalize loader to return the module as form { default: Component } for `React.lazy`.
// Also for backward compatible since next/dynamic allows to resolve a component directly with loader
// Client component reference proxy need to be converted to a module.
function convertModule<P>(mod: React.ComponentType<P> | ComponentModule<P>) {
return { default: (mod as ComponentModule<P>)?.default || mod }
return { default: (mod as ComponentModule<P>)?.default ?? mod }
}

function Loadable(options: any) {
const opts = {
loader: null,
loading: null,
ssr: true,
...options,
}
const defaultOptions = {
loader: () => Promise.resolve(convertModule(() => null)),
loading: null,
ssr: true,
}

const loader = () =>
opts.loader != null
? opts.loader().then(convertModule)
: Promise.resolve(convertModule(() => null))
interface LoadableOptions {
loader?: () => Promise<React.ComponentType<any> | ComponentModule<any>>
loading?: React.ComponentType<any> | null
ssr?: boolean
}

const Lazy = lazy(loader)
function Loadable(options: LoadableOptions) {
const opts = { ...defaultOptions, ...options }
const Lazy = lazy(() => opts.loader().then(convertModule))
const Loading = opts.loading
const Wrap = opts.ssr ? Fragment : NoSSR

function LoadableComponent(props: any) {
const fallbackElement = Loading ? (
<Loading isLoading={true} pastDelay={true} error={null} />
) : null

return (
<Suspense fallback={fallbackElement}>
<Wrap>
<Lazy {...props} />
</Wrap>
</Suspense>
const children = opts.ssr ? (
<Lazy {...props} />
) : (
<BailoutToCSR reason="next/dynamic">
<Lazy {...props} />
</BailoutToCSR>
)

return <Suspense fallback={fallbackElement}>{children}</Suspense>
}

LoadableComponent.displayName = 'LoadableComponent'
Expand Down
13 changes: 0 additions & 13 deletions packages/next/src/shared/lib/lazy-dynamic/no-ssr-error.ts

This file was deleted.

Loading