refactor dev overlay into hot reloader (vercel#41231)

Move react-dev-overlay into hot reloader client components, and let it imported by app router. So then we don't need to have hot reloader and react tree as adjacent sibilings but the overlay with error boundary is wrapping the react tree Co-authored-by: Tim Neutkens <6324199+timneutkens@users.noreply.github.com>
Kikobeats · Oct 24, 2022 · e95693c · e95693c
1 parent 925b49b
commit e95693c
Show file tree

Hide file tree

Showing 9 changed files with 92 additions and 109 deletions.
diff --git a/packages/next/build/webpack/loaders/next-app-loader.ts b/packages/next/build/webpack/loaders/next-app-loader.ts
@@ -185,12 +185,6 @@ const nextAppLoader: webpack.LoaderDefinitionFunction<{
     export const AppRouter = require('next/dist/client/components/app-router.client.js').default
     export const LayoutRouter = require('next/dist/client/components/layout-router.client.js').default
     export const RenderFromTemplateContext = require('next/dist/client/components/render-from-template-context.client.js').default
-    export const HotReloader = ${
-      // Disable HotReloader component in production
-      this.mode === 'development'
-        ? `require('next/dist/client/components/hot-reloader.client.js').default`
-        : 'null'
-    }
 
     export const staticGenerationAsyncStorage = require('next/dist/client/components/static-generation-async-storage.js').staticGenerationAsyncStorage
     export const requestAsyncStorage = require('next/dist/client/components/request-async-storage.js').requestAsyncStorage

diff --git a/packages/next/client/components/app-router.client.tsx b/packages/next/client/components/app-router.client.tsx
@@ -1,6 +1,6 @@
 'client'
 
-import type { PropsWithChildren, ReactElement, ReactNode } from 'react'
+import type { ReactNode } from 'react'
 import React, { useEffect, useMemo, useCallback } from 'react'
 import { createFromFetch } from 'next/dist/compiled/react-server-dom-webpack'
 import {
@@ -35,6 +35,12 @@ function urlToUrlWithoutFlightMarker(url: string): URL {
   return urlWithoutFlightParameters
 }
 
+const HotReloader: typeof import('./hot-reloader').default | null =
+  process.env.NODE_ENV === 'production'
+    ? null
+    : (require('./hot-reloader')
+        .default as typeof import('./hot-reloader').default)
+
 /**
  * Fetch the flight data for the provided url. Takes in the current router state to decide what to render server-side.
  */
@@ -78,20 +84,6 @@ export async function fetchServerResponse(
   return [flightData, canonicalUrl]
 }
 
-/**
- * Renders development error overlay when NODE_ENV is development.
- */
-function ErrorOverlay({ children }: PropsWithChildren<{}>): ReactElement {
-  if (process.env.NODE_ENV === 'production') {
-    return <>{children}</>
-  } else {
-    const {
-      ReactDevOverlay,
-    } = require('next/dist/compiled/@next/react-dev-overlay/dist/client')
-    return <ReactDevOverlay globalOverlay>{children}</ReactDevOverlay>
-  }
-}
-
 // Ensure the initialParallelRoutes are not combined because of double-rendering in the browser with Strict Mode.
 // TODO-APP: move this back into AppRouter
 let initialParallelRoutes: CacheNode['parallelRoutes'] =
@@ -106,12 +98,12 @@ export default function AppRouter({
   initialTree,
   initialCanonicalUrl,
   children,
-  hotReloader,
+  assetPrefix,
 }: {
   initialTree: FlightRouterState
   initialCanonicalUrl: string
   children: ReactNode
-  hotReloader?: ReactNode
+  assetPrefix: string
 }) {
   const initialState = useMemo(() => {
     return {
@@ -361,16 +353,13 @@ export default function AppRouter({
                 url: canonicalUrl,
               }}
             >
-              <ErrorOverlay>
-                {
-                  // ErrorOverlay intentionally only wraps the children of app-router.
-                  cache.subTreeData
-                }
-              </ErrorOverlay>
-              {
-                // HotReloader uses the router tree and router.reload() in order to apply Server Component changes.
-                hotReloader
-              }
+              {HotReloader ? (
+                <HotReloader assetPrefix={assetPrefix}>
+                  {cache.subTreeData}
+                </HotReloader>
+              ) : (
+                cache.subTreeData
+              )}
             </LayoutRouterContext.Provider>
           </AppRouterContext.Provider>
         </GlobalLayoutRouterContext.Provider>

diff --git a/...client/components/hot-reloader.client.tsx → ...s/next/client/components/hot-reloader.tsx b/...client/components/hot-reloader.client.tsx → ...s/next/client/components/hot-reloader.tsx
@@ -1,6 +1,5 @@
-'client'
-
-import {
+import type { ReactNode } from 'react'
+import React, {
   useCallback,
   useContext,
   useEffect,
@@ -15,6 +14,7 @@ import {
   onBuildError,
   onBuildOk,
   onRefresh,
+  ReactDevOverlay,
 } from 'next/dist/compiled/@next/react-dev-overlay/dist/client'
 import stripAnsi from 'next/dist/compiled/strip-ansi'
 import formatWebpackMessages from '../dev/error-overlay/format-webpack-messages'
@@ -398,7 +398,13 @@ function processMessage(
   }
 }
 
-export default function HotReload({ assetPrefix }: { assetPrefix: string }) {
+export default function HotReload({
+  assetPrefix,
+  children,
+}: {
+  assetPrefix: string
+  children?: ReactNode
+}) {
   const { tree } = useContext(GlobalLayoutRouterContext)
   const router = useRouter()
 
@@ -429,7 +435,7 @@ export default function HotReload({ assetPrefix }: { assetPrefix: string }) {
     }
 
     const { hostname, port } = window.location
-    const protocol = getSocketProtocol(assetPrefix || '')
+    const protocol = getSocketProtocol(assetPrefix)
     const normalizedAssetPrefix = assetPrefix.replace(/^\/+/, '')
 
     let url = `${protocol}://${hostname}:${port}${
@@ -494,5 +500,6 @@ export default function HotReload({ assetPrefix }: { assetPrefix: string }) {
 
   //   return () => clearInterval(interval)
   // })
-  return null
+
+  return <ReactDevOverlay globalOverlay>{children}</ReactDevOverlay>
 }
diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx
@@ -810,9 +810,6 @@ export async function renderToHTMLOrFlight(
       ComponentMod.LayoutRouter as typeof import('../client/components/layout-router.client').default
     const RenderFromTemplateContext =
       ComponentMod.RenderFromTemplateContext as typeof import('../client/components/render-from-template-context.client').default
-    const HotReloader = ComponentMod.HotReloader as
-      | typeof import('../client/components/hot-reloader.client').default
-      | null
 
     /**
      * Server Context is specifically only available in Server Components.
@@ -1373,11 +1370,7 @@ export async function renderToHTMLOrFlight(
 
         return (
           <AppRouter
-            hotReloader={
-              HotReloader && (
-                <HotReloader assetPrefix={renderOpts.assetPrefix || ''} />
-              )
-            }
+            assetPrefix={renderOpts.assetPrefix || ''}
             initialCanonicalUrl={initialCanonicalUrl}
             initialTree={initialTree}
           >

diff --git a/packages/next/taskfile.js b/packages/next/taskfile.js
@@ -1751,6 +1751,7 @@ export async function ncc_mini_css_extract_plugin(task, opts) {
     })
     .target('compiled/mini-css-extract-plugin')
 }
+
 // eslint-disable-next-line camelcase
 externals['ua-parser-js'] = 'next/dist/compiled/ua-parser-js'
 export async function ncc_ua_parser_js(task, opts) {

diff --git a/packages/react-dev-overlay/src/internal/ReactDevOverlay.tsx b/packages/react-dev-overlay/src/internal/ReactDevOverlay.tsx
@@ -60,69 +60,68 @@ const shouldPreventDisplay = (
   return preventType.includes(errorType)
 }
 
-const ReactDevOverlay: React.FunctionComponent = function ReactDevOverlay({
-  children,
-  preventDisplay,
-  globalOverlay,
-}: {
+type ReactDevOverlayProps = {
   children?: React.ReactNode
   preventDisplay?: ErrorType[]
   globalOverlay?: boolean
-}) {
-  const [state, dispatch] = React.useReducer<
-    React.Reducer<OverlayState, Bus.BusEvent>
-  >(reducer, {
-    nextId: 1,
-    buildError: null,
-    errors: [],
-  })
+}
 
-  React.useEffect(() => {
-    Bus.on(dispatch)
-    return function () {
-      Bus.off(dispatch)
-    }
-  }, [dispatch])
+const ReactDevOverlay: React.FunctionComponent<ReactDevOverlayProps> =
+  function ReactDevOverlay({ children, preventDisplay, globalOverlay }) {
+    const [state, dispatch] = React.useReducer<
+      React.Reducer<OverlayState, Bus.BusEvent>
+    >(reducer, {
+      nextId: 1,
+      buildError: null,
+      errors: [],
+    })
+
+    React.useEffect(() => {
+      Bus.on(dispatch)
+      return function () {
+        Bus.off(dispatch)
+      }
+    }, [dispatch])
 
-  const onComponentError = React.useCallback(
-    (_error: Error, _componentStack: string | null) => {
-      // TODO: special handling
-    },
-    []
-  )
+    const onComponentError = React.useCallback(
+      (_error: Error, _componentStack: string | null) => {
+        // TODO: special handling
+      },
+      []
+    )
 
-  const hasBuildError = state.buildError != null
-  const hasRuntimeErrors = Boolean(state.errors.length)
+    const hasBuildError = state.buildError != null
+    const hasRuntimeErrors = Boolean(state.errors.length)
 
-  const isMounted = hasBuildError || hasRuntimeErrors
+    const isMounted = hasBuildError || hasRuntimeErrors
 
-  return (
-    <React.Fragment>
-      <ErrorBoundary
-        globalOverlay={globalOverlay}
-        isMounted={isMounted}
-        onError={onComponentError}
-      >
-        {children ?? null}
-      </ErrorBoundary>
-      {isMounted ? (
-        <ShadowPortal globalOverlay={globalOverlay}>
-          <CssReset />
-          <Base />
-          <ComponentStyles />
+    return (
+      <React.Fragment>
+        <ErrorBoundary
+          globalOverlay={globalOverlay}
+          isMounted={isMounted}
+          onError={onComponentError}
+        >
+          {children ?? null}
+        </ErrorBoundary>
+        {isMounted ? (
+          <ShadowPortal globalOverlay={globalOverlay}>
+            <CssReset />
+            <Base />
+            <ComponentStyles />
 
-          {shouldPreventDisplay(
-            hasBuildError ? 'build' : hasRuntimeErrors ? 'runtime' : null,
-            preventDisplay
-          ) ? null : hasBuildError ? (
-            <BuildError message={state.buildError!} />
-          ) : hasRuntimeErrors ? (
-            <Errors errors={state.errors} />
-          ) : undefined}
-        </ShadowPortal>
-      ) : undefined}
-    </React.Fragment>
-  )
-}
+            {shouldPreventDisplay(
+              hasBuildError ? 'build' : hasRuntimeErrors ? 'runtime' : null,
+              preventDisplay
+            ) ? null : hasBuildError ? (
+              <BuildError message={state.buildError!} />
+            ) : hasRuntimeErrors ? (
+              <Errors errors={state.errors} />
+            ) : undefined}
+          </ShadowPortal>
+        ) : undefined}
+      </React.Fragment>
+    )
+  }
 
 export default ReactDevOverlay
diff --git a/packages/react-dev-overlay/src/internal/helpers/getRawSourceMap.ts b/packages/react-dev-overlay/src/internal/helpers/getRawSourceMap.ts
@@ -1,5 +1,5 @@
 import dataUriToBuffer, { MimeBuffer } from 'data-uri-to-buffer'
-import { RawSourceMap } from 'source-map'
+import type { RawSourceMap } from 'source-map'
 import { getSourceMapUrl } from './getSourceMapUrl'
 
 export function getRawSourceMap(fileContents: string): RawSourceMap | null {

diff --git a/packages/react-dev-overlay/src/middleware.ts b/packages/react-dev-overlay/src/middleware.ts
@@ -2,11 +2,8 @@ import { codeFrameColumns } from '@babel/code-frame'
 import { constants as FS, promises as fs } from 'fs'
 import { IncomingMessage, ServerResponse } from 'http'
 import path from 'path'
-import {
-  NullableMappedPosition,
-  RawSourceMap,
-  SourceMapConsumer,
-} from 'source-map'
+import type { NullableMappedPosition, RawSourceMap } from 'source-map'
+import { SourceMapConsumer } from 'source-map'
 import { StackFrame } from 'stacktrace-parser'
 import url from 'url'
 // @ts-ignore

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml