Skip to content

Commit

Permalink
Revert: 'Minimized runtime errors in app dir' (#43648)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
hanneslund committed Dec 2, 2022
1 parent c6ec490 commit d41afd7
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 60 deletions.
Expand Up @@ -139,7 +139,7 @@ export const Errors: React.FC<ErrorsProps> = function Errors({ errors }) {

const [displayState, setDisplayState] = React.useState<
'minimized' | 'fullscreen' | 'hidden'
>('minimized')
>('fullscreen')
const [activeIdx, setActiveIndex] = React.useState<number>(0)
const previous = React.useCallback((e?: MouseEvent | TouchEvent) => {
e?.preventDefault()
Expand Down
34 changes: 17 additions & 17 deletions test/development/acceptance-app/ReactRefreshLogBox.test.ts
Expand Up @@ -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()
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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')
Expand All @@ -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(
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand Down
Expand Up @@ -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(`
Expand All @@ -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(`
Expand All @@ -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(`
Expand Down
8 changes: 0 additions & 8 deletions test/development/acceptance-app/helpers.ts
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
},
Expand Down
9 changes: 3 additions & 6 deletions 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'

Expand Down Expand Up @@ -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 <Link> while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"`
)
Expand Down Expand Up @@ -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 <Link> while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"`
)
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -5,9 +5,9 @@ import {
check,
fetchViaHTTP,
getRedboxHeader,
hasRedbox,
renderViaHTTP,
waitFor,
waitForAndOpenRuntimeError,
} from 'next-test-utils'
import path from 'path'
import cheerio from 'cheerio'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
22 changes: 0 additions & 22 deletions test/lib/next-test-utils.js
Expand Up @@ -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(
() =>
Expand Down

0 comments on commit d41afd7

Please sign in to comment.