From 9370c7c1cf29c0070cf1ef250c0a21e9f7731cfd Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Wed, 19 Feb 2025 14:45:00 +0100 Subject: [PATCH] Consolidate `next/link` error tests --- packages/next/src/client/link.tsx | 3 +- .../acceptance/ReactRefreshLogBoxMisc.test.ts | 230 ------------------ .../ReactRefreshLogBoxMisc.test.ts.snap | 11 - .../pages-dir/client-navigation/index.test.ts | 21 -- .../server-source-maps.test.ts | 1 + .../next-link-errors/app/invalid-href/page.js | 5 + test/e2e/next-link-errors/app/layout.js | 7 + .../app/multiple-children/page.js | 10 + .../next-link-errors/app/no-children/page.js | 6 + .../next-link-errors/next-link-errors.test.ts | 91 +++++++ test/e2e/next-link-errors/next.config.js | 6 + test/turbopack-dev-tests-manifest.json | 11 - 12 files changed, 128 insertions(+), 274 deletions(-) delete mode 100644 test/development/acceptance/ReactRefreshLogBoxMisc.test.ts delete mode 100644 test/development/acceptance/__snapshots__/ReactRefreshLogBoxMisc.test.ts.snap create mode 100644 test/e2e/next-link-errors/app/invalid-href/page.js create mode 100644 test/e2e/next-link-errors/app/layout.js create mode 100644 test/e2e/next-link-errors/app/multiple-children/page.js create mode 100644 test/e2e/next-link-errors/app/no-children/page.js create mode 100644 test/e2e/next-link-errors/next-link-errors.test.ts create mode 100644 test/e2e/next-link-errors/next.config.js diff --git a/packages/next/src/client/link.tsx b/packages/next/src/client/link.tsx index 685033eedd5572..ac8b1adc0d2b86 100644 --- a/packages/next/src/client/link.tsx +++ b/packages/next/src/client/link.tsx @@ -293,7 +293,8 @@ const Link = React.forwardRef( return new Error( `Failed prop type: The prop \`${args.key}\` expects a ${args.expected} in \`\`, but got \`${args.actual}\` instead.` + (typeof window !== 'undefined' - ? "\nOpen your browser's console to view the Component stack trace." + ? // TODO: Remove this addendum if Owner Stacks are available + "\nOpen your browser's console to view the Component stack trace." : '') ) } diff --git a/test/development/acceptance/ReactRefreshLogBoxMisc.test.ts b/test/development/acceptance/ReactRefreshLogBoxMisc.test.ts deleted file mode 100644 index 27823b524f71d6..00000000000000 --- a/test/development/acceptance/ReactRefreshLogBoxMisc.test.ts +++ /dev/null @@ -1,230 +0,0 @@ -import { createSandbox } from 'development-sandbox' -import { FileRef, nextTestSetup } from 'e2e-utils' -import path from 'path' -import { outdent } from 'outdent' - -// TODO: re-enable these tests after figuring out what is causing -// them to be so unreliable in CI -describe.skip('ReactRefreshLogBox', () => { - const { next } = nextTestSetup({ - files: new FileRef(path.join(__dirname, 'fixtures', 'default-template')), - skipStart: true, - }) - - test(' with multiple children', async () => { - await using sandbox = await createSandbox(next) - const { session } = sandbox - - await session.patch( - 'index.js', - outdent` - import Link from 'next/link' - - export default function Index() { - return ( - -

One

-

Two

- - ) - } - ` - ) - - await session.assertHasRedbox() - expect(await session.getRedboxDescription()).toMatchInlineSnapshot( - `"Error: Multiple children were passed to with \`href\` of \`/\` but only one child is supported https://nextjs.org/docs/messages/link-multiple-children"` - ) - expect( - await session.evaluate( - () => - ( - document - .querySelector('body > nextjs-portal') - .shadowRoot.querySelector( - '#nextjs__container_errors_desc a:nth-of-type(1)' - ) as any - ).href - ) - ).toMatch('https://nextjs.org/docs/messages/link-multiple-children') - }) - - test(' component props errors', async () => { - await using sandbox = await createSandbox(next) - const { session } = sandbox - - await session.patch( - 'index.js', - outdent` - import Link from 'next/link' - - export default function Hello() { - return - } - ` - ) - - await session.assertHasRedbox() - expect(await session.getRedboxDescription()).toMatchInlineSnapshot( - `"Error: Failed prop type: The prop \`href\` expects a \`string\` or \`object\` in \`\`, but got \`undefined\` instead."` - ) - - await session.patch( - 'index.js', - outdent` - import Link from 'next/link' - - export default function Hello() { - return Abc - } - ` - ) - await session.assertNoRedbox() - - await session.patch( - 'index.js', - outdent` - import Link from 'next/link' - - export default function Hello() { - return ( - - Abc - - ) - } - ` - ) - await session.assertNoRedbox() - - await session.patch( - 'index.js', - outdent` - import Link from 'next/link' - - export default function Hello() { - return ( - - Abc - - ) - } - ` - ) - await session.assertNoRedbox() - - await session.patch( - 'index.js', - outdent` - import Link from 'next/link' - - export default function Hello() { - return ( - - Abc - - ) - } - ` - ) - await session.assertNoRedbox() - - await session.patch( - 'index.js', - outdent` - import Link from 'next/link' - - export default function Hello() { - return ( - - Abc - - ) - } - ` - ) - await session.assertHasRedbox() - expect(await session.getRedboxDescription()).toMatchSnapshot() - - await session.patch( - 'index.js', - outdent` - import Link from 'next/link' - - export default function Hello() { - return ( - - Abc - - ) - } - ` - ) - await session.assertHasRedbox() - expect(await session.getRedboxDescription()).toMatchSnapshot() - }) - - test('server-side only compilation errors', async () => { - await using sandbox = await createSandbox(next) - const { session } = sandbox - - await session.patch( - 'pages/index.js', - outdent` - import myLibrary from 'my-non-existent-library' - export async function getStaticProps() { - return { - props: { - result: myLibrary() - } - } - } - export default function Hello(props) { - return

{props.result}

- } - ` - ) - - await session.assertHasRedbox() - }) -}) diff --git a/test/development/acceptance/__snapshots__/ReactRefreshLogBoxMisc.test.ts.snap b/test/development/acceptance/__snapshots__/ReactRefreshLogBoxMisc.test.ts.snap deleted file mode 100644 index 59e269694d462d..00000000000000 --- a/test/development/acceptance/__snapshots__/ReactRefreshLogBoxMisc.test.ts.snap +++ /dev/null @@ -1,11 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`ReactRefreshLogBox component props errors 2`] = ` -"Error: Failed prop type: The prop \`scroll\` expects a \`boolean\` in \`\`, but got \`string\` instead. -Open your browser's console to view the Component stack trace." -`; - -exports[`ReactRefreshLogBox component props errors 3`] = ` -"Error: Failed prop type: The prop \`href\` expects a \`string\` or \`object\` in \`\`, but got \`boolean\` instead. -Open your browser's console to view the Component stack trace." -`; diff --git a/test/development/pages-dir/client-navigation/index.test.ts b/test/development/pages-dir/client-navigation/index.test.ts index e4e04ecf903826..aa2b28a13d011f 100644 --- a/test/development/pages-dir/client-navigation/index.test.ts +++ b/test/development/pages-dir/client-navigation/index.test.ts @@ -56,27 +56,6 @@ describe('Client Navigation', () => { await browser.close() }) - it('should have a redbox when no children are provided', async () => { - const pageErrors: unknown[] = [] - const browser = await webdriver(next.appPort, '/link-no-child', { - beforePageLoad: (page) => { - page.on('pageerror', (error: unknown) => { - pageErrors.push(error) - }) - }, - }) - await assertHasRedbox(browser) - expect(await getRedboxHeader(browser)).toContain( - 'No children were passed to with `href` of `/about` but one child is required' - ) - expect(pageErrors).toEqual([ - expect.objectContaining({ - message: - 'No children were passed to with `href` of `/about` but one child is required https://nextjs.org/docs/messages/link-no-children', - }), - ]) - }) - it('should not throw error when one number type child is provided', async () => { const browser = await webdriver(next.appPort, '/link-number-child') await assertNoRedbox(browser) diff --git a/test/e2e/app-dir/server-source-maps/server-source-maps.test.ts b/test/e2e/app-dir/server-source-maps/server-source-maps.test.ts index 8e3b308fc2fdc9..7487c2dc20a49a 100644 --- a/test/e2e/app-dir/server-source-maps/server-source-maps.test.ts +++ b/test/e2e/app-dir/server-source-maps/server-source-maps.test.ts @@ -216,6 +216,7 @@ describe('app-dir - server source maps', () => { ) expect(cliOutput).toMatch(/digest: '\d+'/) + // TODO(veil): Should be a single error await expect(browser).toDisplayRedbox(` { "count": 2, diff --git a/test/e2e/next-link-errors/app/invalid-href/page.js b/test/e2e/next-link-errors/app/invalid-href/page.js new file mode 100644 index 00000000000000..62903c97200e07 --- /dev/null +++ b/test/e2e/next-link-errors/app/invalid-href/page.js @@ -0,0 +1,5 @@ +import Link from 'next/link' + +export default function Hello() { + return Hello, Dave! +} diff --git a/test/e2e/next-link-errors/app/layout.js b/test/e2e/next-link-errors/app/layout.js new file mode 100644 index 00000000000000..803f17d863c8ad --- /dev/null +++ b/test/e2e/next-link-errors/app/layout.js @@ -0,0 +1,7 @@ +export default function RootLayout({ children }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/next-link-errors/app/multiple-children/page.js b/test/e2e/next-link-errors/app/multiple-children/page.js new file mode 100644 index 00000000000000..f99ee3aa26515f --- /dev/null +++ b/test/e2e/next-link-errors/app/multiple-children/page.js @@ -0,0 +1,10 @@ +import Link from 'next/link' + +export default function Index() { + return ( + +

Hello

+

Dave!

+ + ) +} diff --git a/test/e2e/next-link-errors/app/no-children/page.js b/test/e2e/next-link-errors/app/no-children/page.js new file mode 100644 index 00000000000000..55b7cbe2d2d9d7 --- /dev/null +++ b/test/e2e/next-link-errors/app/no-children/page.js @@ -0,0 +1,6 @@ +import React from 'react' +import Link from 'next/link' + +export default function Page() { + return +} diff --git a/test/e2e/next-link-errors/next-link-errors.test.ts b/test/e2e/next-link-errors/next-link-errors.test.ts new file mode 100644 index 00000000000000..af90f1c56fa5d4 --- /dev/null +++ b/test/e2e/next-link-errors/next-link-errors.test.ts @@ -0,0 +1,91 @@ +import { nextTestSetup } from 'e2e-utils' +import webdriver from 'next-webdriver' + +describe('next-link', () => { + const { skipped, next, isNextDev } = nextTestSetup({ + files: __dirname, + }) + + if (skipped) return + + it('errors on invalid href', async () => { + const browser = await webdriver(next.appPort, '/invalid-href') + + if (isNextDev) { + // TODO(veil): Should be a single error + // TODO(veil): https://linear.app/vercel/issue/NDX-554/hide-the-anonymous-frames-which-are-between-2-ignored-frames + await expect(browser).toDisplayRedbox(` + { + "count": 2, + "description": "Error: Failed prop type: The prop \`href\` expects a \`string\` or \`object\` in \`\`, but got \`undefined\` instead. + Open your browser's console to view the Component stack trace.", + "environmentLabel": null, + "label": "Unhandled Runtime Error", + "source": "app/invalid-href/page.js (4:10) @ Hello + > 4 | return Hello, Dave! + | ^", + "stack": [ + "Array.forEach (0:0)", + "Hello app/invalid-href/page.js (4:10)", + ], + } + `) + } + expect(await browser.elementByCss('body').text()).toMatchInlineSnapshot( + `"Application error: a client-side exception has occurred while loading localhost (see the browser console for more information)."` + ) + }) + + it('no children', async () => { + const browser = await webdriver(next.appPort, '/no-children') + + if (isNextDev) { + // TODO(veil): Should be a single error + // TODO(veil): https://linear.app/vercel/issue/NDX-554/hide-the-anonymous-frames-which-are-between-2-ignored-frames + await expect(browser).toDisplayRedbox(` + { + "count": 2, + "description": "Error: No children were passed to with \`href\` of \`/about\` but one child is required https://nextjs.org/docs/messages/link-no-children", + "environmentLabel": null, + "label": "Unhandled Runtime Error", + "source": "app/no-children/page.js (5:10) @ Page + > 5 | return + | ^", + "stack": [ + "Page app/no-children/page.js (5:10)", + ], + } + `) + } + expect(await browser.elementByCss('body').text()).toMatchInlineSnapshot( + `"Application error: a client-side exception has occurred while loading localhost (see the browser console for more information)."` + ) + }) + + it('multiple children', async () => { + const browser = await webdriver(next.appPort, '/multiple-children') + + if (isNextDev) { + // TODO(veil): Should be a single error + // TODO(veil): https://linear.app/vercel/issue/NDX-554/hide-the-anonymous-frames-which-are-between-2-ignored-frames + await expect(browser).toDisplayRedbox(` + { + "count": 2, + "description": "Error: Multiple children were passed to with \`href\` of \`/\` but only one child is supported https://nextjs.org/docs/messages/link-multiple-children + Open your browser's console to view the Component stack trace.", + "environmentLabel": null, + "label": "Unhandled Runtime Error", + "source": "app/multiple-children/page.js (5:5) @ Index + > 5 | + | ^", + "stack": [ + "Index app/multiple-children/page.js (5:5)", + ], + } + `) + } + expect(await browser.elementByCss('body').text()).toMatchInlineSnapshot( + `"Application error: a client-side exception has occurred while loading localhost (see the browser console for more information)."` + ) + }) +}) diff --git a/test/e2e/next-link-errors/next.config.js b/test/e2e/next-link-errors/next.config.js new file mode 100644 index 00000000000000..807126e4cf0bf5 --- /dev/null +++ b/test/e2e/next-link-errors/next.config.js @@ -0,0 +1,6 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = {} + +module.exports = nextConfig diff --git a/test/turbopack-dev-tests-manifest.json b/test/turbopack-dev-tests-manifest.json index 1804824aa40269..e343bb7c74c5aa 100644 --- a/test/turbopack-dev-tests-manifest.json +++ b/test/turbopack-dev-tests-manifest.json @@ -1612,17 +1612,6 @@ "flakey": [], "runtimeError": false }, - "test/development/acceptance-app/ReactRefreshLogBoxMisc.test.ts": { - "passed": [], - "failed": [], - "pending": [ - "ReactRefreshLogBox app component props errors", - "ReactRefreshLogBox app with multiple children", - "ReactRefreshLogBox app server-side only compilation errors" - ], - "flakey": [], - "runtimeError": false - }, "test/development/acceptance-app/ReactRefreshModule.test.ts": { "passed": ["ReactRefreshModule app should allow any variable names"], "failed": [],