From 8b802ddb888f598512e820df894f16abcd795c0a Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 26 Sep 2022 13:16:20 +0200 Subject: [PATCH] Update handling of redirect/404 throw to cross server->client boundary (#40890) --- .../components/layout-router.client.tsx | 7 +++--- packages/next/client/components/not-found.ts | 6 ++--- packages/next/client/components/redirect.ts | 3 +-- packages/next/export/worker.ts | 4 +++- packages/next/server/app-render.tsx | 24 ++++--------------- test/e2e/app-dir/index.test.ts | 3 +-- 6 files changed, 16 insertions(+), 31 deletions(-) diff --git a/packages/next/client/components/layout-router.client.tsx b/packages/next/client/components/layout-router.client.tsx index bbe5e3d5c247..f79d188ef0b7 100644 --- a/packages/next/client/components/layout-router.client.tsx +++ b/packages/next/client/components/layout-router.client.tsx @@ -309,8 +309,9 @@ class RedirectErrorBoundary extends React.Component< } static getDerivedStateFromError(error: any) { - if (error.digest === 'NEXT_REDIRECT') { - return { redirect: error.url } + if (error.digest?.startsWith('NEXT_REDIRECT')) { + const url = error.digest.split(';')[1] + return { redirect: url } } // Re-throw if error is not for 404 throw error @@ -354,7 +355,7 @@ class NotFoundErrorBoundary extends React.Component< } static getDerivedStateFromError(error: any) { - if (error.code === 'NEXT_NOT_FOUND') { + if (error.digest === 'NEXT_NOT_FOUND') { return { notFoundTriggered: true } } // Re-throw if error is not for 404 diff --git a/packages/next/client/components/not-found.ts b/packages/next/client/components/not-found.ts index a343c5a81ab8..e5cad6a30139 100644 --- a/packages/next/client/components/not-found.ts +++ b/packages/next/client/components/not-found.ts @@ -2,7 +2,7 @@ export const NOT_FOUND_ERROR_CODE = 'NEXT_NOT_FOUND' export function notFound() { // eslint-disable-next-line no-throw-literal - throw { - code: NOT_FOUND_ERROR_CODE, - } + const error = new Error(NOT_FOUND_ERROR_CODE) + ;(error as any).digest = NOT_FOUND_ERROR_CODE + throw error } diff --git a/packages/next/client/components/redirect.ts b/packages/next/client/components/redirect.ts index 396bd7655ba1..0b500ae87256 100644 --- a/packages/next/client/components/redirect.ts +++ b/packages/next/client/components/redirect.ts @@ -3,7 +3,6 @@ export const REDIRECT_ERROR_CODE = 'NEXT_REDIRECT' export function redirect(url: string) { // eslint-disable-next-line no-throw-literal const error = new Error(REDIRECT_ERROR_CODE) - ;(error as any).url = url - ;(error as any).digest = REDIRECT_ERROR_CODE + ;(error as any).digest = REDIRECT_ERROR_CODE + ';' + url throw error } diff --git a/packages/next/export/worker.ts b/packages/next/export/worker.ts index d649c575b9fe..3f2d37774300 100644 --- a/packages/next/export/worker.ts +++ b/packages/next/export/worker.ts @@ -30,6 +30,7 @@ import { addRequestMeta } from '../server/request-meta' import { normalizeAppPath } from '../shared/lib/router/utils/app-paths' import { REDIRECT_ERROR_CODE } from '../client/components/redirect' import { DYNAMIC_ERROR_CODE } from '../client/components/hooks-server-context' +import { NOT_FOUND_ERROR_CODE } from '../client/components/not-found' loadRequireHook() const envConfig = require('../shared/lib/runtime-config') @@ -419,7 +420,8 @@ export default async function exportPage({ } catch (err: any) { if ( err.digest !== DYNAMIC_ERROR_CODE && - err.digest !== REDIRECT_ERROR_CODE + err.digest !== NOT_FOUND_ERROR_CODE && + !err.digest?.startsWith(REDIRECT_ERROR_CODE) ) { throw err } diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 1834f62e261b..dc79969c9684 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -31,6 +31,7 @@ import type { ComponentsType } from '../build/webpack/loaders/next-app-loader' import { REDIRECT_ERROR_CODE } from '../client/components/redirect' import { NextCookies } from './web/spec-extension/cookies' import { DYNAMIC_ERROR_CODE } from '../client/components/hooks-server-context' +import { NOT_FOUND_ERROR_CODE } from '../client/components/not-found' const INTERNAL_HEADERS_INSTANCE = Symbol('internal for headers readonly') @@ -173,7 +174,8 @@ function createErrorHandler( if ( // TODO-APP: Handle redirect throw err.digest !== DYNAMIC_ERROR_CODE && - err.digest !== REDIRECT_ERROR_CODE + err.digest !== NOT_FOUND_ERROR_CODE && + !err.digest?.startsWith(REDIRECT_ERROR_CODE) ) { // Used for debugging error source // console.error(_source, err) @@ -1299,10 +1301,6 @@ export async function renderToHTMLOrFlight( flushEffectsToHead: true, }) } catch (err: any) { - if (err.digest === REDIRECT_ERROR_CODE) { - throw err - } - // TODO-APP: show error overlay in development. `element` should probably be wrapped in AppRouter for this case. const renderStream = await renderToInitialStream({ ReactDOMServer, @@ -1358,21 +1356,7 @@ export async function renderToHTMLOrFlight( return new RenderResult(staticHtml) } - try { - return new RenderResult(await bodyResult()) - } catch (err: any) { - if (err.digest === REDIRECT_ERROR_CODE) { - ;(renderOpts as any).pageData = { - pageProps: { - __N_REDIRECT: err.url, - __N_REDIRECT_STATUS: 307, - }, - } - ;(renderOpts as any).isRedirect = true - return RenderResult.fromStatic('') - } - throw err - } + return new RenderResult(await bodyResult()) } const initialStaticGenerationStore = { diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index ae7c61ea75a2..02752cc8531f 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1490,8 +1490,7 @@ describe('app dir', () => { await browser.waitForElementByCss('#not-found-component').text() ).toBe('404!') }) - - it('should trigger 404 client-side', async () => { + ;(isDev ? it.skip : it)('should trigger 404 client-side', async () => { const browser = await webdriver(next.url, '/not-found/client-side') await browser .elementByCss('button')