From 98b99e408bf205492e373339519bb861cb832890 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 11 Jan 2024 23:28:17 +0100 Subject: [PATCH] Fix global-error for nested routes (#60539) ## What This fixes when the deep nested routes throws a client side error, it can still be caught by the `global-error.js` ## How We should always resolve global-error from root app directory instead of current route's layout. Also fixed a bad test before where the gloabl-error.js is not located correctly Fixes #53756 Closes NEXT-1760 --- .../build/webpack/loaders/next-app-loader.ts | 20 +++++++++---------- .../global-error/basic/app/global-error.js | 3 +++ .../global-error/basic/app/nested/layout.js | 3 +++ .../basic/app/nested/nested/page.js | 19 ++++++++++++++++++ .../app-dir/global-error/basic/index.test.ts | 12 +++++++++++ .../app/{[lang] => }/global-error.js | 0 6 files changed, 47 insertions(+), 10 deletions(-) create mode 100644 test/e2e/app-dir/global-error/basic/app/nested/layout.js create mode 100644 test/e2e/app-dir/global-error/basic/app/nested/nested/page.js rename test/e2e/app-dir/global-error/catch-all/app/{[lang] => }/global-error.js (100%) diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts index 8fc5ab01c0..5fdae7e867 100644 --- a/packages/next/src/build/webpack/loaders/next-app-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-app-loader.ts @@ -198,7 +198,7 @@ async function createTreeCodeFromPath( const pages: string[] = [] let rootLayout: string | undefined - let globalError: string = defaultGlobalErrorPath + let globalError: string | undefined async function resolveAdjacentParallelSegments( segmentPath: string @@ -356,18 +356,18 @@ async function createTreeCodeFromPath( )?.[1] rootLayout = layoutPath - if (isDefaultNotFound && !layoutPath) { + if (isDefaultNotFound && !layoutPath && !rootLayout) { rootLayout = defaultLayoutPath definedFilePaths.push(['layout', rootLayout]) } + } - if (layoutPath) { - const resolvedGlobalErrorPath = await resolver( - `${path.dirname(layoutPath)}/${GLOBAL_ERROR_FILE_TYPE}` - ) - if (resolvedGlobalErrorPath) { - globalError = resolvedGlobalErrorPath - } + if (!globalError) { + const resolvedGlobalErrorPath = await resolver( + `${appDirPrefix}/${GLOBAL_ERROR_FILE_TYPE}` + ) + if (resolvedGlobalErrorPath) { + globalError = resolvedGlobalErrorPath } } @@ -468,7 +468,7 @@ async function createTreeCodeFromPath( treeCode: `${treeCode}.children;`, pages: `${JSON.stringify(pages)};`, rootLayout, - globalError, + globalError: globalError ?? defaultGlobalErrorPath, } } diff --git a/test/e2e/app-dir/global-error/basic/app/global-error.js b/test/e2e/app-dir/global-error/basic/app/global-error.js index d960556d75..40d18f27cf 100644 --- a/test/e2e/app-dir/global-error/basic/app/global-error.js +++ b/test/e2e/app-dir/global-error/basic/app/global-error.js @@ -12,3 +12,6 @@ export default function GlobalError({ error }) { ) } + +// for inspecting purpose +GlobalError.displayName = 'GlobalError' diff --git a/test/e2e/app-dir/global-error/basic/app/nested/layout.js b/test/e2e/app-dir/global-error/basic/app/nested/layout.js new file mode 100644 index 0000000000..6fe147a36d --- /dev/null +++ b/test/e2e/app-dir/global-error/basic/app/nested/layout.js @@ -0,0 +1,3 @@ +export default function NestedLayout({ children }) { + return
{children}
+} diff --git a/test/e2e/app-dir/global-error/basic/app/nested/nested/page.js b/test/e2e/app-dir/global-error/basic/app/nested/nested/page.js new file mode 100644 index 0000000000..135d9845b2 --- /dev/null +++ b/test/e2e/app-dir/global-error/basic/app/nested/nested/page.js @@ -0,0 +1,19 @@ +'use client' + +import { useEffect, useState } from 'react' + +export default function ClientPage() { + const [bad, setBad] = useState(false) + + useEffect(() => { + setTimeout(() => { + setBad(true) + }) + }, []) + + if (bad) { + throw Error('nested error') + } + + return

client page

+} diff --git a/test/e2e/app-dir/global-error/basic/index.test.ts b/test/e2e/app-dir/global-error/basic/index.test.ts index 1041c89d40..9fad140712 100644 --- a/test/e2e/app-dir/global-error/basic/index.test.ts +++ b/test/e2e/app-dir/global-error/basic/index.test.ts @@ -79,5 +79,17 @@ createNextDescribe( ) } }) + + it('should catch the client error thrown in the nested routes', async () => { + const browser = await next.browser('/nested/nested') + if (isNextDev) { + await testDev(browser, /Error: nested error/) + } else { + expect(await browser.elementByCss('h1').text()).toBe('Global Error') + expect(await browser.elementByCss('#error').text()).toBe( + 'Global error: nested error' + ) + } + }) } ) diff --git a/test/e2e/app-dir/global-error/catch-all/app/[lang]/global-error.js b/test/e2e/app-dir/global-error/catch-all/app/global-error.js similarity index 100% rename from test/e2e/app-dir/global-error/catch-all/app/[lang]/global-error.js rename to test/e2e/app-dir/global-error/catch-all/app/global-error.js