From 4c128a5d6b0babe8ffe59997e2f0914a04f7658c Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 10 Apr 2024 10:09:03 +0200 Subject: [PATCH] Ensure configuration is checked for Turbopack build (#64247) ## What? Currently any configuration issue like including `.babelrc` was not highlighted during Turbopack build. This PR solves that issue as well as ensuring the warnings are not double-logged because of a change that was supposed to be development-only. Closes NEXT-3049 --- packages/next/src/build/index.ts | 35 ++++++++- packages/next/src/lib/turbopack-warning.ts | 32 ++++---- .../src/server/dev/hot-reloader-turbopack.ts | 7 +- .../next/src/server/dev/turbopack-utils.ts | 73 +++++++++++++------ packages/next/src/server/lib/start-server.ts | 2 +- 5 files changed, 104 insertions(+), 45 deletions(-) diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index c763c37578681..4ded891f673e3 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -45,6 +45,7 @@ import type { import { nonNullable } from '../lib/non-nullable' import { recursiveDelete } from '../lib/recursive-delete' import { verifyPartytownSetup } from '../lib/verify-partytown-setup' +import { validateTurboNextConfig } from '../lib/turbopack-warning' import { BUILD_ID_FILE, BUILD_MANIFEST, @@ -176,7 +177,7 @@ import { handleRouteType, handlePagesErrorRoute, formatIssue, - printNonFatalIssue, + isRelevantWarning, } from '../server/dev/turbopack-utils' import { TurbopackManifestLoader } from '../server/dev/turbopack/manifest-loader' import type { Entrypoints } from '../server/dev/turbopack/types' @@ -1346,6 +1347,11 @@ export default async function build( throw new Error("next build doesn't support turbopack yet") } + await validateTurboNextConfig({ + dir, + isDev: false, + }) + const startTime = process.hrtime() const bindings = await loadBindings(config?.experimental?.useWasmBinary) const dev = false @@ -1448,6 +1454,7 @@ export default async function build( manifestLoader, nextConfig: config, rewrites: emptyRewritesObjToBeImplemented, + logErrors: false, }) const progress = createProgress( @@ -1482,6 +1489,7 @@ export default async function build( entrypoints: currentEntrypoints, manifestLoader, rewrites: emptyRewritesObjToBeImplemented, + logErrors: false, }) ) } @@ -1497,6 +1505,7 @@ export default async function build( entrypoints: currentEntrypoints, manifestLoader, rewrites: emptyRewritesObjToBeImplemented, + logErrors: false, }) ) } @@ -1507,6 +1516,7 @@ export default async function build( entrypoints: currentEntrypoints, manifestLoader, rewrites: emptyRewritesObjToBeImplemented, + logErrors: false, }) ) await Promise.all(promises) @@ -1520,6 +1530,10 @@ export default async function build( page: string message: string }[] = [] + const warnings: { + page: string + message: string + }[] = [] for (const [page, entryIssues] of currentEntryIssues) { for (const issue of entryIssues.values()) { if (issue.severity !== 'warning') { @@ -1528,14 +1542,29 @@ export default async function build( message: formatIssue(issue), }) } else { - printNonFatalIssue(issue) + if (isRelevantWarning(issue)) { + warnings.push({ + page, + message: formatIssue(issue), + }) + } } } } + if (warnings.length > 0) { + Log.warn( + `Turbopack build collected ${warnings.length} warnings:\n${warnings + .map((e) => { + return 'Page: ' + e.page + '\n' + e.message + }) + .join('\n')}` + ) + } + if (errors.length > 0) { throw new Error( - `Turbopack build failed with ${errors.length} issues:\n${errors + `Turbopack build failed with ${errors.length} errors:\n${errors .map((e) => { return 'Page: ' + e.page + '\n' + e.message }) diff --git a/packages/next/src/lib/turbopack-warning.ts b/packages/next/src/lib/turbopack-warning.ts index 2d990316e4a33..c8a6d3b565cac 100644 --- a/packages/next/src/lib/turbopack-warning.ts +++ b/packages/next/src/lib/turbopack-warning.ts @@ -2,7 +2,10 @@ import type { NextConfig } from '../server/config-shared' import path from 'path' import loadConfig from '../server/config' import * as Log from '../build/output/log' -import { PHASE_DEVELOPMENT_SERVER } from '../shared/lib/constants' +import { + PHASE_DEVELOPMENT_SERVER, + PHASE_PRODUCTION_BUILD, +} from '../shared/lib/constants' const unsupportedTurbopackNextConfigOptions = [ // is this supported? @@ -48,21 +51,13 @@ const unsupportedTurbopackNextConfigOptions = [ ] // The following will need to be supported by `next build --turbo` -const prodSpecificTurboNextConfigOptions = [ - 'eslint', - 'typescript', +const unsupportedProductionSpecificTurbopackNextConfigOptions = [ 'outputFileTracing', - 'generateBuildId', - 'compress', - 'productionBrowserSourceMaps', - 'optimizeFonts', - 'poweredByHeader', - 'staticPageGenerationTimeout', + // TODO: Support disabling sourcemaps, currently they're always enabled. + // 'productionBrowserSourceMaps', 'reactProductionProfiling', - 'cleanDistDir', 'experimental.turbotrace', 'experimental.outputFileTracingRoot', - 'experimental.outputFileTracingExcludes', 'experimental.outputFileTracingIgnores', 'experimental.outputFileTracingIncludes', ] @@ -72,10 +67,7 @@ export async function validateTurboNextConfig({ dir, isDev, }: { - allowRetry?: boolean dir: string - port: number - hostname?: string isDev?: boolean }) { const { getPkgManager } = @@ -99,15 +91,16 @@ export async function validateTurboNextConfig({ let unsupportedConfig: string[] = [] let rawNextConfig: NextConfig = {} + const phase = isDev ? PHASE_DEVELOPMENT_SERVER : PHASE_PRODUCTION_BUILD try { rawNextConfig = interopDefault( - await loadConfig(PHASE_DEVELOPMENT_SERVER, dir, { + await loadConfig(phase, dir, { rawConfig: true, }) ) as NextConfig if (typeof rawNextConfig === 'function') { - rawNextConfig = (rawNextConfig as any)(PHASE_DEVELOPMENT_SERVER, { + rawNextConfig = (rawNextConfig as any)(phase, { defaultConfig, }) } @@ -150,7 +143,10 @@ export async function validateTurboNextConfig({ let unsupportedKeys = isDev ? unsupportedTurbopackNextConfigOptions - : prodSpecificTurboNextConfigOptions + : [ + ...unsupportedTurbopackNextConfigOptions, + ...unsupportedProductionSpecificTurbopackNextConfigOptions, + ] for (const key of customKeys) { if (key.startsWith('webpack') && rawNextConfig.webpack) { diff --git a/packages/next/src/server/dev/hot-reloader-turbopack.ts b/packages/next/src/server/dev/hot-reloader-turbopack.ts index 79d90d33b1b7f..3220403c2a1cb 100644 --- a/packages/next/src/server/dev/hot-reloader-turbopack.ts +++ b/packages/next/src/server/dev/hot-reloader-turbopack.ts @@ -363,7 +363,7 @@ export async function createHotReloaderTurbopack( const changed = await changedPromise for await (const change of changed) { - processIssues(currentEntryIssues, key, change) + processIssues(currentEntryIssues, key, change, false, true) const payload = await makePayload(change) if (payload) { sendHmr(key, payload) @@ -401,7 +401,7 @@ export async function createHotReloaderTurbopack( await subscription.next() for await (const data of subscription) { - processIssues(state.clientIssues, key, data) + processIssues(state.clientIssues, key, data, false, true) if (data.type !== 'issues') { sendTurbopackMessage(data) } @@ -453,6 +453,7 @@ export async function createHotReloaderTurbopack( manifestLoader, nextConfig: opts.nextConfig, rewrites: opts.fsChecker.rewrites, + logErrors: true, dev: { assetMapper, @@ -778,6 +779,7 @@ export async function createHotReloaderTurbopack( entrypoints: currentEntrypoints, manifestLoader, rewrites: opts.fsChecker.rewrites, + logErrors: true, hooks: { subscribeToChanges, @@ -830,6 +832,7 @@ export async function createHotReloaderTurbopack( manifestLoader, readyIds, rewrites: opts.fsChecker.rewrites, + logErrors: true, hooks: { subscribeToChanges, diff --git a/packages/next/src/server/dev/turbopack-utils.ts b/packages/next/src/server/dev/turbopack-utils.ts index 3ea283ca6a685..75d6d9a98460e 100644 --- a/packages/next/src/server/dev/turbopack-utils.ts +++ b/packages/next/src/server/dev/turbopack-utils.ts @@ -64,15 +64,22 @@ export function isWellKnownError(issue: Issue): boolean { /// Print out an issue to the console which should not block /// the build by throwing out or blocking error overlay. export function printNonFatalIssue(issue: Issue) { - // Currently only warnings will be printed, excluding if the error source - // is coming from foreign (node_modules) codes. - if (issue.severity === 'warning') { - if (!issue.filePath.match(/^(?:.*[\\/])?node_modules(?:[\\/].*)?$/)) { - Log.warn(formatIssue(issue)) - } + if (isRelevantWarning(issue)) { + Log.warn(formatIssue(issue)) } } +function isNodeModulesIssue(issue: Issue): boolean { + return ( + issue.severity === 'warning' && + issue.filePath.match(/^(?:.*[\\/])?node_modules(?:[\\/].*)?$/) !== null + ) +} + +export function isRelevantWarning(issue: Issue): boolean { + return issue.severity === 'warning' && !isNodeModulesIssue(issue) +} + export function formatIssue(issue: Issue) { const { filePath, title, description, source } = issue let { documentationLink } = issue @@ -94,7 +101,7 @@ export function formatIssue(issue: Issue) { .replaceAll('/./', '/') .replace('\\\\?\\', '') - let message + let message = '' if (source && source.range) { const { start } = source.range @@ -174,7 +181,8 @@ export function processIssues( currentEntryIssues: EntryIssuesMap, key: EntryKey, result: TurbopackResult, - throwIssue = false + throwIssue: boolean, + logErrors: boolean ) { const newIssues = new Map() currentEntryIssues.set(key, newIssues) @@ -195,7 +203,7 @@ export function processIssues( if (issue.severity !== 'warning') { relevantIssues.add(formatted) - if (isWellKnownError(issue)) { + if (logErrors && isWellKnownError(issue)) { Log.error(formatted) } } @@ -296,6 +304,7 @@ export async function handleRouteType({ readyIds, rewrites, hooks, + logErrors, }: { dev: boolean page: string @@ -306,6 +315,7 @@ export async function handleRouteType({ entrypoints: Entrypoints manifestLoader: TurbopackManifestLoader rewrites: SetupOpts['fsChecker']['rewrites'] + logErrors: boolean readyIds?: ReadyIds // dev @@ -322,7 +332,13 @@ export async function handleRouteType({ const writtenEndpoint = await entrypoints.global.app.writeToDisk() hooks?.handleWrittenEndpoint(key, writtenEndpoint) - processIssues(currentEntryIssues, key, writtenEndpoint) + processIssues( + currentEntryIssues, + key, + writtenEndpoint, + false, + logErrors + ) } await manifestLoader.loadBuildManifest('_app') await manifestLoader.loadPagesManifest('_app') @@ -333,7 +349,13 @@ export async function handleRouteType({ const writtenEndpoint = await entrypoints.global.document.writeToDisk() hooks?.handleWrittenEndpoint(key, writtenEndpoint) - processIssues(currentEntryIssues, key, writtenEndpoint) + processIssues( + currentEntryIssues, + key, + writtenEndpoint, + false, + logErrors + ) } await manifestLoader.loadPagesManifest('_document') @@ -358,7 +380,13 @@ export async function handleRouteType({ pageEntrypoints: entrypoints.page, }) - processIssues(currentEntryIssues, serverKey, writtenEndpoint) + processIssues( + currentEntryIssues, + serverKey, + writtenEndpoint, + false, + logErrors + ) } finally { // TODO subscriptions should only be caused by the WebSocket connections // otherwise we don't known when to unsubscribe and this leaking @@ -410,7 +438,7 @@ export async function handleRouteType({ pageEntrypoints: entrypoints.page, }) - processIssues(currentEntryIssues, key, writtenEndpoint, true) + processIssues(currentEntryIssues, key, writtenEndpoint, true, logErrors) break } @@ -454,7 +482,7 @@ export async function handleRouteType({ pageEntrypoints: entrypoints.page, }) - processIssues(currentEntryIssues, key, writtenEndpoint, dev) + processIssues(currentEntryIssues, key, writtenEndpoint, dev, logErrors) break } @@ -477,7 +505,7 @@ export async function handleRouteType({ rewrites, pageEntrypoints: entrypoints.page, }) - processIssues(currentEntryIssues, key, writtenEndpoint, true) + processIssues(currentEntryIssues, key, writtenEndpoint, true, logErrors) break } @@ -631,7 +659,7 @@ export async function handleEntrypoints({ manifestLoader, nextConfig, rewrites, - + logErrors, dev, }: { entrypoints: TurbopackResult @@ -642,6 +670,7 @@ export async function handleEntrypoints({ manifestLoader: TurbopackManifestLoader nextConfig: NextConfigComplete rewrites: SetupOpts['fsChecker']['rewrites'] + logErrors: boolean dev?: HandleEntrypointsDevOpts }) { @@ -719,7 +748,7 @@ export async function handleEntrypoints({ const writtenEndpoint = await instrumentation[prop].writeToDisk() dev?.hooks.handleWrittenEndpoint(key, writtenEndpoint) - processIssues(currentEntryIssues, key, writtenEndpoint) + processIssues(currentEntryIssues, key, writtenEndpoint, false, logErrors) } await processInstrumentation('instrumentation.nodeJs', 'nodeJs') await processInstrumentation('instrumentation.edge', 'edge') @@ -757,7 +786,7 @@ export async function handleEntrypoints({ async function processMiddleware() { const writtenEndpoint = await endpoint.writeToDisk() dev?.hooks.handleWrittenEndpoint(key, writtenEndpoint) - processIssues(currentEntryIssues, key, writtenEndpoint) + processIssues(currentEntryIssues, key, writtenEndpoint, false, logErrors) await manifestLoader.loadMiddlewareManifest('middleware', 'middleware') if (dev) { dev.serverFields.middleware = { @@ -880,6 +909,7 @@ export async function handlePagesErrorRoute({ entrypoints, manifestLoader, rewrites, + logErrors, hooks, }: { @@ -887,6 +917,7 @@ export async function handlePagesErrorRoute({ entrypoints: Entrypoints manifestLoader: TurbopackManifestLoader rewrites: SetupOpts['fsChecker']['rewrites'] + logErrors: boolean hooks?: HandleRouteTypeHooks // dev }) { @@ -900,7 +931,7 @@ export async function handlePagesErrorRoute({ // https://github.com/vercel/next.js/blob/08d7a7e5189a835f5dcb82af026174e587575c0e/packages/next/src/client/page-bootstrap.ts#L69-L71 return { event: HMR_ACTIONS_SENT_TO_BROWSER.CLIENT_CHANGES } }) - processIssues(currentEntryIssues, key, writtenEndpoint) + processIssues(currentEntryIssues, key, writtenEndpoint, false, logErrors) } await manifestLoader.loadBuildManifest('_app') await manifestLoader.loadPagesManifest('_app') @@ -914,7 +945,7 @@ export async function handlePagesErrorRoute({ hooks?.subscribeToChanges(key, false, entrypoints.global.document, () => { return { action: HMR_ACTIONS_SENT_TO_BROWSER.RELOAD_PAGE } }) - processIssues(currentEntryIssues, key, writtenEndpoint) + processIssues(currentEntryIssues, key, writtenEndpoint, false, logErrors) } await manifestLoader.loadPagesManifest('_document') @@ -928,7 +959,7 @@ export async function handlePagesErrorRoute({ // https://github.com/vercel/next.js/blob/08d7a7e5189a835f5dcb82af026174e587575c0e/packages/next/src/client/page-bootstrap.ts#L69-L71 return { event: HMR_ACTIONS_SENT_TO_BROWSER.CLIENT_CHANGES } }) - processIssues(currentEntryIssues, key, writtenEndpoint) + processIssues(currentEntryIssues, key, writtenEndpoint, false, logErrors) } await manifestLoader.loadBuildManifest('_error') await manifestLoader.loadPagesManifest('_error') diff --git a/packages/next/src/server/lib/start-server.ts b/packages/next/src/server/lib/start-server.ts index 48620758f0ef4..13f20677acce0 100644 --- a/packages/next/src/server/lib/start-server.ts +++ b/packages/next/src/server/lib/start-server.ts @@ -322,7 +322,7 @@ export async function startServer( if (process.env.TURBOPACK) { await validateTurboNextConfig({ - ...serverOptions, + dir: serverOptions.dir, isDev: true, }) }