Skip to content

Commit

Permalink
Fast refresh should recover from event handler errors in app dir (#43882
Browse files Browse the repository at this point in the history
)

Component state should not be lost due to a full reload after an error occurs in an event handler. Only do a full reload if an error was caught by the error overlay error boundary.

Closes NEXT-182

## 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 13, 2022
1 parent a34b20e commit 04c2509
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 8 deletions.
Expand Up @@ -435,6 +435,9 @@ export default function HotReload({
frames: parseStack(reason.stack!),
})
}, [])
const handleOnReactError = useCallback(() => {
RuntimeErrorHandler.hadRuntimeError = true
}, [])
useErrorHandler(handleOnUnhandledError, handleOnUnhandledRejection)

const webSocketRef = useWebsocket(assetPrefix)
Expand Down Expand Up @@ -467,5 +470,9 @@ export default function HotReload({
return () => websocket && websocket.removeEventListener('message', handler)
}, [sendMessage, router, webSocketRef, dispatcher])

return <ReactDevOverlay state={state}>{children}</ReactDevOverlay>
return (
<ReactDevOverlay onReactError={handleOnReactError} state={state}>
{children}
</ReactDevOverlay>
)
}
Expand Up @@ -21,6 +21,7 @@ class ReactDevOverlay extends React.PureComponent<
{
state: OverlayState
children: React.ReactNode
onReactError: (error: Error) => void
},
ReactDevOverlayState
> {
Expand All @@ -40,6 +41,10 @@ class ReactDevOverlay extends React.PureComponent<
return { reactError: errorEvent }
}

componentDidCatch(componentErr: Error) {
this.props.onReactError(componentErr)
}

render() {
const { state, children } = this.props
const { reactError } = this.state
Expand Down
Expand Up @@ -44,8 +44,6 @@ if (typeof window !== 'undefined') {
return
}

RuntimeErrorHandler.hadRuntimeError = true

const error = ev?.error
if (
!error ||
Expand All @@ -69,8 +67,6 @@ if (typeof window !== 'undefined') {
window.addEventListener(
'unhandledrejection',
(ev: WindowEventMap['unhandledrejection']): void => {
RuntimeErrorHandler.hadRuntimeError = true

const reason = ev?.reason
if (
!reason ||
Expand Down
7 changes: 4 additions & 3 deletions test/development/acceptance-app/ReactRefreshLogBox.test.ts
Expand Up @@ -114,8 +114,7 @@ describe('ReactRefreshLogBox app', () => {
await cleanup()
})

// TODO-APP: re-enable when error recovery doesn't reload the page.
test.skip('logbox: can recover from a event handler error', async () => {
test('logbox: can recover from a event handler error', async () => {
const { session, cleanup } = await sandbox(next)

await session.patch(
Expand Down Expand Up @@ -147,7 +146,7 @@ describe('ReactRefreshLogBox app', () => {
await session.evaluate(() => document.querySelector('p').textContent)
).toBe('1')

expect(await session.hasRedbox(true)).toBe(true)
await session.waitForAndOpenRuntimeError()
if (process.platform === 'win32') {
expect(await session.getRedboxSource()).toMatchSnapshot()
} else {
Expand All @@ -173,6 +172,7 @@ describe('ReactRefreshLogBox app', () => {
)

expect(await session.hasRedbox()).toBe(false)
expect(await session.hasErrorToast()).toBe(false)

expect(
await session.evaluate(() => document.querySelector('p').textContent)
Expand All @@ -183,6 +183,7 @@ describe('ReactRefreshLogBox app', () => {
).toBe('Count: 2')

expect(await session.hasRedbox()).toBe(false)
expect(await session.hasErrorToast()).toBe(false)

await cleanup()
})
Expand Down
Expand Up @@ -66,6 +66,18 @@ exports[`ReactRefreshLogBox app logbox: can recover from a component error 1`] =
6 | "
`;
exports[`ReactRefreshLogBox app logbox: can recover from a event handler error 1`] = `
"index.js (8:18) @ eval
6 | const increment = useCallback(() => {
7 | setCount(c => c + 1)
> 8 | throw new Error('oops')
| ^
9 | }, [setCount])
10 | return (
11 | <main>"
`;
exports[`ReactRefreshLogBox app logbox: can recover from a syntax error without losing state 1`] = `
"./index.js
Error:
Expand Down
9 changes: 9 additions & 0 deletions test/development/acceptance-app/helpers.ts
Expand Up @@ -100,6 +100,15 @@ export async function sandbox(
async hasRedbox(expected = false) {
return hasRedbox(browser, expected)
},
async hasErrorToast() {
return browser.eval(() => {
return Boolean(
Array.from(document.querySelectorAll('nextjs-portal')).find((p) =>
p.shadowRoot.querySelector('[data-nextjs-toast]')
)
)
})
},
async getRedboxDescription() {
return getRedboxDescription(browser)
},
Expand Down

0 comments on commit 04c2509

Please sign in to comment.