Hmr DX improvements (#29753)

2021-10-09 11:51:37 +02:00 · 2021-10-09 11:51:37 +02:00 · 3e22b22afc
commit 3e22b22afc
parent 597fc3312f
5 changed files with 109 additions and 61 deletions
--- a/packages/next/build/output/index.ts
+++ b/packages/next/build/output/index.ts
@ -13,6 +13,7 @@ let previousClient: import('webpack').Compiler | null = null
 let previousServer: import('webpack').Compiler | null = null

 type CompilerDiagnostics = {
+  modules: number
  errors: string[] | null
  warnings: string[] | null
 }
@ -36,31 +37,10 @@ export type AmpPageStatus = {
 type BuildStatusStore = {
  client: WebpackStatus
  server: WebpackStatus
+  trigger: string | undefined
  amp: AmpPageStatus
 }

-// eslint typescript has a bug with TS enums
-/* eslint-disable no-shadow */
-enum WebpackStatusPhase {
-  COMPILING = 1,
-  COMPILED_WITH_ERRORS = 2,
-  COMPILED_WITH_WARNINGS = 4,
-  COMPILED = 5,
-}
-
-function getWebpackStatusPhase(status: WebpackStatus): WebpackStatusPhase {
-  if (status.loading) {
-    return WebpackStatusPhase.COMPILING
-  }
-  if (status.errors) {
-    return WebpackStatusPhase.COMPILED_WITH_ERRORS
-  }
-  if (status.warnings) {
-    return WebpackStatusPhase.COMPILED_WITH_WARNINGS
-  }
-  return WebpackStatusPhase.COMPILED
-}
-
 export function formatAmpMessages(amp: AmpPageStatus) {
  let output = chalk.bold('Amp Validation') + '\n\n'
  let messages: string[][] = []
@ -118,45 +98,66 @@ export function formatAmpMessages(amp: AmpPageStatus) {
 const buildStore = createStore<BuildStatusStore>()

 buildStore.subscribe((state) => {
-  const { amp, client, server } = state
-
-  const [{ status }] = [
-    { status: client, phase: getWebpackStatusPhase(client) },
-    { status: server, phase: getWebpackStatusPhase(server) },
-  ].sort((a, b) => a.phase.valueOf() - b.phase.valueOf())
+  const { amp, client, server, trigger } = state

  const { bootstrap: bootstrapping, appUrl } = consoleStore.getState()
-  if (bootstrapping && status.loading) {
+  if (bootstrapping && (client.loading || server.loading)) {
+    return
+  }
+
+  if (client.loading || server.loading) {
+    consoleStore.setState(
+      {
+        bootstrap: false,
+        appUrl: appUrl!,
+        loading: true,
+        trigger,
+      } as OutputState,
+      true
+    )
    return
  }

  let partialState: Partial<OutputState> = {
    bootstrap: false,
    appUrl: appUrl!,
+    loading: false,
+    typeChecking: false,
+    modules: client.modules + server.modules,
  }
-
-  if (status.loading) {
+  if (client.errors) {
+    // Show only client errors
    consoleStore.setState(
-      { ...partialState, loading: true } as OutputState,
+      {
+        ...partialState,
+        errors: client.errors,
+        warnings: null,
+      } as OutputState,
+      true
+    )
+  } else if (server.errors) {
+    // Show only server errors
+    consoleStore.setState(
+      {
+        ...partialState,
+        errors: server.errors,
+        warnings: null,
+      } as OutputState,
      true
    )
  } else {
-    let { errors, warnings } = status
-
-    if (errors == null) {
-      if (Object.keys(amp).length > 0) {
-        warnings = (warnings || []).concat(formatAmpMessages(amp) || [])
-        if (!warnings.length) warnings = null
-      }
-    }
+    // Show warnings from all of them
+    const warnings = [
+      ...(client.warnings || []),
+      ...(server.warnings || []),
+      ...((Object.keys(amp).length > 0 && formatAmpMessages(amp)) || []),
+    ]

    consoleStore.setState(
      {
        ...partialState,
-        loading: false,
-        typeChecking: false,
-        errors,
-        warnings,
+        errors: null,
+        warnings: warnings.length === 0 ? null : warnings,
      } as OutputState,
      true
    )
@ -200,6 +201,7 @@ export function watchCompilers(
  buildStore.setState({
    client: { loading: true },
    server: { loading: true },
+    trigger: 'initial',
  })

  function tapCompiler(
@ -213,7 +215,7 @@ export function watchCompilers(

    compiler.hooks.done.tap(
      `NextJsDone-${key}`,
-      (stats: import('webpack').Stats) => {
+      (stats: import('webpack5').Stats) => {
        buildStore.setState({ amp: {} })

        const { errors, warnings } = formatWebpackMessages(
@ -225,6 +227,7 @@ export function watchCompilers(

        onEvent({
          loading: false,
+          modules: stats.compilation.modules.size,
          errors: hasErrors ? errors : null,
          warnings: hasWarnings ? warnings : null,
        })
@ -232,13 +235,37 @@ export function watchCompilers(
    )
  }

-  tapCompiler('client', client, (status) =>
-    buildStore.setState({ client: status })
-  )
-  tapCompiler('server', server, (status) =>
-    buildStore.setState({ server: status })
-  )
+  tapCompiler('client', client, (status) => {
+    if (!status.loading && !buildStore.getState().server.loading) {
+      buildStore.setState({
+        client: status,
+        trigger: undefined,
+      })
+    } else {
+      buildStore.setState({
+        client: status,
+      })
+    }
+  })
+  tapCompiler('server', server, (status) => {
+    if (!status.loading && !buildStore.getState().client.loading) {
+      buildStore.setState({
+        server: status,
+        trigger: undefined,
+      })
+    } else {
+      buildStore.setState({
+        server: status,
+      })
+    }
+  })

  previousClient = client
  previousServer = server
 }
+
+export function reportTrigger(trigger: string) {
+  buildStore.setState({
+    trigger,
+  })
+}
--- a/packages/next/build/output/store.ts
+++ b/packages/next/build/output/store.ts
@ -7,10 +7,14 @@ import * as Log from './log'
 export type OutputState =
  | { bootstrap: true; appUrl: string | null; bindAddr: string | null }
  | ({ bootstrap: false; appUrl: string | null; bindAddr: string | null } & (
-      | { loading: true }
+      | {
+          loading: true
+          trigger: string | undefined
+        }
      | {
          loading: false
          typeChecking: boolean
+          modules: number
          errors: string[] | null
          warnings: string[] | null
        }
@ -49,11 +53,16 @@ store.subscribe((state) => {
    if (state.appUrl) {
      Log.ready(`started server on ${state.bindAddr}, url: ${state.appUrl}`)
    }
+    if (startTime === 0) startTime = Date.now()
    return
  }

  if (state.loading) {
+    if (state.trigger) {
+      Log.wait(`compiling ${state.trigger}...`)
+    } else {
      Log.wait('compiling...')
+    }
    if (startTime === 0) startTime = Date.now()
    return
  }
@ -89,6 +98,11 @@ store.subscribe((state) => {
      time > 2000 ? ` in ${Math.round(time / 100) / 10} s` : ` in ${time} ms`
  }

+  let modulesMessage = ''
+  if (state.modules) {
+    modulesMessage = ` (${state.modules} modules)`
+  }
+
  if (state.warnings) {
    Log.warn(state.warnings.join('\n\n'))
    // Ensure traces are flushed after each compile in development mode
@ -98,12 +112,12 @@ store.subscribe((state) => {

  if (state.typeChecking) {
    Log.info(
-      `bundled successfully${timeMessage}, waiting for typecheck results...`
+      `bundled successfully${timeMessage}${modulesMessage}, waiting for typecheck results...`
    )
    return
  }

-  Log.event(`compiled successfully${timeMessage}`)
+  Log.event(`compiled successfully${timeMessage}${modulesMessage}`)
  // Ensure traces are flushed after each compile in development mode
  flushAllTraces()
 })
--- a/packages/next/server/dev/on-demand-entry-handler.ts
+++ b/packages/next/server/dev/on-demand-entry-handler.ts
@ -3,12 +3,12 @@ import { IncomingMessage, ServerResponse } from 'http'
 import { join, posix } from 'path'
 import { parse } from 'url'
 import { webpack } from 'next/dist/compiled/webpack/webpack'
-import * as Log from '../../build/output/log'
 import { normalizePagePath, normalizePathSep } from '../normalize-page-path'
 import { pageNotFoundError } from '../require'
 import { findPageFile } from '../lib/find-page-file'
 import getRouteFromEntrypoint from '../get-route-from-entrypoint'
 import { API_ROUTE } from '../../lib/constants'
+import { reportTrigger } from '../../build/output'

 export const ADDED = Symbol('added')
 export const BUILDING = Symbol('building')
@ -228,12 +228,12 @@ export default function onDemandEntryHandler(
        : Promise.all([addPageEntry('client'), addPageEntry('server')])

      if (entriesChanged) {
-        Log.event(
+        reportTrigger(
          isApiRoute
-            ? `build page: ${normalizedPage} (server only)`
+            ? `${normalizedPage} (server only)`
            : clientOnly
-            ? `build page: ${normalizedPage} (client only)`
-            : `build page: ${normalizedPage}`
+            ? `${normalizedPage} (client only)`
+            : normalizedPage
        )
        invalidator.invalidate()
      }
--- a/test/integration/amphtml/test/index.test.js
+++ b/test/integration/amphtml/test/index.test.js
@ -520,7 +520,7 @@ describe('AMP Usage', () => {

    it('should not contain missing files warning', async () => {
      expect(output).toContain('compiled successfully')
-      expect(output).toContain('build page: /only-amp')
+      expect(output).toContain('compiling /only-amp')
      expect(output).not.toContain('Could not find files for')
    })
  })
--- a/test/integration/gssp-ssr-change-reloading/test/index.test.js
+++ b/test/integration/gssp-ssr-change-reloading/test/index.test.js
@ -31,6 +31,7 @@ describe('GS(S)P Server-Side Change Reloading', () => {

  it('should not reload page when client-side is changed too GSP', async () => {
    const browser = await webdriver(appPort, '/gsp-blog/first')
+    await check(() => browser.elementByCss('#change').text(), 'change me')
    await browser.eval(() => (window.beforeChange = 'hi'))

    const props = JSON.parse(await browser.elementByCss('#props').text())
@ -145,6 +146,7 @@ describe('GS(S)P Server-Side Change Reloading', () => {

  it('should not reload page when client-side is changed too GSSP', async () => {
    const browser = await webdriver(appPort, '/gssp-blog/first')
+    await check(() => browser.elementByCss('#change').text(), 'change me')
    await browser.eval(() => (window.beforeChange = 'hi'))

    const props = JSON.parse(await browser.elementByCss('#props').text())
@ -165,6 +167,11 @@ describe('GS(S)P Server-Side Change Reloading', () => {

  it('should update page when getServerSideProps is changed only', async () => {
    const browser = await webdriver(appPort, '/gssp-blog/first')
+    await check(
+      async () =>
+        JSON.parse(await browser.elementByCss('#props').text()).count + '',
+      '1'
+    )
    await browser.eval(() => (window.beforeChange = 'hi'))

    const props = JSON.parse(await browser.elementByCss('#props').text())