Fix race condition in ReactDevOverlay

packages/next/client/dev/error-overlay/hot-dev-client.js

@@ -30,6 +30,7 @@ import {
   register,
   onBuildError,
   onBuildOk,
+  onBeforeRefresh,
   onRefresh,
 } from 'next/dist/compiled/@next/react-dev-overlay/dist/client'
 import stripAnsi from 'next/dist/compiled/strip-ansi'
@@ -98,11 +99,7 @@ function handleSuccess() {
 
   // Attempt to apply hot updates or reload.
   if (isHotUpdate) {
-    tryApplyUpdates(function onSuccessfulHotUpdate(hasUpdates) {
-      // Only dismiss it when we're sure it's a hot update.
-      // Otherwise it would flicker right before the reload.
-      onFastRefresh(hasUpdates)
-    })
+    tryApplyUpdates(onBeforeFastRefresh, onFastRefresh)
   }
 }
 
@@ -139,11 +136,7 @@ function handleWarnings(warnings) {
 
   // Attempt to apply hot updates or reload.
   if (isHotUpdate) {
-    tryApplyUpdates(function onSuccessfulHotUpdate(hasUpdates) {
-      // Only dismiss it when we're sure it's a hot update.
-      // Otherwise it would flicker right before the reload.
-      onFastRefresh(hasUpdates)
-    })
+    tryApplyUpdates(onBeforeFastRefresh, onFastRefresh)
   }
 }
 
@@ -182,9 +175,19 @@ function handleErrors(errors) {
 
 let startLatency = undefined
 
+function onBeforeFastRefresh(hasUpdates) {
+  if (hasUpdates) {
+    // Only trigger a pending state if we have updates to apply
+    // (cf. onFastRefresh)
+    onBeforeRefresh()
+  }
+}
+
 function onFastRefresh(hasUpdates) {
   onBuildOk()
   if (hasUpdates) {
+    // Only complete a pending state if we applied updates
+    // (cf. onBeforeFastRefresh)
     onRefresh()
   }
 
@@ -301,7 +304,7 @@ function afterApplyUpdates(fn) {
 }
 
 // Attempt to update code on the fly, fall back to a hard reload.
-function tryApplyUpdates(onHotUpdateSuccess) {
+function tryApplyUpdates(onBeforeHotUpdate, onHotUpdateSuccess) {
   if (!module.hot) {
     // HotModuleReplacementPlugin is not in Webpack configuration.
     console.error('HotModuleReplacementPlugin is not in Webpack configuration.')
@@ -342,7 +345,10 @@ function tryApplyUpdates(onHotUpdateSuccess) {
 
     if (isUpdateAvailable()) {
       // While we were updating, there was a new update! Do it again.
-      tryApplyUpdates(hasUpdates ? onBuildOk : onHotUpdateSuccess)
+      tryApplyUpdates(
+        onBeforeHotUpdate,
+        hasUpdates ? onBuildOk : onHotUpdateSuccess
+      )
     } else {
       onBuildOk()
       if (process.env.__NEXT_TEST_MODE) {
@@ -357,14 +363,21 @@ function tryApplyUpdates(onHotUpdateSuccess) {
   }
 
   // https://webpack.js.org/api/hot-module-replacement/#check
-  module.hot.check(/* autoApply */ true).then(
-    (updatedModules) => {
-      handleApplyUpdates(null, updatedModules)
-    },
-    (err) => {
-      handleApplyUpdates(err, null)
-    }
-  )
+  module.hot
+    .check(/* autoApply */ false)
+    .then((updatedModules) => {
+      const hasUpdates = Boolean(updatedModules.length)
+      onBeforeHotUpdate(hasUpdates)
+      return module.hot.apply()
+    })
+    .then(
+      (updatedModules) => {
+        handleApplyUpdates(null, updatedModules)
+      },
+      (err) => {
+        handleApplyUpdates(err, null)
+      }
+    )
 }
 
 function performFullReload(err) {

packages/next/client/index.tsx

@@ -304,15 +304,16 @@ function renderApp(App: AppComponent, appProps: AppProps) {
 function AppContainer({
   children,
 }: React.PropsWithChildren<{}>): React.ReactElement {
+  console.log('hello world 1')
   return (
     <Container
-      fn={(error) =>
+      fn={(error) => {
         // TODO: Fix disabled eslint rule
         // eslint-disable-next-line @typescript-eslint/no-use-before-define
         renderError({ App: CachedApp, err: error }).catch((err) =>
           console.error('Error rendering page: ', err)
         )
-      }
+      }}
     >
       <AppRouterContext.Provider value={adaptForAppRouterInstance(router)}>
         <SearchParamsContext.Provider value={adaptForSearchParams(router)}>

packages/react-dev-overlay/src/client.ts

@@ -89,7 +89,18 @@ function onRefresh() {
   Bus.emit({ type: Bus.TYPE_REFRESH })
 }
 
+function onBeforeRefresh() {
+  Bus.emit({ type: Bus.TYPE_BEFORE_REFRESH })
+}
+
 export { getErrorByType } from './internal/helpers/getErrorByType'
 export { getServerError } from './internal/helpers/nodeStackFrames'
 export { default as ReactDevOverlay } from './internal/ReactDevOverlay'
-export { onBuildOk, onBuildError, register, unregister, onRefresh }
+export {
+  onBuildOk,
+  onBuildError,
+  register,
+  unregister,
+  onBeforeRefresh,
+  onRefresh,
+}

packages/react-dev-overlay/src/internal/ReactDevOverlay.tsx

@@ -9,10 +9,23 @@ import { Base } from './styles/Base'
 import { ComponentStyles } from './styles/ComponentStyles'
 import { CssReset } from './styles/CssReset'
 
+type RefreshState =
+  | {
+      // No refresh in progress.
+      type: 'idle'
+    }
+  | {
+      // The refresh process has been triggered, but the new code has not been
+      // executed yet.
+      type: 'pending'
+      errors: SupportedErrorEvent[]
+    }
+
 type OverlayState = {
   nextId: number
   buildError: string | null
   errors: SupportedErrorEvent[]
+  refreshState: RefreshState
 }
 
 function reducer(state: OverlayState, ev: Bus.BusEvent): OverlayState {
@@ -23,21 +36,52 @@ function reducer(state: OverlayState, ev: Bus.BusEvent): OverlayState {
     case Bus.TYPE_BUILD_ERROR: {
       return { ...state, buildError: ev.message }
     }
+    case Bus.TYPE_BEFORE_REFRESH: {
+      return { ...state, refreshState: { type: 'pending', errors: [] } }
+    }
     case Bus.TYPE_REFRESH: {
-      return { ...state, buildError: null, errors: [] }
+      return {
+        ...state,
+        buildError: null,
+        errors:
+          // Errors can come in during updates. In this case, UNHANDLED_ERROR
+          // and UNHANDLED_REJECTION events might be dispatched between the
+          // BEFORE_REFRESH and the REFRESH event. We want to keep those errors
+          // around until the next refresh. Otherwise we run into a race
+          // condition where those errors would be cleared on refresh completion
+          // before they can be displayed.
+          state.refreshState.type === 'pending'
+            ? state.refreshState.errors
+            : [],
+        refreshState: { type: 'idle' },
+      }
     }
     case Bus.TYPE_UNHANDLED_ERROR:
     case Bus.TYPE_UNHANDLED_REJECTION: {
-      return {
-        ...state,
-        nextId: state.nextId + 1,
-        errors: [
-          ...state.errors.filter((err) => {
-            // Filter out duplicate errors
-            return err.event.reason !== ev.reason
-          }),
-          { id: state.nextId, event: ev },
-        ],
+      switch (state.refreshState.type) {
+        case 'idle': {
+          return {
+            ...state,
+            nextId: state.nextId + 1,
+            errors: pushErrorFilterDuplicates(state.errors, {
+              id: state.nextId,
+              event: ev,
+            }),
+          }
+        }
+        case 'pending': {
+          return {
+            ...state,
+            nextId: state.nextId + 1,
+            refreshState: {
+              ...state.refreshState,
+              errors: pushErrorFilterDuplicates(state.refreshState.errors, {
+                id: state.nextId,
+                event: ev,
+              }),
+            },
+          }
+        }
       }
     }
     default: {
@@ -48,6 +92,19 @@ function reducer(state: OverlayState, ev: Bus.BusEvent): OverlayState {
   }
 }
 
+function pushErrorFilterDuplicates(
+  errors: SupportedErrorEvent[],
+  err: SupportedErrorEvent
+): SupportedErrorEvent[] {
+  return [
+    ...errors.filter((e) => {
+      // Filter out duplicate errors
+      return e.event.reason !== err.event.reason
+    }),
+    err,
+  ]
+}
+
 type ErrorType = 'runtime' | 'build'
 
 const shouldPreventDisplay = (
@@ -74,6 +131,9 @@ const ReactDevOverlay: React.FunctionComponent<ReactDevOverlayProps> =
       nextId: 1,
       buildError: null,
       errors: [],
+      refreshState: {
+        type: 'idle',
+      },
     })
 
     React.useEffect(() => {

packages/react-dev-overlay/src/internal/bus.ts

@@ -3,6 +3,7 @@ import { StackFrame } from 'stacktrace-parser'
 export const TYPE_BUILD_OK = 'build-ok'
 export const TYPE_BUILD_ERROR = 'build-error'
 export const TYPE_REFRESH = 'fast-refresh'
+export const TYPE_BEFORE_REFRESH = 'before-fast-refresh'
 export const TYPE_UNHANDLED_ERROR = 'unhandled-error'
 export const TYPE_UNHANDLED_REJECTION = 'unhandled-rejection'
 
@@ -11,6 +12,7 @@ export type BuildError = {
   type: typeof TYPE_BUILD_ERROR
   message: string
 }
+export type BeforeFastRefresh = { type: typeof TYPE_BEFORE_REFRESH }
 export type FastRefresh = { type: typeof TYPE_REFRESH }
 export type UnhandledError = {
   type: typeof TYPE_UNHANDLED_ERROR
@@ -26,6 +28,7 @@ export type BusEvent =
   | BuildOk
   | BuildError
   | FastRefresh
+  | BeforeFastRefresh
   | UnhandledError
   | UnhandledRejection