From a94eeadaec784d0b25df1ab8d3a0281bf3c77843 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 21 Jul 2023 09:04:38 +0200 Subject: [PATCH 1/7] Revert "Revert "Refine the not-found rendering process for app router" (#52977)" This reverts commit d7335b75d14bfecf65d6905dd64e25181b8c255a. --- .../next/src/client/components/app-router.tsx | 41 +-- .../dev-root-not-found-boundary.tsx | 25 ++ .../src/client/components/layout-router.tsx | 3 - .../client/components/not-found-boundary.tsx | 3 + .../react-dev-overlay/hot-reloader-client.tsx | 62 ++-- .../internal/ReactDevOverlay.tsx | 5 - .../internal/error-overlay-reducer.ts | 12 +- .../next/src/server/app-render/app-render.tsx | 346 +++++++++++------- .../create-server-components-renderer.tsx | 15 - .../next/src/server/lib/app-dir-module.ts | 2 +- packages/next/src/server/render.tsx | 1 - .../stream-utils/node-web-streams-helper.ts | 19 + .../app-dir/actions/app/server/client-form.js | 2 +- test/e2e/app-dir/actions/app/server/form.js | 2 +- test/e2e/app-dir/metadata/app/not-found.tsx | 2 +- .../e2e/app-dir/navigation/navigation.test.ts | 7 + test/e2e/app-dir/not-found/app/layout.js | 14 +- test/e2e/app-dir/not-found/app/not-found.js | 4 +- test/e2e/app-dir/not-found/not-found.test.ts | 8 +- .../root-layout-not-found/app/layout.js | 27 ++ .../app/not-found-trigger.js | 12 + .../app-dir/root-layout-not-found/app/page.js | 3 + .../root-layout-not-found/index.test.ts | 52 +++ 23 files changed, 421 insertions(+), 246 deletions(-) create mode 100644 packages/next/src/client/components/dev-root-not-found-boundary.tsx create mode 100644 test/e2e/app-dir/root-layout-not-found/app/layout.js create mode 100644 test/e2e/app-dir/root-layout-not-found/app/not-found-trigger.js create mode 100644 test/e2e/app-dir/root-layout-not-found/app/page.js create mode 100644 test/e2e/app-dir/root-layout-not-found/index.test.ts diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index 7aefd5773f859..b00f6ff57c34c 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -54,7 +54,6 @@ import { isBot } from '../../shared/lib/router/utils/is-bot' import { addBasePath } from '../add-base-path' import { AppRouterAnnouncer } from './app-router-announcer' import { RedirectBoundary } from './redirect-boundary' -import { NotFoundBoundary } from './not-found-boundary' import { findHeadInCache } from './router-reducer/reducers/find-head-in-cache' import { createInfinitePromise } from './infinite-promise' import { NEXT_RSC_UNION_QUERY } from './app-router-headers' @@ -89,14 +88,6 @@ export function urlToUrlWithoutFlightMarker(url: string): URL { return urlWithoutFlightParameters } -const HotReloader: - | typeof import('./react-dev-overlay/hot-reloader-client').default - | null = - process.env.NODE_ENV === 'production' - ? null - : (require('./react-dev-overlay/hot-reloader-client') - .default as typeof import('./react-dev-overlay/hot-reloader-client').default) - type AppRouterProps = Omit< Omit, 'initialParallelRoutes' @@ -104,9 +95,6 @@ type AppRouterProps = Omit< buildId: string initialHead: ReactNode assetPrefix: string - // Top level boundaries props - notFound: React.ReactNode | undefined - asNotFound?: boolean } function isExternalURL(url: URL) { @@ -224,8 +212,6 @@ function Router({ initialCanonicalUrl, children, assetPrefix, - notFound, - asNotFound, }: AppRouterProps) { const initialState = useMemo( () => @@ -445,9 +431,7 @@ function Router({ return findHeadInCache(cache, tree[1]) }, [cache, tree]) - const notFoundProps = { notFound, asNotFound } - - const content = ( + let content = ( {head} {cache.subTreeData} @@ -455,6 +439,18 @@ function Router({ ) + if (process.env.NODE_ENV !== 'production') { + if (typeof window !== 'undefined') { + const DevRootNotFoundBoundary: typeof import('./dev-root-not-found-boundary').DevRootNotFoundBoundary = + require('./dev-root-not-found-boundary').DevRootNotFoundBoundary + content = {content} + } + const HotReloader: typeof import('./react-dev-overlay/hot-reloader-client').default = + require('./react-dev-overlay/hot-reloader-client').default + + content = {content} + } + return ( <> - {HotReloader ? ( - // HotReloader implements a separate NotFoundBoundary to maintain the HMR ping interval - - {content} - - ) : ( - - {content} - - )} + {content} diff --git a/packages/next/src/client/components/dev-root-not-found-boundary.tsx b/packages/next/src/client/components/dev-root-not-found-boundary.tsx new file mode 100644 index 0000000000000..d2391062104c3 --- /dev/null +++ b/packages/next/src/client/components/dev-root-not-found-boundary.tsx @@ -0,0 +1,25 @@ +'use client' + +import React from 'react' +import { NotFoundBoundary } from './not-found-boundary' + +export function bailOnNotFound() { + throw new Error('notFound() is not allowed to use in root layout') +} + +function NotAllowedRootNotFoundError() { + bailOnNotFound() + return null +} + +export function DevRootNotFoundBoundary({ + children, +}: { + children: React.ReactNode +}) { + return ( + }> + {children} + + ) +} diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx index d3c9a0733383d..2cd8ce9309ca2 100644 --- a/packages/next/src/client/components/layout-router.tsx +++ b/packages/next/src/client/components/layout-router.tsx @@ -491,7 +491,6 @@ export default function OuterLayoutRouter({ template, notFound, notFoundStyles, - asNotFound, styles, }: { parallelRouterKey: string @@ -506,7 +505,6 @@ export default function OuterLayoutRouter({ hasLoading: boolean notFound: React.ReactNode | undefined notFoundStyles: React.ReactNode | undefined - asNotFound?: boolean styles?: React.ReactNode }) { const context = useContext(LayoutRouterContext) @@ -574,7 +572,6 @@ export default function OuterLayoutRouter({ + {process.env.NODE_ENV === 'development' && ( + + )} {this.props.notFoundStyles} {this.props.notFound} diff --git a/packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx b/packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx index 878abd5fd9f71..e049d3f22a397 100644 --- a/packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx +++ b/packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx @@ -10,7 +10,6 @@ import stripAnsi from 'next/dist/compiled/strip-ansi' import formatWebpackMessages from '../../dev/error-overlay/format-webpack-messages' import { useRouter } from '../navigation' import { - ACTION_NOT_FOUND, ACTION_VERSION_INFO, INITIAL_OVERLAY_STATE, errorOverlayReducer, @@ -36,8 +35,6 @@ import { } from './internal/helpers/use-websocket' import { parseComponentStack } from './internal/helpers/parse-component-stack' import type { VersionInfo } from '../../../server/dev/parse-version-info' -import { isNotFoundError } from '../not-found' -import { NotFoundBoundary } from '../not-found-boundary' interface Dispatcher { onBuildOk(): void @@ -45,7 +42,6 @@ interface Dispatcher { onVersionInfo(versionInfo: VersionInfo): void onBeforeRefresh(): void onRefresh(): void - onNotFound(): void } // TODO-APP: add actual type @@ -54,8 +50,6 @@ type PongEvent = any let mostRecentCompilationHash: any = null let __nextDevClientId = Math.round(Math.random() * 100 + Date.now()) -// let startLatency = undefined - function onBeforeFastRefresh(dispatcher: Dispatcher, hasUpdates: boolean) { if (hasUpdates) { dispatcher.onBeforeRefresh() @@ -422,18 +416,30 @@ function processMessage( fetch(window.location.href, { credentials: 'same-origin', }).then((pageRes) => { - if (pageRes.status === 200) { - // Page exists now, reload - startTransition(() => { - // @ts-ignore it exists, it's just hidden - router.fastRefresh() - dispatcher.onRefresh() - }) - } else if (pageRes.status === 404) { + let shouldRefresh = pageRes.ok + // TODO-APP: investigate why edge runtime needs to reload + const isEdgeRuntime = pageRes.headers.get('x-edge-runtime') === '1' + if (pageRes.status === 404) { + // Check if head present as document.head could be null // We are still on the page, // dispatch an error so it's caught by the NotFound handler - dispatcher.onNotFound() + const devErrorMetaTag = document.head?.querySelector( + 'meta[name="next-error"]' + ) + shouldRefresh = !devErrorMetaTag } + // Page exists now, reload + startTransition(() => { + if (shouldRefresh) { + if (isEdgeRuntime) { + window.location.reload() + } else { + // @ts-ignore it exists, it's just hidden + router.fastRefresh() + dispatcher.onRefresh() + } + } + }) }) } return @@ -450,15 +456,9 @@ function processMessage( export default function HotReload({ assetPrefix, children, - notFound, - notFoundStyles, - asNotFound, }: { assetPrefix: string children?: ReactNode - notFound?: React.ReactNode - notFoundStyles?: React.ReactNode - asNotFound?: boolean }) { const [state, dispatch] = useReducer( errorOverlayReducer, @@ -481,9 +481,6 @@ export default function HotReload({ onVersionInfo(versionInfo) { dispatch({ type: ACTION_VERSION_INFO, versionInfo }) }, - onNotFound() { - dispatch({ type: ACTION_NOT_FOUND }) - }, } }, [dispatch]) @@ -505,9 +502,7 @@ export default function HotReload({ frames: parseStack(reason.stack!), }) }, []) - const handleOnReactError = useCallback((error: Error) => { - // not found errors are handled by the parent boundary, not the dev overlay - if (isNotFoundError(error)) throw error + const handleOnReactError = useCallback(() => { RuntimeErrorHandler.hadRuntimeError = true }, []) useErrorHandler(handleOnUnhandledError, handleOnUnhandledRejection) @@ -538,15 +533,8 @@ export default function HotReload({ }, [sendMessage, router, webSocketRef, dispatcher]) return ( - - - {children} - - + + {children} + ) } diff --git a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx index 5b88f1b03c00f..715e3fd1b9571 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx @@ -13,7 +13,6 @@ import { parseStack } from './helpers/parseStack' import { Base } from './styles/Base' import { ComponentStyles } from './styles/ComponentStyles' import { CssReset } from './styles/CssReset' -import { notFound } from '../../not-found' interface ReactDevOverlayState { reactError: SupportedErrorEvent | null @@ -59,10 +58,6 @@ class ReactDevOverlay extends React.PureComponent< reactError || rootLayoutMissingTagsError - if (state.notFound) { - notFound() - } - return ( <> {reactError ? ( diff --git a/packages/next/src/client/components/react-dev-overlay/internal/error-overlay-reducer.ts b/packages/next/src/client/components/react-dev-overlay/internal/error-overlay-reducer.ts index f9b44e7723c40..44cb2470db7a4 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/error-overlay-reducer.ts +++ b/packages/next/src/client/components/react-dev-overlay/internal/error-overlay-reducer.ts @@ -10,7 +10,6 @@ export const ACTION_REFRESH = 'fast-refresh' export const ACTION_UNHANDLED_ERROR = 'unhandled-error' export const ACTION_UNHANDLED_REJECTION = 'unhandled-rejection' export const ACTION_VERSION_INFO = 'version-info' -export const ACTION_NOT_FOUND = 'not-found' export const INITIAL_OVERLAY_STATE: OverlayState = { nextId: 1, buildError: null, @@ -34,10 +33,6 @@ interface FastRefreshAction { type: typeof ACTION_REFRESH } -interface NotFoundAction { - type: typeof ACTION_NOT_FOUND -} - export interface UnhandledErrorAction { type: typeof ACTION_UNHANDLED_ERROR reason: Error @@ -96,7 +91,6 @@ export const errorOverlayReducer: React.Reducer< | BuildErrorAction | BeforeFastRefreshAction | FastRefreshAction - | NotFoundAction | UnhandledErrorAction | UnhandledRejectionAction | VersionInfoAction @@ -104,7 +98,7 @@ export const errorOverlayReducer: React.Reducer< > = (state, action) => { switch (action.type) { case ACTION_BUILD_OK: { - return { ...state, buildError: null, notFound: false } + return { ...state, buildError: null } } case ACTION_BUILD_ERROR: { return { ...state, buildError: action.message } @@ -112,14 +106,10 @@ export const errorOverlayReducer: React.Reducer< case ACTION_BEFORE_REFRESH: { return { ...state, refreshState: { type: 'pending', errors: [] } } } - case ACTION_NOT_FOUND: { - return { ...state, notFound: true } - } case ACTION_REFRESH: { return { ...state, buildError: null, - notFound: false, errors: // Errors can come in during updates. In this case, UNHANDLED_ERROR // and UNHANDLED_REJECTION events might be dispatched between the diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 1a5b951c8a52d..56acc6127cfbe 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -17,10 +17,7 @@ import type { RequestAsyncStorage } from '../../client/components/request-async- import React from 'react' import { NotFound as DefaultNotFound } from '../../client/components/error' -import { - createServerComponentRenderer, - ErrorHtml, -} from './create-server-components-renderer' +import { createServerComponentRenderer } from './create-server-components-renderer' import { ParsedUrlQuery } from 'querystring' import { NextParsedUrlQuery } from '../request-meta' @@ -30,6 +27,7 @@ import { createBufferedTransformStream, continueFromInitialStream, streamToBufferedResult, + cloneTransformStream, } from '../stream-utils/node-web-streams-helper' import { canSegmentBeOverridden, @@ -81,8 +79,6 @@ import { appendMutableCookies } from '../web/spec-extension/adapters/request-coo import { ComponentsType } from '../../build/webpack/loaders/next-app-loader' import { ModuleReference } from '../../build/webpack/loaders/metadata/types' -export const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge' - export type GetDynamicParamFromSegment = ( // [slug] / [[slug]] / [...slug] segment: string @@ -93,6 +89,19 @@ export type GetDynamicParamFromSegment = ( type: DynamicParamTypesShort } | null +function ErrorHtml({ + children, +}: { + head?: React.ReactNode + children?: React.ReactNode +}) { + return ( + + {children} + + ) +} + // Find the closest matched component in the loader tree for a given component type function findMatchedComponent( loaderTree: LoaderTree, @@ -605,7 +614,7 @@ export async function renderToHTMLOrFlight( firstItem?: boolean injectedCSS: Set injectedFontPreloadTags: Set - asNotFound?: boolean + asNotFound?: boolean | 'force' }): Promise<{ Component: React.ComponentType styles: React.ReactNode @@ -935,12 +944,26 @@ export async function renderToHTMLOrFlight( // If it's a not found route, and we don't have any matched parallel // routes, we try to render the not found component if it exists. + let isLeaf = + process.env.NODE_ENV === 'production' + ? !segment && !rootLayoutIncluded + : !parallelRouteMap.length && segment === '__DEFAULT__' // hit parallel-route-default + let notFoundComponent = {} - if (asNotFound && !parallelRouteMap.length && NotFound) { + if ( + NotFound && + // For action not-found we force render the NotFound and stop checking the parallel routes. + (asNotFound === 'force' || + // For normal case where we should look up for not-found, keep checking the parallel routes. + (asNotFound && isLeaf)) + ) { notFoundComponent = { children: ( <> + {process.env.NODE_ENV === 'development' && ( + + )} {notFoundStyles} @@ -1287,11 +1310,6 @@ export async function renderToHTMLOrFlight( Uint8Array > = new TransformStream() - const serverErrorComponentsInlinedTransformStream: TransformStream< - Uint8Array, - Uint8Array - > = new TransformStream() - // Get the nonce from the incoming request if it has one. const csp = req.headers['content-security-policy'] let nonce: string | undefined @@ -1306,13 +1324,6 @@ export async function renderToHTMLOrFlight( rscChunks: [], } - const serverErrorComponentsRenderOpts = { - transformStream: serverErrorComponentsInlinedTransformStream, - clientReferenceManifest, - serverContexts, - rscChunks: [], - } - const validateRootLayout = dev ? { validateRootLayout: { @@ -1332,32 +1343,47 @@ export async function renderToHTMLOrFlight( injectedCSS: Set, requestPathname: string ) { - const { layout } = tree[2] // `depth` represents how many layers we need to search into the tree. // For instance: // pathname '/abc' will be 0 depth, means stop at the root level // pathname '/abc/def' will be 1 depth, means stop at the first level const depth = requestPathname.split('/').length - 2 const notFound = findMatchedComponent(tree, 'not-found', depth) - const rootLayoutAtThisLevel = typeof layout !== 'undefined' const [NotFound, notFoundStyles] = notFound ? await createComponentAndStyles({ filePath: notFound[1], getComponent: notFound[0], injectedCSS, }) - : rootLayoutAtThisLevel - ? [DefaultNotFound] : [] return [NotFound, notFoundStyles] } + async function getRootLayout( + tree: LoaderTree, + injectedCSS: Set, + injectedFontPreloadTags: Set + ) { + const { layout } = tree[2] + const layoutPath = layout?.[1] + const styles = getLayerAssets({ + layoutOrPagePath: layoutPath, + injectedCSS: new Set(injectedCSS), + injectedFontPreloadTags: new Set(injectedFontPreloadTags), + }) + const rootLayoutModule = layout?.[0] + const RootLayout = rootLayoutModule + ? interopDefault(await rootLayoutModule()) + : null + return [RootLayout, styles] + } + /** * A new React Component that renders the provided React Component * using Flight which can then be rendered to HTML. */ const ServerComponentsRenderer = createServerComponentRenderer<{ - asNotFound: boolean + asNotFound: boolean | 'force' }>( async (props) => { // Create full component tree from root to leaf. @@ -1375,12 +1401,6 @@ export async function renderToHTMLOrFlight( asNotFound: props.asNotFound, }) - const initialTree = createFlightRouterStateFromLoaderTree( - loaderTree, - getDynamicParamFromSegment, - query - ) - const createMetadata = (tree: LoaderTree, errorType?: 'not-found') => ( // Adding key={requestId} to make metadata remount for each render // @ts-expect-error allow to use async server component @@ -1395,10 +1415,10 @@ export async function renderToHTMLOrFlight( /> ) - const [NotFound, notFoundStyles] = await getNotFound( + const initialTree = createFlightRouterStateFromLoaderTree( loaderTree, - injectedCSS, - pathname + getDynamicParamFromSegment, + query ) return ( @@ -1409,18 +1429,11 @@ export async function renderToHTMLOrFlight( assetPrefix={assetPrefix} initialCanonicalUrl={pathname} initialTree={initialTree} - initialHead={<>{createMetadata(loaderTree, undefined)}} + initialHead={createMetadata( + loaderTree, + props.asNotFound ? 'not-found' : undefined + )} globalErrorComponent={GlobalError} - notFound={ - NotFound ? ( - - {createMetadata(loaderTree, 'not-found')} - {notFoundStyles} - - - ) : undefined - } - asNotFound={props.asNotFound} > @@ -1475,8 +1488,10 @@ export async function renderToHTMLOrFlight( * This option is used to indicate that the page should be rendered as * if it was not found. When it's enabled, instead of rendering the * page component, it renders the not-found segment. + * + * If it's 'force', we don't traverse the tree and directly render the NotFound. */ - asNotFound?: boolean + asNotFound: boolean | 'force' }) => { const polyfills = buildManifest.polyfillFiles .filter( @@ -1492,7 +1507,7 @@ export async function renderToHTMLOrFlight( const content = ( - + ) @@ -1508,9 +1523,17 @@ export async function renderToHTMLOrFlight( flushedErrorMetaTagsUntilIndex++ ) { const error = serverCapturedErrors[flushedErrorMetaTagsUntilIndex] + if (isNotFoundError(error)) { errorMetaTags.push( - + , + process.env.NODE_ENV === 'development' ? ( + + ) : null ) } else if (isRedirectError(error)) { const redirectUrl = getURLFromRedirectError(error) @@ -1586,7 +1609,7 @@ export async function renderToHTMLOrFlight( }) const result = await continueFromInitialStream(renderStream, { - dataStream: serverComponentsInlinedTransformStream.readable, + dataStream: serverComponentsRenderOpts.transformStream.readable, generateStaticHTML: staticGenerationStore.isStaticGeneration || generateStaticHTML, getServerInsertedHTML: () => @@ -1612,6 +1635,7 @@ export async function renderToHTMLOrFlight( pagePath ) } + if (isNotFoundError(err)) { res.statusCode = 404 } @@ -1631,104 +1655,154 @@ export async function renderToHTMLOrFlight( res.setHeader('Location', getURLFromRedirectError(err)) } - const use404Error = res.statusCode === 404 - const useDefaultError = res.statusCode < 400 || hasRedirectError + const is404 = res.statusCode === 404 - const { layout } = loaderTree[2] const injectedCSS = new Set() + const injectedFontPreloadTags = new Set() + const [RootLayout, rootStyles] = await getRootLayout( + loaderTree, + injectedCSS, + injectedFontPreloadTags + ) const [NotFound, notFoundStyles] = await getNotFound( loaderTree, injectedCSS, pathname ) - const rootLayoutModule = layout?.[0] - const RootLayout = rootLayoutModule - ? interopDefault(await rootLayoutModule()) - : null - - const metadata = ( - // @ts-expect-error allow to use async server component - - ) - const serverErrorElement = ( - - {useDefaultError - ? null - : React.createElement( - createServerComponentRenderer( - async () => { - return ( - <> - {/* For server components error metadata needs to be inside inline flight data, so they can be hydrated */} - {metadata} - {use404Error ? ( - - {notFoundStyles} - - - - ) : undefined} - - ) - }, - ComponentMod, - serverErrorComponentsRenderOpts, - serverComponentsErrorHandler, - nonce - ) - )} - + // Preserve the existing RSC inline chunks from the page rendering. + // For 404 errors: the metadata from layout can be skipped with the error page. + // For other errors (such as redirection): it can still be re-thrown on client. + const serverErrorComponentsRenderOpts: typeof serverComponentsRenderOpts = + { + ...serverComponentsRenderOpts, + rscChunks: [], + transformStream: is404 + ? new TransformStream() + : cloneTransformStream( + serverComponentsRenderOpts.transformStream + ), + } + + const errorType = is404 + ? 'not-found' + : hasRedirectError + ? 'redirect' + : undefined + + const errorMeta = ( + <> + {res.statusCode >= 400 && ( + + )} + {process.env.NODE_ENV === 'development' && ( + + )} + ) + const ErrorPage = createServerComponentRenderer( + async () => { + const head = ( + <> + {/* @ts-expect-error allow to use async server component */} + + {errorMeta} + + ) - const renderStream = await renderToInitialStream({ - ReactDOMServer: require('react-dom/server.edge'), - element: serverErrorElement, - streamOptions: { - nonce, - // Include hydration scripts in the HTML - bootstrapScripts: subresourceIntegrityManifest - ? buildManifest.rootMainFiles.map((src) => ({ - src: - `${assetPrefix}/_next/` + - src + - getAssetQueryString(false), - integrity: subresourceIntegrityManifest[src], - })) - : buildManifest.rootMainFiles.map( - (src) => - `${assetPrefix}/_next/` + src + getAssetQueryString(false) - ), + const notFoundLoaderTree: LoaderTree = is404 + ? ['__DEFAULT__', {}, loaderTree[2]] + : loaderTree + + const initialTree = createFlightRouterStateFromLoaderTree( + notFoundLoaderTree, + getDynamicParamFromSegment, + query + ) + + const GlobalNotFound = NotFound || DefaultNotFound + const ErrorLayout = RootLayout || ErrorHtml + + const notFoundElement = ( + + {rootStyles} + {notFoundStyles} + + + ) + + // For metadata notFound error there's no global not found boundary on top + // so we create a not found page with AppRouter + return ( + + {is404 ? notFoundElement : } + + ) }, - }) + ComponentMod, + serverErrorComponentsRenderOpts, + serverComponentsErrorHandler, + nonce + ) - return await continueFromInitialStream(renderStream, { - dataStream: (useDefaultError - ? serverComponentsInlinedTransformStream - : serverErrorComponentsInlinedTransformStream - ).readable, - generateStaticHTML: staticGenerationStore.isStaticGeneration, - getServerInsertedHTML: () => getServerInsertedHTML([]), - serverInsertedHTMLToHead: true, - ...validateRootLayout, - }) + try { + const renderStream = await renderToInitialStream({ + ReactDOMServer: require('react-dom/server.edge'), + element: , + streamOptions: { + nonce, + // Include hydration scripts in the HTML + bootstrapScripts: subresourceIntegrityManifest + ? buildManifest.rootMainFiles.map((src) => ({ + src: + `${assetPrefix}/_next/` + + src + + getAssetQueryString(false), + integrity: subresourceIntegrityManifest[src], + })) + : buildManifest.rootMainFiles.map( + (src) => + `${assetPrefix}/_next/` + + src + + getAssetQueryString(false) + ), + }, + }) + + return await continueFromInitialStream(renderStream, { + dataStream: + serverErrorComponentsRenderOpts.transformStream.readable, + generateStaticHTML: staticGenerationStore.isStaticGeneration, + getServerInsertedHTML: () => getServerInsertedHTML([]), + serverInsertedHTMLToHead: true, + ...validateRootLayout, + }) + } catch (finalErr: any) { + if ( + process.env.NODE_ENV !== 'production' && + isNotFoundError(finalErr) + ) { + const bailOnNotFound: typeof import('../../client/components/dev-root-not-found-boundary').bailOnNotFound = + require('../../client/components/dev-root-not-found-boundary').bailOnNotFound + bailOnNotFound() + } + throw finalErr + } } } ) @@ -1747,7 +1821,7 @@ export async function renderToHTMLOrFlight( }) if (actionRequestResult === 'not-found') { - return new RenderResult(await bodyResult({ asNotFound: true })) + return new RenderResult(await bodyResult({ asNotFound: 'force' })) } else if (actionRequestResult) { return actionRequestResult } diff --git a/packages/next/src/server/app-render/create-server-components-renderer.tsx b/packages/next/src/server/app-render/create-server-components-renderer.tsx index 5fdc1008bf819..0d11231ed110d 100644 --- a/packages/next/src/server/app-render/create-server-components-renderer.tsx +++ b/packages/next/src/server/app-render/create-server-components-renderer.tsx @@ -75,18 +75,3 @@ export function createServerComponentRenderer( return use(response) } } - -export function ErrorHtml({ - head, - children, -}: { - head?: React.ReactNode - children?: React.ReactNode -}) { - return ( - - {head} - {children} - - ) -} diff --git a/packages/next/src/server/lib/app-dir-module.ts b/packages/next/src/server/lib/app-dir-module.ts index 02e735a3edf7a..cc07efeff5321 100644 --- a/packages/next/src/server/lib/app-dir-module.ts +++ b/packages/next/src/server/lib/app-dir-module.ts @@ -36,7 +36,7 @@ export async function getLayoutOrPageModule(loaderTree: LoaderTree) { // First check not-found, if it doesn't exist then pick layout export async function getErrorOrLayoutModule( loaderTree: LoaderTree, - errorType: 'error' | 'not-found' + errorType: 'not-found' ) { const { [errorType]: error, layout } = loaderTree[2] if (typeof error !== 'undefined') { diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index 1e1ca4da11397..aab167b244f3c 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -457,7 +457,6 @@ export async function renderToHTMLImpl( let Document = extra.Document - // Component will be wrapped by ServerComponentWrapper for RSC let Component: React.ComponentType<{}> | ((props: any) => JSX.Element) = renderOpts.Component const OriginComponent = Component diff --git a/packages/next/src/server/stream-utils/node-web-streams-helper.ts b/packages/next/src/server/stream-utils/node-web-streams-helper.ts index 7f6312965902e..6b230596a4d9a 100644 --- a/packages/next/src/server/stream-utils/node-web-streams-helper.ts +++ b/packages/next/src/server/stream-utils/node-web-streams-helper.ts @@ -30,6 +30,25 @@ export const streamToBufferedResult = async ( return renderChunks.join('') } +export function cloneTransformStream(source: TransformStream) { + const sourceReader = source.readable.getReader() + const clone = new TransformStream({ + async start(controller) { + while (true) { + const { done, value } = await sourceReader.read() + if (done) { + break + } + controller.enqueue(value) + } + }, + // skip the its own written chunks + transform() {}, + }) + + return clone +} + export function readableStreamTee( readable: ReadableStream ): [ReadableStream, ReadableStream] { diff --git a/test/e2e/app-dir/actions/app/server/client-form.js b/test/e2e/app-dir/actions/app/server/client-form.js index 676adf93a9669..6c940e8974031 100644 --- a/test/e2e/app-dir/actions/app/server/client-form.js +++ b/test/e2e/app-dir/actions/app/server/client-form.js @@ -5,7 +5,7 @@ import { redirectAction } from './actions' export default function Form() { return (
- + + {children} + + + ) +} + +export const dynamic = 'force-dynamic' diff --git a/test/e2e/app-dir/root-layout-not-found/app/not-found-trigger.js b/test/e2e/app-dir/root-layout-not-found/app/not-found-trigger.js new file mode 100644 index 0000000000000..4fa8d4cba0d40 --- /dev/null +++ b/test/e2e/app-dir/root-layout-not-found/app/not-found-trigger.js @@ -0,0 +1,12 @@ +'use client' + +import { useSearchParams, notFound } from 'next/navigation' + +export default function NotFoundTrigger() { + const searchParams = useSearchParams() + + if (searchParams.get('root-not-found')) { + notFound() + } + return null +} diff --git a/test/e2e/app-dir/root-layout-not-found/app/page.js b/test/e2e/app-dir/root-layout-not-found/app/page.js new file mode 100644 index 0000000000000..ff7159d9149fe --- /dev/null +++ b/test/e2e/app-dir/root-layout-not-found/app/page.js @@ -0,0 +1,3 @@ +export default function Page() { + return

hello world

+} diff --git a/test/e2e/app-dir/root-layout-not-found/index.test.ts b/test/e2e/app-dir/root-layout-not-found/index.test.ts new file mode 100644 index 0000000000000..e3e86a78d64da --- /dev/null +++ b/test/e2e/app-dir/root-layout-not-found/index.test.ts @@ -0,0 +1,52 @@ +import { createNextDescribe } from 'e2e-utils' +import { check, getRedboxDescription, hasRedbox } from 'next-test-utils' + +createNextDescribe( + 'app dir - root layout not found', + { + files: __dirname, + skipDeployment: true, + }, + ({ next, isNextDev }) => { + it('should error on client notFound from root layout in browser', async () => { + const browser = await next.browser('/') + + await browser.elementByCss('#trigger-not-found').click() + + if (isNextDev) { + await check(async () => { + expect(await hasRedbox(browser, true)).toBe(true) + expect(await getRedboxDescription(browser)).toMatch( + /notFound\(\) is not allowed to use in root layout/ + ) + return 'success' + }, /success/) + } else { + expect(await browser.elementByCss('h2').text()).toBe( + 'Application error: a server-side exception has occurred (see the server logs for more information).' + ) + expect(await browser.elementByCss('p').text()).toBe( + 'Digest: NEXT_NOT_FOUND' + ) + } + }) + + it('should error on server notFound from root layout on server-side', async () => { + const browser = await next.browser('/?root-not-found=1') + + if (isNextDev) { + expect(await hasRedbox(browser, true)).toBe(true) + expect(await getRedboxDescription(browser)).toBe( + 'Error: notFound() is not allowed to use in root layout' + ) + } else { + expect(await browser.elementByCss('h2').text()).toBe( + 'Application error: a server-side exception has occurred (see the server logs for more information).' + ) + expect(await browser.elementByCss('p').text()).toBe( + 'Digest: NEXT_NOT_FOUND' + ) + } + }) + } +) From 4747c8dadfee677952e8a9bfce756f4eb807713f Mon Sep 17 00:00:00 2001 From: Alex Kirszenberg Date: Fri, 21 Jul 2023 14:30:26 +0200 Subject: [PATCH 2/7] Increase timeout for another 404 test --- .../tests/integration/next/app/404-custom/input/app/test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx b/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx index 3f059c305953e..fb3db2527086c 100644 --- a/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx +++ b/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx @@ -48,6 +48,7 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) { TIMEOUT ) + // TODO: This test is flaky, so it needs a particularly long timeout. it( 'renders a custom 404 page', async () => { @@ -58,7 +59,7 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) { iframe.contentDocument!.querySelector('[data-test-notfound]') ).not.toBeNull() }, - TIMEOUT + LONG_TIMEOUT ) // TODO: This test is flaky, so it needs a particularly long timeout. From cb45755074a058762ab885f6ec173344be03d005 Mon Sep 17 00:00:00 2001 From: Alex Kirszenberg Date: Fri, 21 Jul 2023 14:53:35 +0200 Subject: [PATCH 3/7] Skip timing out test --- .../tests/integration/next/app/404-custom/input/app/test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx b/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx index fb3db2527086c..932ac27a82f51 100644 --- a/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx +++ b/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx @@ -48,8 +48,8 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) { TIMEOUT ) - // TODO: This test is flaky, so it needs a particularly long timeout. - it( + // TODO(alexkirsz) This test is timing out. + it.skip( 'renders a custom 404 page', async () => { await harness.load(iframe, '/not-found') From 0151c5eb41ee38690da7c6b74a2f03cbe8e24dfe Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 21 Jul 2023 13:33:59 +0000 Subject: [PATCH 4/7] only print issues once --- .../crates/next-dev-tests/tests/integration.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/next-swc/crates/next-dev-tests/tests/integration.rs b/packages/next-swc/crates/next-dev-tests/tests/integration.rs index eb6be1c6f20c4..4fb9774242bc6 100644 --- a/packages/next-swc/crates/next-dev-tests/tests/integration.rs +++ b/packages/next-swc/crates/next-dev-tests/tests/integration.rs @@ -3,6 +3,7 @@ #![cfg(test)] use std::{ + collections::{hash_map::Entry, HashMap}, env, fmt::Write, future::{pending, Future}, @@ -34,6 +35,7 @@ use next_core::turbopack::{ }; use next_dev::{EntryRequest, NextDevServerBuilder}; use owo_colors::OwoColorize; +use parking_lot::Mutex; use regex::{Captures, Regex, Replacer}; use serde::Deserialize; use tempdir::TempDir; @@ -637,10 +639,12 @@ struct ChangeFileCommand { replace_with: String, } -#[turbo_tasks::value(shared)] +#[turbo_tasks::value(shared, serialization = "none", eq = "manual", cell = "new")] struct TestIssueReporter { #[turbo_tasks(trace_ignore, debug_ignore)] pub issue_tx: State, ReadRef)>>, + #[turbo_tasks(trace_ignore, debug_ignore)] + pub already_printed: Mutex>, } #[turbo_tasks::value_impl] @@ -653,6 +657,7 @@ impl TestIssueReporter { ) -> Vc { TestIssueReporter { issue_tx: State::new((*issue_tx).clone()), + already_printed: Default::default(), } .cell() } @@ -678,7 +683,11 @@ impl IssueReporter for TestIssueReporter { for (issue, path) in captured_issues.iter_with_shortest_path() { let plain = NormalizedIssue(issue).cell().into_plain(path); issue_tx.send((plain.await?, plain.dbg().await?))?; - println!("{}", format_issue(&*plain.await?, None, &log_options)); + let str = format_issue(&*plain.await?, None, &log_options); + if let Entry::Vacant(e) = self.already_printed.lock().entry(str) { + println!("{}", e.key()); + e.insert(()); + } } Ok(Vc::cell(false)) } From e5615d96a56d01e7893ed1a29507330ab8b9bb4d Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 21 Jul 2023 13:58:39 +0000 Subject: [PATCH 5/7] update turbopack code --- .../next-core/js/src/dev/hot-reloader.tsx | 23 ++----------------- .../src/overlay/internal/ReactDevOverlay.tsx | 5 ---- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/packages/next-swc/crates/next-core/js/src/dev/hot-reloader.tsx b/packages/next-swc/crates/next-core/js/src/dev/hot-reloader.tsx index 65fd5ad86e38d..0b1734bd4c140 100644 --- a/packages/next-swc/crates/next-core/js/src/dev/hot-reloader.tsx +++ b/packages/next-swc/crates/next-core/js/src/dev/hot-reloader.tsx @@ -5,22 +5,12 @@ import { useRouter, usePathname } from 'next/dist/client/components/navigation' import { useEffect } from 'react' import { subscribeToUpdate } from '@vercel/turbopack-ecmascript-runtime/dev/client/hmr-client' import { ReactDevOverlay } from './client' -import { NotFoundBoundary } from 'next/dist/client/components/not-found-boundary' type HotReloadProps = React.PropsWithChildren<{ assetPrefix?: string - notFound?: React.ReactNode - notFoundStyles?: React.ReactNode - asNotFound?: boolean }> -export default function HotReload({ - assetPrefix, - children, - notFound, - notFoundStyles, - asNotFound, -}: HotReloadProps) { +export default function HotReload({ children }: HotReloadProps) { const router = useRouter() const path = usePathname()!.slice(1) @@ -41,14 +31,5 @@ export default function HotReload({ return unsubscribe }, [router, path]) - return ( - - {children} - - ) + return {children} } diff --git a/packages/next-swc/crates/next-core/js/src/overlay/internal/ReactDevOverlay.tsx b/packages/next-swc/crates/next-core/js/src/overlay/internal/ReactDevOverlay.tsx index 102087028c998..b971951ae8f1e 100644 --- a/packages/next-swc/crates/next-core/js/src/overlay/internal/ReactDevOverlay.tsx +++ b/packages/next-swc/crates/next-core/js/src/overlay/internal/ReactDevOverlay.tsx @@ -7,7 +7,6 @@ import { ErrorBoundary } from './ErrorBoundary' import { Base } from './styles/Base' import { ComponentStyles } from './styles/ComponentStyles' import { CssReset } from './styles/CssReset' -import { notFound } from 'next/dist/client/components/not-found' type RefreshState = | { @@ -210,10 +209,6 @@ export default function ReactDevOverlay({ const isMounted = hasBuildError || hasRuntimeErrors - if (state.notFound) { - notFound() - } - return ( Date: Fri, 21 Jul 2023 13:59:13 +0000 Subject: [PATCH 6/7] Revert "Skip timing out test" This reverts commit cb45755074a058762ab885f6ec173344be03d005. --- .../tests/integration/next/app/404-custom/input/app/test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx b/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx index 932ac27a82f51..fb3db2527086c 100644 --- a/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx +++ b/packages/next-swc/crates/next-dev-tests/tests/integration/next/app/404-custom/input/app/test.tsx @@ -48,8 +48,8 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) { TIMEOUT ) - // TODO(alexkirsz) This test is timing out. - it.skip( + // TODO: This test is flaky, so it needs a particularly long timeout. + it( 'renders a custom 404 page', async () => { await harness.load(iframe, '/not-found') From a9a3c7e692cdce826faf04664cc569975c1c33f9 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 21 Jul 2023 18:04:10 +0200 Subject: [PATCH 7/7] add loader tree for not found in turbopack --- .../next-swc/crates/next-core/src/app_source.rs | 2 +- .../crates/next-core/src/app_structure.rs | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/next-swc/crates/next-core/src/app_source.rs b/packages/next-swc/crates/next-core/src/app_source.rs index 04aed92c48a79..61843cbbb07a5 100644 --- a/packages/next-swc/crates/next-core/src/app_source.rs +++ b/packages/next-swc/crates/next-core/src/app_source.rs @@ -601,7 +601,7 @@ pub async fn create_app_source( ))) .collect(); - if let Some(&Entrypoint::AppPage { loader_tree }) = entrypoints.get("/") { + if let Some(&Entrypoint::AppPage { loader_tree }) = entrypoints.get("/_not-found") { if loader_tree.await?.components.await?.not_found.is_some() { // Only add a source for the app 404 page if a top-level not-found page is // defined. Otherwise, the 404 page is handled by the pages logic. diff --git a/packages/next-swc/crates/next-core/src/app_structure.rs b/packages/next-swc/crates/next-core/src/app_structure.rs index 58bdf032ea7e5..a733a9c4aad2d 100644 --- a/packages/next-swc/crates/next-core/src/app_structure.rs +++ b/packages/next-swc/crates/next-core/src/app_structure.rs @@ -14,7 +14,7 @@ use turbopack_binding::{ turbopack::core::issue::{Issue, IssueExt, IssueSeverity}, }; -use crate::next_config::NextConfig; +use crate::{next_config::NextConfig, next_import_map::get_next_package}; /// A final route in the app directory. #[turbo_tasks::value] @@ -675,9 +675,16 @@ async fn directory_tree_to_entrypoints_internal( LoaderTree { segment: directory_name.to_string(), parallel_routes: indexmap! { - // TODO(alexkirsz) Next.js has a __DEFAULT__ entry for - // next/dist/client/components/parallel-route-default - // here. + "children".to_string() => LoaderTree { + segment: "__DEFAULT__".to_string(), + parallel_routes: IndexMap::new(), + components: Components { + default: Some(get_next_package(app_dir).join("dist/client/components/parallel-route-default.js".to_string())), + ..Default::default() + } + .cell(), + } + .cell(), }, components: components.without_leafs().cell(), }