From 3e22b22afc7e736e04d00f3111f4aeb75457d8cf Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Sat, 9 Oct 2021 11:51:37 +0200 Subject: [PATCH] Hmr DX improvements (#29753) --- packages/next/build/output/index.ts | 129 +++++++++++------- packages/next/build/output/store.ts | 22 ++- .../server/dev/on-demand-entry-handler.ts | 10 +- test/integration/amphtml/test/index.test.js | 2 +- .../test/index.test.js | 7 + 5 files changed, 109 insertions(+), 61 deletions(-) diff --git a/packages/next/build/output/index.ts b/packages/next/build/output/index.ts index a0ea1dc7a6..26aac08424 100644 --- 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() 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 = { 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, + }) +} diff --git a/packages/next/build/output/store.ts b/packages/next/build/output/store.ts index 1090803d56..0c09b2e3f2 100644 --- 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) { - Log.wait('compiling...') + 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() }) diff --git a/packages/next/server/dev/on-demand-entry-handler.ts b/packages/next/server/dev/on-demand-entry-handler.ts index ca14c719e2..aa2c4efb47 100644 --- 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() } diff --git a/test/integration/amphtml/test/index.test.js b/test/integration/amphtml/test/index.test.js index 9e7f257a30..82493b71a4 100644 --- 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') }) }) diff --git a/test/integration/gssp-ssr-change-reloading/test/index.test.js b/test/integration/gssp-ssr-change-reloading/test/index.test.js index 41de7ac124..3fcd32e4a2 100644 --- 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())