From d41afd71525c85002ffeefaedce3bd7343c4739e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Fri, 2 Dec 2022 20:51:23 +0100 Subject: [PATCH] Revert: 'Minimized runtime errors in app dir' (#43648) Reverts: #43511 It incorrectly shows render errors minimized although it should only be for uncaught errors/rejections. Keep old behaviour for now. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm build && pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) --- .../internal/container/Errors.tsx | 2 +- .../acceptance-app/ReactRefreshLogBox.test.ts | 34 +++++++++---------- .../ReactRefreshRegression.test.ts | 6 ++-- test/development/acceptance-app/helpers.ts | 8 ----- test/e2e/app-dir/dynamic-href.test.ts | 9 ++--- test/e2e/app-dir/index.test.ts | 6 ++-- test/lib/next-test-utils.js | 22 ------------ 7 files changed, 27 insertions(+), 60 deletions(-) diff --git a/packages/next/client/components/react-dev-overlay/internal/container/Errors.tsx b/packages/next/client/components/react-dev-overlay/internal/container/Errors.tsx index bb27bc94aedbd70..84d83cd46f58c65 100644 --- a/packages/next/client/components/react-dev-overlay/internal/container/Errors.tsx +++ b/packages/next/client/components/react-dev-overlay/internal/container/Errors.tsx @@ -139,7 +139,7 @@ export const Errors: React.FC = function Errors({ errors }) { const [displayState, setDisplayState] = React.useState< 'minimized' | 'fullscreen' | 'hidden' - >('minimized') + >('fullscreen') const [activeIdx, setActiveIndex] = React.useState(0) const previous = React.useCallback((e?: MouseEvent | TouchEvent) => { e?.preventDefault() diff --git a/test/development/acceptance-app/ReactRefreshLogBox.test.ts b/test/development/acceptance-app/ReactRefreshLogBox.test.ts index b12561077e0efa4..b7ad6c224298c71 100644 --- a/test/development/acceptance-app/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance-app/ReactRefreshLogBox.test.ts @@ -49,7 +49,7 @@ describe('ReactRefreshLogBox app', () => { ) await session.evaluate(() => document.querySelector('a').click()) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) expect(await session.getRedboxSource()).toMatchSnapshot() await cleanup() @@ -228,7 +228,7 @@ describe('ReactRefreshLogBox app', () => { ` ) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) expect(await session.getRedboxSource()).toMatchSnapshot() // TODO-APP: re-enable when error recovery doesn't reload the page. @@ -325,7 +325,7 @@ describe('ReactRefreshLogBox app', () => { export default ClassDefault; ` ) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) expect(await session.getRedboxSource()).toMatchSnapshot() await cleanup() @@ -370,7 +370,7 @@ describe('ReactRefreshLogBox app', () => { ` ) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) if (process.platform === 'win32') { expect(await session.getRedboxSource()).toMatchSnapshot() } else { @@ -423,7 +423,7 @@ describe('ReactRefreshLogBox app', () => { ) // We get an error because Foo didn't import React. Fair. - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) expect(await session.getRedboxSource()).toMatchSnapshot() // Let's add that to Foo. @@ -475,7 +475,7 @@ describe('ReactRefreshLogBox app', () => { ) await new Promise((resolve) => setTimeout(resolve, 1000)) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) if (process.platform === 'win32') { expect(await session.getRedboxSource()).toMatchSnapshot() } else { @@ -562,7 +562,7 @@ describe('ReactRefreshLogBox app', () => { `export default function FunctionDefault() { throw new Error('no'); }` ) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) expect(await session.getRedboxSource()).toMatchSnapshot() expect( await session.evaluate(() => document.querySelector('h2').textContent) @@ -664,7 +664,7 @@ describe('ReactRefreshLogBox app', () => { ` ) - expect(await session.hasErrorToast()).toBe(false) + expect(await session.hasRedbox()).toBe(false) expect( await session.evaluate(() => document.querySelector('p').textContent) ).toBe('hello') @@ -681,7 +681,7 @@ describe('ReactRefreshLogBox app', () => { ` ) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) expect(await session.getRedboxSource()).toMatchSnapshot() await session.patch( @@ -764,9 +764,9 @@ describe('ReactRefreshLogBox app', () => { ` ) - expect(await session.hasErrorToast()).toBe(false) + expect(await session.hasRedbox()).toBe(false) await session.evaluate(() => document.querySelector('button').click()) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) const header = await session.getRedboxDescription() expect(header).toMatchSnapshot() @@ -810,9 +810,9 @@ describe('ReactRefreshLogBox app', () => { ` ) - expect(await session.hasErrorToast()).toBe(false) + expect(await session.hasRedbox()).toBe(false) await session.evaluate(() => document.querySelector('button').click()) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) const header2 = await session.getRedboxDescription() expect(header2).toMatchSnapshot() @@ -856,9 +856,9 @@ describe('ReactRefreshLogBox app', () => { ` ) - expect(await session.hasErrorToast()).toBe(false) + expect(await session.hasRedbox()).toBe(false) await session.evaluate(() => document.querySelector('button').click()) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) const header3 = await session.getRedboxDescription() expect(header3).toMatchSnapshot() @@ -902,9 +902,9 @@ describe('ReactRefreshLogBox app', () => { ` ) - expect(await session.hasErrorToast()).toBe(false) + expect(await session.hasRedbox()).toBe(false) await session.evaluate(() => document.querySelector('button').click()) - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) const header4 = await session.getRedboxDescription() expect(header4).toMatchInlineSnapshot( diff --git a/test/development/acceptance-app/ReactRefreshRegression.test.ts b/test/development/acceptance-app/ReactRefreshRegression.test.ts index 9725b9b0580d2d2..88c19bf0cd53b4f 100644 --- a/test/development/acceptance-app/ReactRefreshRegression.test.ts +++ b/test/development/acceptance-app/ReactRefreshRegression.test.ts @@ -280,7 +280,7 @@ describe('ReactRefreshRegression app', () => { await browser.refresh() - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) const source = await session.getRedboxSource() expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(` @@ -301,7 +301,7 @@ describe('ReactRefreshRegression app', () => { await browser.refresh() - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) const source = await session.getRedboxSource() expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(` @@ -323,7 +323,7 @@ describe('ReactRefreshRegression app', () => { await browser.refresh() - await session.waitForAndOpenRuntimeError() + expect(await session.hasRedbox(true)).toBe(true) const source = await session.getRedboxSource() expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(` diff --git a/test/development/acceptance-app/helpers.ts b/test/development/acceptance-app/helpers.ts index 0ad5c8db77e1c54..f0787fb84b76dad 100644 --- a/test/development/acceptance-app/helpers.ts +++ b/test/development/acceptance-app/helpers.ts @@ -2,9 +2,7 @@ import { getRedboxDescription, getRedboxHeader, getRedboxSource, - hasErrorToast, hasRedbox, - waitForAndOpenRuntimeError, } from 'next-test-utils' import webdriver from 'next-webdriver' import { NextInstance } from 'test/lib/next-modes/base' @@ -102,12 +100,6 @@ export async function sandbox( async hasRedbox(expected = false) { return hasRedbox(browser, expected) }, - async hasErrorToast(expected = false) { - return hasErrorToast(browser, expected) - }, - async waitForAndOpenRuntimeError() { - await waitForAndOpenRuntimeError(browser) - }, async getRedboxDescription() { return getRedboxDescription(browser) }, diff --git a/test/e2e/app-dir/dynamic-href.test.ts b/test/e2e/app-dir/dynamic-href.test.ts index 3337e29d7098f13..6615538c55e262d 100644 --- a/test/e2e/app-dir/dynamic-href.test.ts +++ b/test/e2e/app-dir/dynamic-href.test.ts @@ -1,9 +1,6 @@ import { createNext, FileRef } from 'e2e-utils' import { NextInstance } from 'test/lib/next-modes/base' -import { - getRedboxDescription, - waitForAndOpenRuntimeError, -} from 'next-test-utils' +import { getRedboxDescription, hasRedbox } from 'next-test-utils' import path from 'path' import webdriver from 'next-webdriver' @@ -32,7 +29,7 @@ describe('dynamic-href', () => { const browser = await webdriver(next.url, '/object') // Error should show up - await waitForAndOpenRuntimeError(browser) + expect(await hasRedbox(browser, true)).toBeTrue() expect(await getRedboxDescription(browser)).toMatchInlineSnapshot( `"Error: Dynamic href \`/object/[slug]\` found in while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"` ) @@ -60,7 +57,7 @@ describe('dynamic-href', () => { const browser = await webdriver(next.url, '/string') // Error should show up - await waitForAndOpenRuntimeError(browser) + expect(await hasRedbox(browser, true)).toBeTrue() expect(await getRedboxDescription(browser)).toMatchInlineSnapshot( `"Error: Dynamic href \`/object/[slug]\` found in while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"` ) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index a4e0278080c98f6..3bf7b92d542f8a7 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -5,9 +5,9 @@ import { check, fetchViaHTTP, getRedboxHeader, + hasRedbox, renderViaHTTP, waitFor, - waitForAndOpenRuntimeError, } from 'next-test-utils' import path from 'path' import cheerio from 'cheerio' @@ -2055,7 +2055,7 @@ describe('app dir', () => { await browser.elementByCss('#error-trigger-button').click() if (isDev) { - await waitForAndOpenRuntimeError(browser) + expect(await hasRedbox(browser)).toBe(true) expect(await getRedboxHeader(browser)).toMatch(/this is a test/) } else { await browser @@ -2107,7 +2107,7 @@ describe('app dir', () => { await browser.elementByCss('#error-trigger-button').click() if (isDev) { - await waitForAndOpenRuntimeError(browser) + expect(await hasRedbox(browser)).toBe(true) expect(await getRedboxHeader(browser)).toMatch(/this is a test/) } else { expect( diff --git a/test/lib/next-test-utils.js b/test/lib/next-test-utils.js index c98686689a2c279..ec4592e1287c67a 100644 --- a/test/lib/next-test-utils.js +++ b/test/lib/next-test-utils.js @@ -643,28 +643,6 @@ export async function hasRedbox(browser, expected = true) { return false } -export async function hasErrorToast(browser, expected = true) { - for (let i = 0; i < 30; i++) { - const result = await evaluate(browser, () => { - return Boolean( - [].slice - .call(document.querySelectorAll('nextjs-portal')) - .find((p) => p.shadowRoot.querySelector('[data-nextjs-toast]')) - ) - }) - - if (result === expected) { - return result - } - await waitFor(1000) - } - return false -} - -export async function waitForAndOpenRuntimeError(browser) { - return browser.waitForElementByCss('[data-nextjs-toast]').click() -} - export async function getRedboxHeader(browser) { return retry( () =>