Minimized runtime errors in app dir (#43511)

Currently when there's a runtime error - the error overlay opens up in full screen mode. In app it should open in the minimized state. Build errors will still have to open up in full screen since the app will not be runnable due to it being broken. Before ![image](https://user-images.githubusercontent.com/25056922/204551137-5a72c4b9-5340-49d7-8369-9087a7830732.png) After ![image](https://user-images.githubusercontent.com/25056922/204550919-503a0dc5-1c36-4965-818e-c97e455f73bf.png) Fixes #43460 ## 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)
vercel · Dec 1, 2022 · b2dd167 · b2dd167
1 parent 1c89da6
commit b2dd167
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 30 deletions.
diff --git a/packages/next/client/components/react-dev-overlay/hot-reloader-client.tsx b/packages/next/client/components/react-dev-overlay/hot-reloader-client.tsx
@@ -106,6 +106,7 @@ function performFullReload(err: any, sendMessage: any) {
     JSON.stringify({
       event: 'client-full-reload',
       stackTrace,
+      hadRuntimeError: !!RuntimeErrorHandler.hadRuntimeError,
     })
   )
 

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
@@ -139,7 +139,7 @@ export const Errors: React.FC<ErrorsProps> = function Errors({ errors }) {
 
   const [displayState, setDisplayState] = React.useState<
     'minimized' | 'fullscreen' | 'hidden'
-  >('fullscreen')
+  >('minimized')
   const [activeIdx, setActiveIndex] = React.useState<number>(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
@@ -49,7 +49,7 @@ describe('ReactRefreshLogBox app', () => {
     )
     await session.evaluate(() => document.querySelector('a').click())
 
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
     expect(await session.getRedboxSource()).toMatchSnapshot()
 
     await cleanup()
@@ -228,7 +228,7 @@ describe('ReactRefreshLogBox app', () => {
       `
     )
 
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
     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;
       `
     )
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
     expect(await session.getRedboxSource()).toMatchSnapshot()
 
     await cleanup()
@@ -370,7 +370,7 @@ describe('ReactRefreshLogBox app', () => {
       `
     )
 
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
     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.
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
     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))
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
     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'); }`
     )
 
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
     expect(await session.getRedboxSource()).toMatchSnapshot()
     expect(
       await session.evaluate(() => document.querySelector('h2').textContent)
@@ -664,7 +664,7 @@ describe('ReactRefreshLogBox app', () => {
       `
     )
 
-    expect(await session.hasRedbox()).toBe(false)
+    expect(await session.hasErrorToast()).toBe(false)
     expect(
       await session.evaluate(() => document.querySelector('p').textContent)
     ).toBe('hello')
@@ -681,7 +681,7 @@ describe('ReactRefreshLogBox app', () => {
       `
     )
 
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
     expect(await session.getRedboxSource()).toMatchSnapshot()
 
     await session.patch(
@@ -764,9 +764,9 @@ describe('ReactRefreshLogBox app', () => {
       `
     )
 
-    expect(await session.hasRedbox()).toBe(false)
+    expect(await session.hasErrorToast()).toBe(false)
     await session.evaluate(() => document.querySelector('button').click())
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
 
     const header = await session.getRedboxDescription()
     expect(header).toMatchSnapshot()
@@ -810,9 +810,9 @@ describe('ReactRefreshLogBox app', () => {
       `
     )
 
-    expect(await session.hasRedbox()).toBe(false)
+    expect(await session.hasErrorToast()).toBe(false)
     await session.evaluate(() => document.querySelector('button').click())
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
 
     const header2 = await session.getRedboxDescription()
     expect(header2).toMatchSnapshot()
@@ -856,9 +856,9 @@ describe('ReactRefreshLogBox app', () => {
       `
     )
 
-    expect(await session.hasRedbox()).toBe(false)
+    expect(await session.hasErrorToast()).toBe(false)
     await session.evaluate(() => document.querySelector('button').click())
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
 
     const header3 = await session.getRedboxDescription()
     expect(header3).toMatchSnapshot()
@@ -902,9 +902,9 @@ describe('ReactRefreshLogBox app', () => {
       `
     )
 
-    expect(await session.hasRedbox()).toBe(false)
+    expect(await session.hasErrorToast()).toBe(false)
     await session.evaluate(() => document.querySelector('button').click())
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
 
     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
@@ -280,7 +280,7 @@ describe('ReactRefreshRegression app', () => {
 
     await browser.refresh()
 
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
 
     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()
 
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
 
     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()
 
-    expect(await session.hasRedbox(true)).toBe(true)
+    await session.waitForAndOpenRuntimeError()
 
     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
@@ -2,7 +2,9 @@ import {
   getRedboxDescription,
   getRedboxHeader,
   getRedboxSource,
+  hasErrorToast,
   hasRedbox,
+  waitForAndOpenRuntimeError,
 } from 'next-test-utils'
 import webdriver from 'next-webdriver'
 import { NextInstance } from 'test/lib/next-modes/base'
@@ -100,6 +102,12 @@ 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
@@ -1,6 +1,9 @@
 import { createNext, FileRef } from 'e2e-utils'
 import { NextInstance } from 'test/lib/next-modes/base'
-import { getRedboxDescription, hasRedbox } from 'next-test-utils'
+import {
+  getRedboxDescription,
+  waitForAndOpenRuntimeError,
+} from 'next-test-utils'
 import path from 'path'
 import webdriver from 'next-webdriver'
 
@@ -29,7 +32,7 @@ describe('dynamic-href', () => {
       const browser = await webdriver(next.url, '/object')
 
       // Error should show up
-      expect(await hasRedbox(browser, true)).toBeTrue()
+      await waitForAndOpenRuntimeError(browser)
       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"`
       )
@@ -57,7 +60,7 @@ describe('dynamic-href', () => {
       const browser = await webdriver(next.url, '/string')
 
       // Error should show up
-      expect(await hasRedbox(browser, true)).toBeTrue()
+      await waitForAndOpenRuntimeError(browser)
       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"`
       )

diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts
@@ -7,11 +7,11 @@ import {
   fetchViaHTTP,
   findPort,
   getRedboxHeader,
-  hasRedbox,
   initNextServerScript,
   killApp,
   renderViaHTTP,
   waitFor,
+  waitForAndOpenRuntimeError,
 } from 'next-test-utils'
 import path from 'path'
 import cheerio from 'cheerio'
@@ -2060,7 +2060,7 @@ describe('app dir', () => {
         await browser.elementByCss('#error-trigger-button').click()
 
         if (isDev) {
-          expect(await hasRedbox(browser)).toBe(true)
+          await waitForAndOpenRuntimeError(browser)
           expect(await getRedboxHeader(browser)).toMatch(/this is a test/)
         } else {
           await browser
@@ -2110,12 +2110,10 @@ describe('app dir', () => {
           '/error/global-error-boundary'
         )
         await browser.elementByCss('#error-trigger-button').click()
-        // .waitForElementByCss('body')
 
         if (isDev) {
-          expect(await hasRedbox(browser)).toBe(true)
-          console.log('getRedboxHeader', await getRedboxHeader(browser))
-          // expect(await getRedboxHeader(browser)).toMatch(/An error occurred: this is a test/)
+          await waitForAndOpenRuntimeError(browser)
+          expect(await getRedboxHeader(browser)).toMatch(/this is a test/)
         } else {
           expect(
             await browser

diff --git a/test/lib/next-test-utils.js b/test/lib/next-test-utils.js
@@ -643,6 +643,28 @@ 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(
     () =>