From 67e68b4c2afb5e8fa441a27e359aabcba1714baf Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 20 Nov 2023 18:05:52 +0800 Subject: [PATCH 1/7] Initial cleanup --- packages/astro/src/core/build/static-build.ts | 5 +-- packages/astro/src/core/create-vite.ts | 16 +------- packages/astro/src/core/logger/vite.ts | 39 +++++++++++++++++++ packages/astro/src/vite-plugin-astro/hmr.ts | 7 ---- packages/astro/src/vite-plugin-astro/index.ts | 30 -------------- 5 files changed, 42 insertions(+), 55 deletions(-) create mode 100644 packages/astro/src/core/logger/vite.ts diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts index 95404c6d677f..ff73e924c7c6 100644 --- a/packages/astro/src/core/build/static-build.ts +++ b/packages/astro/src/core/build/static-build.ts @@ -159,8 +159,7 @@ async function ssrBuild( const viteBuildConfig: vite.InlineConfig = { ...viteConfig, mode: viteConfig.mode || 'production', - // Check using `settings...` as `viteConfig` always defaults to `warn` by Astro - logLevel: settings.config.vite.logLevel ?? 'error', + logLevel: viteConfig.logLevel ?? 'error', build: { target: 'esnext', // Vite defaults cssMinify to false in SSR by default, but we want to minify it @@ -292,8 +291,6 @@ async function clientBuild( const viteBuildConfig: vite.InlineConfig = { ...viteConfig, mode: viteConfig.mode || 'production', - // Check using `settings...` as `viteConfig` always defaults to `warn` by Astro - logLevel: settings.config.vite.logLevel ?? 'info', build: { target: 'esnext', ...viteConfig.build, diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index 2b677c1d9d34..62b848363c68 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -1,6 +1,5 @@ import nodeFs from 'node:fs'; import { fileURLToPath } from 'node:url'; -import type { Logger as ViteLogger } from 'vite'; import * as vite from 'vite'; import { crawlFrameworkPkgs } from 'vitefu'; import type { AstroSettings } from '../@types/astro.js'; @@ -31,6 +30,7 @@ import astroScriptsPlugin from '../vite-plugin-scripts/index.js'; import astroScriptsPageSSRPlugin from '../vite-plugin-scripts/page-ssr.js'; import { vitePluginSSRManifest } from '../vite-plugin-ssr-manifest/index.js'; import type { Logger } from './logger/core.js'; +import { createViteLogger } from './logger/vite.js'; import { vitePluginMiddleware } from './middleware/vite-plugin.js'; import { joinPaths } from './path.js'; @@ -102,25 +102,13 @@ export async function createVite( }, }); - const viteCustomLogger: ViteLogger = { - ...vite.createLogger('warn'), - // All error log messages are also thrown as real errors, - // so we can safely ignore them here and let the error handler - // log them for the user instead. - error: (msg) => logger.debug('vite', 'ERROR ' + msg), - // Warnings are usually otherwise ignored by Vite, so it's - // important that we catch and log them here. - warn: (msg) => logger.warn('vite', msg), - }; - // Start with the Vite configuration that Astro core needs const commonConfig: vite.InlineConfig = { // Tell Vite not to combine config from vite.config.js with our provided inline config configFile: false, cacheDir: fileURLToPath(new URL('./node_modules/.vite/', settings.config.root)), // using local caches allows Astro to be used in monorepos, etc. clearScreen: false, // we want to control the output, not Vite - logLevel: 'warn', // log warnings and errors only - customLogger: viteCustomLogger, + customLogger: createViteLogger(logger), appType: 'custom', optimizeDeps: { entries: ['src/**/*'], diff --git a/packages/astro/src/core/logger/vite.ts b/packages/astro/src/core/logger/vite.ts new file mode 100644 index 000000000000..c347fbdd04de --- /dev/null +++ b/packages/astro/src/core/logger/vite.ts @@ -0,0 +1,39 @@ +import type { Logger as ViteLogger, Rollup } from 'vite'; +import type { Logger as AstroLogger } from './core.js'; + +export function createViteLogger(astroLogger: AstroLogger): ViteLogger { + const warnedMessages = new Set(); + const loggedErrors = new WeakSet(); + + const logger: ViteLogger = { + hasWarned: false, + + info(msg) { + astroLogger.info('vite', msg); + }, + warn(msg) { + logger.hasWarned = true; + astroLogger.warn('vite', msg); + }, + warnOnce(msg) { + if (warnedMessages.has(msg)) return; + logger.hasWarned = true; + astroLogger.warn('vite', msg); + warnedMessages.add(msg); + }, + error(msg, opts) { + logger.hasWarned = true; + astroLogger.error('vite', msg); + if (opts?.error) { + loggedErrors.add(opts.error); + } + }, + // Don't allow clear screen + clearScreen: () => {}, + hasErrorLogged(error) { + return loggedErrors.has(error); + }, + }; + + return logger; +} diff --git a/packages/astro/src/vite-plugin-astro/hmr.ts b/packages/astro/src/vite-plugin-astro/hmr.ts index 50e1bd25ae5b..a4bfca966892 100644 --- a/packages/astro/src/vite-plugin-astro/hmr.ts +++ b/packages/astro/src/vite-plugin-astro/hmr.ts @@ -1,4 +1,3 @@ -import { slash } from '@astrojs/internal-helpers/path'; import { fileURLToPath } from 'node:url'; import type { HmrContext, ModuleNode } from 'vite'; import type { AstroConfig } from '../@types/astro.js'; @@ -108,12 +107,6 @@ export async function handleHotUpdate( } } - // TODO: Svelte files should be marked as `isSelfAccepting` but they don't appear to be - const isSelfAccepting = mods.every((m) => m.isSelfAccepting || m.url.endsWith('.svelte')); - if (!isSelfAccepting) { - logger.debug('watch', 'full page reload triggered'); - } - return mods; } diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index 63198990387b..2aa6236ffe95 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -4,7 +4,6 @@ import type { AstroSettings } from '../@types/astro.js'; import type { Logger } from '../core/logger/core.js'; import type { PluginMetadata as AstroPluginMetadata } from './types.js'; -import { fileURLToPath } from 'url'; import { normalizePath } from 'vite'; import { cachedCompilation, @@ -24,28 +23,6 @@ interface AstroPluginOptions { logger: Logger; } -const PKG_PREFIX = fileURLToPath(new URL('../../', import.meta.url)); -const E2E_PREFIX = fileURLToPath(new URL('../../e2e', import.meta.url)); -const isPkgFile = (id: string | null) => { - return id?.startsWith(PKG_PREFIX) && !id.startsWith(E2E_PREFIX); -}; - -const dedupeHotUpdateLogsCache = new Map(); - -// TODO(fks): For some reason, we're seeing duplicate handleHotUpdate() calls -// when hitting save multiple times in a row. This is a temporary workaround -// to prevent duplicate logging until the (vite?) issue is fixed. -function dedupeHotUpdateLogs(filename: string) { - if (dedupeHotUpdateLogsCache.has(filename)) { - return false; - } - dedupeHotUpdateLogsCache.set( - filename, - setTimeout(() => dedupeHotUpdateLogsCache.delete(filename), 150) - ); - return true; -} - /** Transform .astro files for Vite */ export default function astro({ settings, logger }: AstroPluginOptions): vite.Plugin[] { const { config } = settings; @@ -197,13 +174,6 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl async handleHotUpdate(context) { if (context.server.config.isProduction) return; const filename = context.file; - const isSkipLog = - /astro\.config\.[cm][jt]s$/.test(filename) || - /(\/|\\)\.astro(\/|\\)/.test(filename) || - isPkgFile(filename); - if (!isSkipLog && dedupeHotUpdateLogs(filename)) { - logger.info('watch', filename.replace(config.root.pathname, '/')); - } const source = await context.read(); const compile = () => cachedCompilation({ From 7b25cace695b225bd588abac34599a681c57aaa2 Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 20 Nov 2023 18:31:41 +0800 Subject: [PATCH 2/7] Rewrite HMR messages to Astro --- packages/astro/src/core/logger/vite.ts | 31 ++++++++++++++++++++- packages/astro/src/vite-plugin-astro/hmr.ts | 12 ++------ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/packages/astro/src/core/logger/vite.ts b/packages/astro/src/core/logger/vite.ts index c347fbdd04de..7fa523c27ae3 100644 --- a/packages/astro/src/core/logger/vite.ts +++ b/packages/astro/src/core/logger/vite.ts @@ -1,6 +1,19 @@ +import { fileURLToPath } from 'url'; +import stripAnsi from 'strip-ansi'; import type { Logger as ViteLogger, Rollup } from 'vite'; import type { Logger as AstroLogger } from './core.js'; +const PKG_PREFIX = fileURLToPath(new URL('../../../', import.meta.url)); +const E2E_PREFIX = fileURLToPath(new URL('../../../e2e', import.meta.url)); +export function isAstroSrcFile(id: string | null) { + return id?.startsWith(PKG_PREFIX) && !id.startsWith(E2E_PREFIX); +} + +// capture "page reload some/Component.vue (additional info)" messages +const vitePageReloadMsg = /page reload (.*)( \(.*\))?/; +// capture "hmr update some/Component.vue" messages +const viteHmrUpdateMsg = /hmr update (.*)/; + export function createViteLogger(astroLogger: AstroLogger): ViteLogger { const warnedMessages = new Set(); const loggedErrors = new WeakSet(); @@ -9,7 +22,23 @@ export function createViteLogger(astroLogger: AstroLogger): ViteLogger { hasWarned: false, info(msg) { - astroLogger.info('vite', msg); + const stripped = stripAnsi(msg); + let m; + // Rewrite HMR page reload message + if ((m = vitePageReloadMsg.exec(stripped))) { + if (isAstroSrcFile(m[1])) return; + const extra = m[2] ?? ''; + astroLogger.info('watch', m[1] + extra); + } + // Rewrite HMR update message + else if ((m = viteHmrUpdateMsg.exec(stripped))) { + if (isAstroSrcFile(m[1])) return; + astroLogger.info('watch', m[1]); + } + // Fallback + else { + astroLogger.info('vite', msg); + } }, warn(msg) { logger.hasWarned = true; diff --git a/packages/astro/src/vite-plugin-astro/hmr.ts b/packages/astro/src/vite-plugin-astro/hmr.ts index a4bfca966892..6ffb660174fe 100644 --- a/packages/astro/src/vite-plugin-astro/hmr.ts +++ b/packages/astro/src/vite-plugin-astro/hmr.ts @@ -1,4 +1,3 @@ -import { fileURLToPath } from 'node:url'; import type { HmrContext, ModuleNode } from 'vite'; import type { AstroConfig } from '../@types/astro.js'; import { @@ -8,14 +7,9 @@ import { type CompileResult, } from '../core/compile/index.js'; import type { Logger } from '../core/logger/core.js'; +import { isAstroSrcFile } from '../core/logger/vite.js'; import { isAstroScript } from './query.js'; -const PKG_PREFIX = fileURLToPath(new URL('../../', import.meta.url)); -const E2E_PREFIX = fileURLToPath(new URL('../../e2e', import.meta.url)); -const isPkgFile = (id: string | null) => { - return id?.startsWith(PKG_PREFIX) && !id.startsWith(E2E_PREFIX); -}; - export interface HandleHotUpdateOptions { config: AstroConfig; logger: Logger; @@ -45,7 +39,7 @@ export async function handleHotUpdate( } // Skip monorepo files to avoid console spam - if (isPkgFile(ctx.file)) { + if (isAstroSrcFile(ctx.file)) { return; } @@ -55,7 +49,7 @@ export async function handleHotUpdate( const files = new Set(); for (const mod of ctx.modules) { // Skip monorepo files to avoid console spam - if (isPkgFile(mod.id ?? mod.file)) { + if (isAstroSrcFile(mod.id ?? mod.file)) { filtered.delete(mod); continue; } From ec7d8e03d618d58e485b5302e963ac4eaee5614e Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 20 Nov 2023 18:35:51 +0800 Subject: [PATCH 3/7] Suppress Vite build message --- packages/astro/src/core/logger/vite.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/astro/src/core/logger/vite.ts b/packages/astro/src/core/logger/vite.ts index 7fa523c27ae3..fad5bc22b5ca 100644 --- a/packages/astro/src/core/logger/vite.ts +++ b/packages/astro/src/core/logger/vite.ts @@ -13,6 +13,8 @@ export function isAstroSrcFile(id: string | null) { const vitePageReloadMsg = /page reload (.*)( \(.*\))?/; // capture "hmr update some/Component.vue" messages const viteHmrUpdateMsg = /hmr update (.*)/; +// capture "vite v5.0.0 building SSR bundle for production..." and "vite v5.0.0 building for production..." messages +const viteBuildMsg = /vite.*building.*for production/; export function createViteLogger(astroLogger: AstroLogger): ViteLogger { const warnedMessages = new Set(); @@ -35,6 +37,10 @@ export function createViteLogger(astroLogger: AstroLogger): ViteLogger { if (isAstroSrcFile(m[1])) return; astroLogger.info('watch', m[1]); } + // Don't log Vite build messages + else if (viteBuildMsg.test(stripped)) { + // noop + } // Fallback else { astroLogger.info('vite', msg); From e118127022f2190216fd55b52b1f881e5e2eac70 Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 20 Nov 2023 18:38:25 +0800 Subject: [PATCH 4/7] Add changeset --- .changeset/gold-grapes-jump.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gold-grapes-jump.md diff --git a/.changeset/gold-grapes-jump.md b/.changeset/gold-grapes-jump.md new file mode 100644 index 000000000000..d3da8b72e9eb --- /dev/null +++ b/.changeset/gold-grapes-jump.md @@ -0,0 +1,5 @@ +--- +'astro': minor +--- + +Reworks Vite's logger to use Astro's logger to correctly log HMR messages From 065c70b951385a1b7ceb1c8a5851ea0f3d43c056 Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 20 Nov 2023 21:10:33 +0800 Subject: [PATCH 5/7] Suppress Astro errors in Vite logger --- packages/astro/src/core/logger/vite.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/astro/src/core/logger/vite.ts b/packages/astro/src/core/logger/vite.ts index fad5bc22b5ca..23cb607acdf9 100644 --- a/packages/astro/src/core/logger/vite.ts +++ b/packages/astro/src/core/logger/vite.ts @@ -1,6 +1,7 @@ import { fileURLToPath } from 'url'; import stripAnsi from 'strip-ansi'; import type { Logger as ViteLogger, Rollup } from 'vite'; +import { isAstroError } from '../errors/errors.js'; import type { Logger as AstroLogger } from './core.js'; const PKG_PREFIX = fileURLToPath(new URL('../../../', import.meta.url)); @@ -22,7 +23,6 @@ export function createViteLogger(astroLogger: AstroLogger): ViteLogger { const logger: ViteLogger = { hasWarned: false, - info(msg) { const stripped = stripAnsi(msg); let m; @@ -58,10 +58,13 @@ export function createViteLogger(astroLogger: AstroLogger): ViteLogger { }, error(msg, opts) { logger.hasWarned = true; + + const err = opts?.error; + if (err) loggedErrors.add(err); + // Astro errors are already logged by us, skip logging + if (err && isAstroError(err)) return; + astroLogger.error('vite', msg); - if (opts?.error) { - loggedErrors.add(opts.error); - } }, // Don't allow clear screen clearScreen: () => {}, From 7077426b56ab5c4ee03c33af118f7332a51d45de Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 20 Nov 2023 21:28:40 +0800 Subject: [PATCH 6/7] Fix log level threshold check --- packages/astro/src/core/create-vite.ts | 2 +- packages/astro/src/core/logger/core.ts | 6 +++++- packages/astro/src/core/logger/vite.ts | 17 ++++++++++++++--- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index 62b848363c68..44af6208b064 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -108,7 +108,7 @@ export async function createVite( configFile: false, cacheDir: fileURLToPath(new URL('./node_modules/.vite/', settings.config.root)), // using local caches allows Astro to be used in monorepos, etc. clearScreen: false, // we want to control the output, not Vite - customLogger: createViteLogger(logger), + customLogger: createViteLogger(logger, settings.config.vite.logLevel), appType: 'custom', optimizeDeps: { entries: ['src/**/*'], diff --git a/packages/astro/src/core/logger/core.ts b/packages/astro/src/core/logger/core.ts index 5d617a1a2fa2..91dcdc8387c5 100644 --- a/packages/astro/src/core/logger/core.ts +++ b/packages/astro/src/core/logger/core.ts @@ -75,13 +75,17 @@ export function log(opts: LogOptions, level: LoggerLevel, label: string | null, }; // test if this level is enabled or not - if (levels[logLevel] > levels[level]) { + if (!isLogLevelEnabled(logLevel, level)) { return; // do nothing } dest.write(event); } +export function isLogLevelEnabled(configuredLogLevel: LoggerLevel, level: LoggerLevel) { + return levels[configuredLogLevel] <= levels[level]; +} + /** Emit a user-facing message. Useful for UI and other console messages. */ export function info(opts: LogOptions, label: string | null, message: string) { return log(opts, 'info', label, message); diff --git a/packages/astro/src/core/logger/vite.ts b/packages/astro/src/core/logger/vite.ts index 23cb607acdf9..98a774e1e44f 100644 --- a/packages/astro/src/core/logger/vite.ts +++ b/packages/astro/src/core/logger/vite.ts @@ -1,8 +1,8 @@ import { fileURLToPath } from 'url'; import stripAnsi from 'strip-ansi'; -import type { Logger as ViteLogger, Rollup } from 'vite'; +import type { Logger as ViteLogger, Rollup, LogLevel } from 'vite'; import { isAstroError } from '../errors/errors.js'; -import type { Logger as AstroLogger } from './core.js'; +import { isLogLevelEnabled, type Logger as AstroLogger } from './core.js'; const PKG_PREFIX = fileURLToPath(new URL('../../../', import.meta.url)); const E2E_PREFIX = fileURLToPath(new URL('../../../e2e', import.meta.url)); @@ -17,13 +17,18 @@ const viteHmrUpdateMsg = /hmr update (.*)/; // capture "vite v5.0.0 building SSR bundle for production..." and "vite v5.0.0 building for production..." messages const viteBuildMsg = /vite.*building.*for production/; -export function createViteLogger(astroLogger: AstroLogger): ViteLogger { +export function createViteLogger( + astroLogger: AstroLogger, + viteLogLevel: LogLevel = 'info' +): ViteLogger { const warnedMessages = new Set(); const loggedErrors = new WeakSet(); const logger: ViteLogger = { hasWarned: false, info(msg) { + if (!isLogLevelEnabled(viteLogLevel, 'info')) return; + const stripped = stripAnsi(msg); let m; // Rewrite HMR page reload message @@ -47,16 +52,22 @@ export function createViteLogger(astroLogger: AstroLogger): ViteLogger { } }, warn(msg) { + if (!isLogLevelEnabled(viteLogLevel, 'warn')) return; + logger.hasWarned = true; astroLogger.warn('vite', msg); }, warnOnce(msg) { + if (!isLogLevelEnabled(viteLogLevel, 'warn')) return; + if (warnedMessages.has(msg)) return; logger.hasWarned = true; astroLogger.warn('vite', msg); warnedMessages.add(msg); }, error(msg, opts) { + if (!isLogLevelEnabled(viteLogLevel, 'error')) return; + logger.hasWarned = true; const err = opts?.error; From a8751f6c17273937ecab8222a57a15471c07574f Mon Sep 17 00:00:00 2001 From: bluwy Date: Thu, 30 Nov 2023 13:08:51 +0800 Subject: [PATCH 7/7] Suppress more errors --- packages/astro/src/core/logger/vite.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/astro/src/core/logger/vite.ts b/packages/astro/src/core/logger/vite.ts index 98a774e1e44f..ac48369a3693 100644 --- a/packages/astro/src/core/logger/vite.ts +++ b/packages/astro/src/core/logger/vite.ts @@ -74,6 +74,15 @@ export function createViteLogger( if (err) loggedErrors.add(err); // Astro errors are already logged by us, skip logging if (err && isAstroError(err)) return; + // SSR module and pre-transform errors are always handled by us, + // send to debug logs + if ( + msg.includes('Error when evaluating SSR module') || + msg.includes('Pre-transform error:') + ) { + astroLogger.debug('vite', msg); + return; + } astroLogger.error('vite', msg); },