Logging rewrite 2 (#9105)

* update logging add back warning for header access improve labels and formatting improve error logging remove outdated error fix build new error messages and hints new error messages and hints * walk back error message changes * fix rebase issues * add changeset * Remove accidental log * revert bad env change * Update index.ts * Update packages/astro/src/core/build/index.ts Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com> --------- Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com>
withastro · Nov 18, 2023 · 6201bbe · 6201bbe
1 parent 1c48ed2
commit 6201bbe
Show file tree

Hide file tree

Showing 47 changed files with 395 additions and 451 deletions.
diff --git a/.changeset/calm-baboons-watch.md b/.changeset/calm-baboons-watch.md
@@ -0,0 +1,5 @@
+---
+'astro': minor
+---
+
+Update CLI logging experience
diff --git a/.changeset/modern-candles-sip.md b/.changeset/modern-candles-sip.md
@@ -0,0 +1,5 @@
+---
+'create-astro': patch
+---
+
+Stop clearing the console on start
diff --git a/packages/astro/src/assets/build/generate.ts b/packages/astro/src/assets/build/generate.ts
@@ -58,7 +58,7 @@ export async function prepareAssetsGenerationEnv(
 		await fs.promises.mkdir(assetsCacheDir, { recursive: true });
 	} catch (err) {
 		logger.warn(
-			'astro:assets',
+			null,
 			`An error was encountered while creating the cache directory. Proceeding without caching. Error: ${err}`
 		);
 		useCache = false;
@@ -231,7 +231,7 @@ export async function generateImagesForPath(
 			}
 		} catch (e) {
 			env.logger.warn(
-				'astro:assets',
+				null,
 				`An error was encountered while creating the cache directory. Proceeding without caching. Error: ${e}`
 			);
 		} finally {

diff --git a/packages/astro/src/cli/install-package.ts b/packages/astro/src/cli/install-package.ts
@@ -26,7 +26,7 @@ export async function getPackage<T>(
 		packageImport = await import(packageName);
 	} catch (e) {
 		logger.info(
-			'',
+			null,
 			`To continue, Astro requires the following dependency to be installed: ${bold(packageName)}.`
 		);
 		const result = await installPackage([packageName, ...otherDeps], options, logger);

diff --git a/packages/astro/src/cli/telemetry/index.ts b/packages/astro/src/cli/telemetry/index.ts
@@ -1,23 +1,23 @@
 /* eslint-disable no-console */
-import whichPm from 'which-pm';
 import type yargs from 'yargs-parser';
 import * as msg from '../../core/messages.js';
 import { telemetry } from '../../events/index.js';
+import { createLoggerFromFlags } from '../flags.js';
 
 interface TelemetryOptions {
 	flags: yargs.Arguments;
 }
 
 export async function notify() {
-	const packageManager = (await whichPm(process.cwd()))?.name ?? 'npm';
 	await telemetry.notify(() => {
-		console.log(msg.telemetryNotice(packageManager) + '\n');
+		console.log(msg.telemetryNotice() + '\n');
 		return true;
 	});
 }
 
 export async function update(subcommand: string, { flags }: TelemetryOptions) {
 	const isValid = ['enable', 'disable', 'reset'].includes(subcommand);
+	const logger = createLoggerFromFlags(flags);
 
 	if (flags.help || flags.h || !isValid) {
 		msg.printHelp({
@@ -37,17 +37,17 @@ export async function update(subcommand: string, { flags }: TelemetryOptions) {
 	switch (subcommand) {
 		case 'enable': {
 			telemetry.setEnabled(true);
-			console.log(msg.telemetryEnabled());
+			logger.info('SKIP_FORMAT', msg.telemetryEnabled());
 			return;
 		}
 		case 'disable': {
 			telemetry.setEnabled(false);
-			console.log(msg.telemetryDisabled());
+			logger.info('SKIP_FORMAT', msg.telemetryDisabled());
 			return;
 		}
 		case 'reset': {
 			telemetry.clear();
-			console.log(msg.telemetryReset());
+			logger.info('SKIP_FORMAT', msg.telemetryReset());
 			return;
 		}
 	}

diff --git a/packages/astro/src/content/server-listeners.ts b/packages/astro/src/content/server-listeners.ts
@@ -1,4 +1,4 @@
-import { bold, cyan } from 'kleur/colors';
+import { bold, cyan, underline } from 'kleur/colors';
 import type fsMod from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
@@ -26,7 +26,7 @@ export async function attachContentServerListeners({
 	const contentPaths = getContentPaths(settings.config, fs);
 
 	if (fs.existsSync(contentPaths.contentDir)) {
-		logger.info(
+		logger.debug(
 			'content',
 			`Watching ${cyan(
 				contentPaths.contentDir.href.replace(settings.config.root.href, '')
@@ -39,7 +39,7 @@ export async function attachContentServerListeners({
 		viteServer.watcher.on('addDir', contentDirListener);
 		async function contentDirListener(dir: string) {
 			if (appendForwardSlash(pathToFileURL(dir).href) === contentPaths.contentDir.href) {
-				logger.info('content', `Content dir found. Watching for changes`);
+				logger.debug('content', `Content directory found. Watching for changes`);
 				await attachListeners();
 				viteServer.watcher.removeListener('addDir', contentDirListener);
 			}
@@ -55,7 +55,7 @@ export async function attachContentServerListeners({
 			contentConfigObserver: globalContentConfigObserver,
 		});
 		await contentGenerator.init();
-		logger.info('content', 'Types generated');
+		logger.debug('content', 'Types generated');
 
 		viteServer.watcher.on('add', (entry) => {
 			contentGenerator.queueEvent({ name: 'add', entry });
@@ -90,9 +90,9 @@ function warnAllowJsIsFalse({
 			'true'
 		)} in your ${bold(tsConfigFileName)} file to have autocompletion in your ${bold(
 			contentConfigFileName
-		)} file.
-See ${bold('https://www.typescriptlang.org/tsconfig#allowJs')} for more information.
-			`
+		)} file. See ${underline(
+			cyan('https://www.typescriptlang.org/tsconfig#allowJs')
+		)} for more information.`
 	);
 }
 

diff --git a/packages/astro/src/content/types-generator.ts b/packages/astro/src/content/types-generator.ts
@@ -1,5 +1,5 @@
 import glob from 'fast-glob';
-import { cyan } from 'kleur/colors';
+import { bold, cyan } from 'kleur/colors';
 import type fsMod from 'node:fs';
 import * as path from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
@@ -56,13 +56,6 @@ type CreateContentGeneratorParams = {
 	fs: typeof fsMod;
 };
 
-type EventOpts = { logLevel: 'info' | 'warn' };
-
-type EventWithOptions = {
-	type: ContentEvent;
-	opts: EventOpts | undefined;
-};
-
 class UnsupportedFileTypeError extends Error {}
 
 export async function createContentTypesGenerator({
@@ -78,7 +71,7 @@ export async function createContentTypesGenerator({
 	const contentEntryExts = [...contentEntryConfigByExt.keys()];
 	const dataEntryExts = getDataEntryExts(settings);
 
-	let events: EventWithOptions[] = [];
+	let events: ContentEvent[] = [];
 	let debounceTimeout: NodeJS.Timeout | undefined;
 
 	const typeTemplateContent = await fs.promises.readFile(contentPaths.typesTemplate, 'utf-8');
@@ -90,10 +83,7 @@ export async function createContentTypesGenerator({
 			return { typesGenerated: false, reason: 'no-content-dir' };
 		}
 
-		events.push({
-			type: { name: 'add', entry: contentPaths.config.url },
-			opts: { logLevel: 'warn' },
-		});
+		events.push({ name: 'add', entry: contentPaths.config.url });
 
 		const globResult = await glob('**', {
 			cwd: fileURLToPath(contentPaths.contentDir),
@@ -110,24 +100,18 @@ export async function createContentTypesGenerator({
 			const entryURL = pathToFileURL(fullPath);
 			if (entryURL.href.startsWith(contentPaths.config.url.href)) continue;
 			if (entry.dirent.isFile()) {
-				events.push({
-					type: { name: 'add', entry: entryURL },
-					opts: { logLevel: 'warn' },
-				});
+				events.push({ name: 'add', entry: entryURL });
 			} else if (entry.dirent.isDirectory()) {
-				events.push({ type: { name: 'addDir', entry: entryURL }, opts: { logLevel: 'warn' } });
+				events.push({ name: 'addDir', entry: entryURL });
 			}
 		}
 		await runEvents();
 		return { typesGenerated: true };
 	}
 
 	async function handleEvent(
-		event: ContentEvent,
-		opts?: EventOpts
+		event: ContentEvent
 	): Promise<{ shouldGenerateTypes: boolean; error?: Error }> {
-		const logLevel = opts?.logLevel ?? 'info';
-
 		if (event.name === 'addDir' || event.name === 'unlinkDir') {
 			const collection = normalizePath(
 				path.relative(fileURLToPath(contentPaths.contentDir), fileURLToPath(event.entry))
@@ -140,9 +124,7 @@ export async function createContentTypesGenerator({
 			switch (event.name) {
 				case 'addDir':
 					collectionEntryMap[JSON.stringify(collection)] = { type: 'unknown', entries: {} };
-					if (logLevel === 'info') {
-						logger.info('content', `${cyan(collection)} collection added`);
-					}
+					logger.debug('content', `${cyan(collection)} collection added`);
 					break;
 				case 'unlinkDir':
 					if (collectionKey in collectionEntryMap) {
@@ -186,16 +168,14 @@ export async function createContentTypesGenerator({
 
 		const collection = getEntryCollectionName({ entry, contentDir });
 		if (collection === undefined) {
-			if (['info', 'warn'].includes(logLevel)) {
-				logger.warn(
-					'content',
-					`${cyan(
-						normalizePath(
-							path.relative(fileURLToPath(contentPaths.contentDir), fileURLToPath(event.entry))
-						)
-					)} must be nested in a collection directory. Skipping.`
-				);
-			}
+			logger.warn(
+				'content',
+				`${bold(
+					normalizePath(
+						path.relative(fileURLToPath(contentPaths.contentDir), fileURLToPath(event.entry))
+					)
+				)} must live in a ${bold('content/...')} collection subdirectory.`
+			);
 			return { shouldGenerateTypes: false };
 		}
 
@@ -308,22 +288,19 @@ export async function createContentTypesGenerator({
 		}
 	}
 
-	function queueEvent(rawEvent: RawContentEvent, opts?: EventOpts) {
+	function queueEvent(rawEvent: RawContentEvent) {
 		const event = {
-			type: {
-				entry: pathToFileURL(rawEvent.entry),
-				name: rawEvent.name,
-			},
-			opts,
+			entry: pathToFileURL(rawEvent.entry),
+			name: rawEvent.name,
 		};
-		if (!event.type.entry.pathname.startsWith(contentPaths.contentDir.pathname)) return;
+		if (!event.entry.pathname.startsWith(contentPaths.contentDir.pathname)) return;
 
 		events.push(event);
 
 		debounceTimeout && clearTimeout(debounceTimeout);
 		const runEventsSafe = async () => {
 			try {
-				await runEvents(opts);
+				await runEvents();
 			} catch {
 				// Prevent frontmatter errors from crashing the server. The errors
 				// are still reported on page reflects as desired.
@@ -333,30 +310,25 @@ export async function createContentTypesGenerator({
 		debounceTimeout = setTimeout(runEventsSafe, 50 /* debounce to batch chokidar events */);
 	}
 
-	async function runEvents(opts?: EventOpts) {
-		const logLevel = opts?.logLevel ?? 'info';
+	async function runEvents() {
 		const eventResponses = [];
 
 		for (const event of events) {
-			const response = await handleEvent(event.type, event.opts);
+			const response = await handleEvent(event);
 			eventResponses.push(response);
 		}
 
 		events = [];
-		let unsupportedFiles = [];
 		for (const response of eventResponses) {
 			if (response.error instanceof UnsupportedFileTypeError) {
-				unsupportedFiles.push(response.error.message);
+				logger.warn(
+					'content',
+					`Unsupported file type ${bold(
+						response.error.message
+					)} found. Prefix filename with an underscore (\`_\`) to ignore.`
+				);
 			}
 		}
-		if (unsupportedFiles.length > 0 && ['info', 'warn'].includes(logLevel)) {
-			logger.warn(
-				'content',
-				`Unsupported file types found. Prefix with an underscore (\`_\`) to ignore:\n- ${unsupportedFiles.join(
-					'\n'
-				)}`
-			);
-		}
 		const observable = contentConfigObserver.get();
 		if (eventResponses.some((r) => r.shouldGenerateTypes)) {
 			await writeContentFiles({
@@ -369,7 +341,7 @@ export async function createContentTypesGenerator({
 				viteServer,
 			});
 			invalidateVirtualMod(viteServer);
-			if (observable.status === 'loaded' && ['info', 'warn'].includes(logLevel)) {
+			if (observable.status === 'loaded') {
 				warnNonexistentCollections({
 					logger,
 					contentConfig: observable.config,
@@ -475,6 +447,7 @@ async function writeContentFiles({
 	let configPathRelativeToCacheDir = normalizePath(
 		path.relative(contentPaths.cacheDir.pathname, contentPaths.config.url.pathname)
 	);
+
 	if (!isRelativePath(configPathRelativeToCacheDir))
 		configPathRelativeToCacheDir = './' + configPathRelativeToCacheDir;
 
@@ -514,9 +487,9 @@ function warnNonexistentCollections({
 		if (!collectionEntryMap[JSON.stringify(configuredCollection)]) {
 			logger.warn(
 				'content',
-				`The ${JSON.stringify(
-					configuredCollection
-				)} collection does not have an associated folder in your \`content\` directory. Make sure the folder exists, or check your content config for typos.`
+				`The ${bold(configuredCollection)} collection is defined but no ${bold(
+					'content/' + configuredCollection
+				)} folder exists in the content directory. Create a new folder for the collection, or check your content configuration file for typos.`
 			);
 		}
 	}

diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
@@ -190,7 +190,7 @@ export class App {
 			if (err instanceof EndpointNotFoundError) {
 				return this.#renderError(request, { status: 404, response: err.originalResponse });
 			} else {
-				this.#logger.error('ssr', err.stack || err.message || String(err));
+				this.#logger.error(null, err.stack || err.message || String(err));
 				return this.#renderError(request, { status: 500 });
 			}
 		}