From bf3e41082932f4bf7d828e18ab0346b2ac8b59c9 Mon Sep 17 00:00:00 2001 From: mato533 <35281743+mato533@users.noreply.github.com> Date: Thu, 23 Jan 2025 23:26:23 +0900 Subject: [PATCH] feat: call Logger for plugin logs in build (#13757) Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com> --- eslint.config.js | 14 +- .../vite/src/node/__tests__/build.spec.ts | 166 +++++++++++++++++- packages/vite/src/node/build.ts | 123 ++++++++----- packages/vite/src/node/plugins/worker.ts | 6 +- 4 files changed, 251 insertions(+), 58 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 968a7a89b5e1a4..6fc2d08520070a 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -8,7 +8,6 @@ import tseslint from 'typescript-eslint' import globals from 'globals' const require = createRequire(import.meta.url) -const pkg = require('./package.json') const pkgVite = require('./packages/vite/package.json') // Some rules work better with typechecking enabled, but as enabling it is slow, @@ -51,6 +50,11 @@ export default tseslint.config( ...globals.node, }, }, + settings: { + node: { + version: '^18.0.0 || ^20.0.0 || >=22.0.0', + }, + }, plugins: { n: pluginN, 'import-x': pluginImportX, @@ -213,17 +217,9 @@ export default tseslint.config( name: 'playground/test', files: ['playground/**/__tests__/**/*.?([cm])[jt]s?(x)'], rules: { - // engine field doesn't exist in playgrounds - 'n/no-unsupported-features/es-builtins': [ - 'error', - { - version: pkg.engines.node, - }, - ], 'n/no-unsupported-features/node-builtins': [ 'error', { - version: pkg.engines.node, // ideally we would like to allow all experimental features // https://github.com/eslint-community/eslint-plugin-n/issues/199 ignores: ['fetch'], diff --git a/packages/vite/src/node/__tests__/build.spec.ts b/packages/vite/src/node/__tests__/build.spec.ts index f5dd1419f04d7c..8f4e8f5455056c 100644 --- a/packages/vite/src/node/__tests__/build.spec.ts +++ b/packages/vite/src/node/__tests__/build.spec.ts @@ -1,17 +1,27 @@ import { basename, resolve } from 'node:path' import { fileURLToPath } from 'node:url' +import { stripVTControlCharacters } from 'node:util' import colors from 'picocolors' -import { describe, expect, test, vi } from 'vitest' -import type { OutputChunk, OutputOptions, RollupOutput } from 'rollup' +import { afterEach, describe, expect, test, vi } from 'vitest' +import type { + LogLevel, + OutputChunk, + OutputOptions, + RollupLog, + RollupOptions, + RollupOutput, +} from 'rollup' import type { LibraryFormats, LibraryOptions } from '../build' import { build, createBuilder, + onRollupLog, resolveBuildOutputs, resolveLibFilename, } from '../build' import type { Logger } from '../logger' import { createLogger } from '../logger' +import { BuildEnvironment, resolveConfig } from '..' const __dirname = resolve(fileURLToPath(import.meta.url), '..') @@ -876,6 +886,158 @@ test('adjust worker build error for worker.format', async () => { expect.unreachable() }) +describe('onRollupLog', () => { + const pluginName = 'rollup-plugin-test' + const msgInfo = 'This is the INFO message.' + const msgWarn = 'This is the WARN message.' + const buildProject = async ( + level: LogLevel | 'error', + message: string | RollupLog, + logger: Logger, + options?: Pick, + ) => { + await build({ + root: resolve(__dirname, 'packages/build-project'), + logLevel: 'info', + build: { + write: false, + rollupOptions: { + ...options, + logLevel: 'debug', + }, + }, + customLogger: logger, + plugins: [ + { + name: pluginName, + resolveId(id) { + this[level](message) + if (id === 'entry.js') { + return '\0' + id + } + }, + load(id) { + if (id === '\0entry.js') { + return `export default "This is test module";` + } + }, + }, + ], + }) + } + + const callOnRollupLog = async ( + logger: Logger, + level: LogLevel, + log: RollupLog, + ) => { + const config = await resolveConfig( + { customLogger: logger }, + 'build', + 'production', + 'production', + ) + const buildEnvironment = new BuildEnvironment('client', config) + onRollupLog(level, log, buildEnvironment) + } + + afterEach(() => { + vi.restoreAllMocks() + }) + + test('Rollup logs of info should be handled by vite', async () => { + const logger = createLogger() + const loggerSpy = vi.spyOn(logger, 'info').mockImplementation(() => {}) + + await buildProject('info', msgInfo, logger) + const logs = loggerSpy.mock.calls.map((args) => + stripVTControlCharacters(args[0]), + ) + expect(logs).contain(`[plugin ${pluginName}] ${msgInfo}`) + }) + + test('Rollup logs of warn should be handled by vite', async () => { + const logger = createLogger('silent') + const loggerSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + await buildProject('warn', msgWarn, logger) + const logs = loggerSpy.mock.calls.map((args) => + stripVTControlCharacters(args[0]), + ) + expect(logs).contain(`[plugin ${pluginName}] ${msgWarn}`) + }) + + test('onLog passed by user is called', async () => { + const logger = createLogger('silent') + + const onLogInfo = vi.fn((_log: RollupLog) => {}) + await buildProject('info', msgInfo, logger, { + onLog(level, log) { + if (level === 'info') { + onLogInfo(log) + } + }, + }) + expect(onLogInfo).toBeCalledWith( + expect.objectContaining({ message: `[plugin ${pluginName}] ${msgInfo}` }), + ) + }) + + test('onwarn passed by user is called', async () => { + const logger = createLogger('silent') + + const onWarn = vi.fn((_log: RollupLog) => {}) + await buildProject('warn', msgWarn, logger, { + onwarn(warning) { + onWarn(warning) + }, + }) + expect(onWarn).toBeCalledWith( + expect.objectContaining({ message: `[plugin ${pluginName}] ${msgWarn}` }), + ) + }) + + test('should throw error when warning contains UNRESOLVED_IMPORT', async () => { + const logger = createLogger() + await expect(() => + callOnRollupLog(logger, 'warn', { + code: 'UNRESOLVED_IMPORT', + message: 'test', + }), + ).rejects.toThrowError(/Rollup failed to resolve import/) + }) + + test.each([[`Unsupported expression`], [`statically analyzed`]])( + 'should ignore dynamic import warnings (%s)', + async (message: string) => { + const logger = createLogger() + const loggerSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + await callOnRollupLog(logger, 'warn', { + code: 'PLUGIN_WARNING', + message: message, + plugin: 'rollup-plugin-dynamic-import-variables', + }) + expect(loggerSpy).toBeCalledTimes(0) + }, + ) + + test.each([[`CIRCULAR_DEPENDENCY`], [`THIS_IS_UNDEFINED`]])( + 'should ignore some warnings (%s)', + async (code: string) => { + const logger = createLogger() + const loggerSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + await callOnRollupLog(logger, 'warn', { + code: code, + message: 'test message', + plugin: pluginName, + }) + expect(loggerSpy).toBeCalledTimes(0) + }, + ) +}) + /** * for each chunks in output1, if there's a chunk in output2 with the same fileName, * ensure that the chunk code is the same. if not, the chunk hash should have changed. diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts index ccc953c010043c..53f979655e9885 100644 --- a/packages/vite/src/node/build.ts +++ b/packages/vite/src/node/build.ts @@ -5,7 +5,8 @@ import type { ExternalOption, InputOption, InternalModuleFormat, - LoggingFunction, + LogLevel, + LogOrStringHandler, ModuleFormat, OutputOptions, RollupBuild, @@ -14,6 +15,7 @@ import type { RollupOptions, RollupOutput, RollupWatcher, + WarningHandlerWithDefault, WatcherOptions, } from 'rollup' import commonjsPlugin from '@rollup/plugin-commonjs' @@ -42,6 +44,7 @@ import { arraify, asyncFlatten, copyDir, + createDebugger, displayTime, emptyDir, getPkgName, @@ -596,8 +599,8 @@ async function buildEnvironment( input, plugins, external: options.rollupOptions.external, - onwarn(warning, warn) { - onRollupWarning(warning, warn, environment) + onLog(level, log) { + onRollupLog(level, log, environment) }, } @@ -1006,70 +1009,102 @@ function clearLine() { } } -export function onRollupWarning( - warning: RollupLog, - warn: LoggingFunction, +export function onRollupLog( + level: LogLevel, + log: RollupLog, environment: BuildEnvironment, ): void { - const viteWarn: LoggingFunction = (warnLog) => { - let warning: string | RollupLog - - if (typeof warnLog === 'function') { - warning = warnLog() - } else { - warning = warnLog - } - - if (typeof warning === 'object') { - if (warning.code === 'UNRESOLVED_IMPORT') { - const id = warning.id - const exporter = warning.exporter - // throw unless it's commonjs external... - if (!id || !id.endsWith('?commonjs-external')) { - throw new Error( - `[vite]: Rollup failed to resolve import "${exporter}" from "${id}".\n` + - `This is most likely unintended because it can break your application at runtime.\n` + - `If you do want to externalize this module explicitly add it to\n` + - `\`build.rollupOptions.external\``, - ) - } + const debugLogger = createDebugger('vite:build') + const viteLog: LogOrStringHandler = (logLeveling, rawLogging) => { + const logging = + typeof rawLogging === 'object' ? rawLogging : { message: rawLogging } + + if (logging.code === 'UNRESOLVED_IMPORT') { + const id = logging.id + const exporter = logging.exporter + // throw unless it's commonjs external... + if (!id || !id.endsWith('?commonjs-external')) { + throw new Error( + `[vite]: Rollup failed to resolve import "${exporter}" from "${id}".\n` + + `This is most likely unintended because it can break your application at runtime.\n` + + `If you do want to externalize this module explicitly add it to\n` + + `\`build.rollupOptions.external\``, + ) } + } + if (logLeveling === 'warn') { if ( - warning.plugin === 'rollup-plugin-dynamic-import-variables' && + logging.plugin === 'rollup-plugin-dynamic-import-variables' && dynamicImportWarningIgnoreList.some((msg) => - warning.message.includes(msg), + logging.message.includes(msg), ) ) { return } - if (warningIgnoreList.includes(warning.code!)) { + if (warningIgnoreList.includes(logging.code!)) { return } + } - if (warning.code === 'PLUGIN_WARNING') { - environment.logger.warn( - `${colors.bold( - colors.yellow(`[plugin:${warning.plugin}]`), - )} ${colors.yellow(warning.message)}`, - ) + switch (logLeveling) { + case 'info': + environment.logger.info(logging.message) + return + case 'warn': + environment.logger.warn(colors.yellow(logging.message)) + return + case 'error': + environment.logger.error(colors.red(logging.message)) + return + case 'debug': + debugLogger?.(logging.message) + return + default: + logLeveling satisfies never + // fallback to info if a unknown log level is passed + environment.logger.info(logging.message) return - } } - - warn(warnLog) } clearLine() - const userOnWarn = environment.config.build.rollupOptions.onwarn - if (userOnWarn) { - userOnWarn(warning, viteWarn) + const userOnLog = environment.config.build.rollupOptions?.onLog + const userOnWarn = environment.config.build.rollupOptions?.onwarn + if (userOnLog) { + if (userOnWarn) { + const normalizedUserOnWarn = normalizeUserOnWarn(userOnWarn, viteLog) + userOnLog(level, log, normalizedUserOnWarn) + } else { + userOnLog(level, log, viteLog) + } + } else if (userOnWarn) { + const normalizedUserOnWarn = normalizeUserOnWarn(userOnWarn, viteLog) + normalizedUserOnWarn(level, log) } else { - viteWarn(warning) + viteLog(level, log) } } +function normalizeUserOnWarn( + userOnWarn: WarningHandlerWithDefault, + defaultHandler: LogOrStringHandler, +): LogOrStringHandler { + return (logLevel, logging) => { + if (logLevel === 'warn') { + userOnWarn(normalizeLog(logging), (log) => + defaultHandler('warn', typeof log === 'function' ? log() : log), + ) + } else { + defaultHandler(logLevel, logging) + } + } +} + +const normalizeLog = (log: RollupLog | string): RollupLog => + typeof log === 'string' ? { message: log } : log + export function resolveUserExternal( user: ExternalOption, id: string, diff --git a/packages/vite/src/node/plugins/worker.ts b/packages/vite/src/node/plugins/worker.ts index b7eea41aac0a6a..15dd48879b7479 100644 --- a/packages/vite/src/node/plugins/worker.ts +++ b/packages/vite/src/node/plugins/worker.ts @@ -15,7 +15,7 @@ import { BuildEnvironment, createToImportMetaURLBasedRelativeRuntime, injectEnvironmentToHooks, - onRollupWarning, + onRollupLog, toOutputFilePathInJS, } from '../build' import { cleanUrl } from '../../shared/utils' @@ -85,8 +85,8 @@ async function bundleWorkerEntry( plugins: workerEnvironment.plugins.map((p) => injectEnvironmentToHooks(workerEnvironment, p), ), - onwarn(warning, warn) { - onRollupWarning(warning, warn, workerEnvironment) + onLog(level, log) { + onRollupLog(level, log, workerEnvironment) }, preserveEntrySignatures: false, })