feat: Improved errors and warnings (#1758)

* refactor: Better root error handling * refactor: Switch prerender plugin to be async for error gen * refactor: Simplify & improve compilation messages
preactjs · Dec 20, 2022 · fc9eccd · fc9eccd
1 parent c20eb59
commit fc9eccd
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 79 deletions.
diff --git a/packages/cli/src/index.js b/packages/cli/src/index.js
@@ -2,6 +2,7 @@
 const envinfo = require('envinfo');
 const sade = require('sade');
 const notifier = require('update-notifier');
+const { green } = require('kleur');
 const { error } = require('./util');
 const pkg = require('../package.json');
 const { isNodeVersionGreater } = require('./util');
@@ -19,10 +20,6 @@ const commands = require('./commands');
 // installHooks();
 notifier({ pkg }).notify();
 
-process.on('unhandledRejection', err => {
-	error(err.stack || err.message);
-});
-
 const prog = sade('preact').version(pkg.version);
 
 prog
@@ -53,7 +50,7 @@ prog
 	.option('--inlineCss', 'Adds critical CSS to the prerendered HTML', true)
 	.option('-c, --config', 'Path to custom CLI config', 'preact.config.js')
 	.option('-v, --verbose', 'Verbose output', false)
-	.action(commands.build);
+	.action((src, argv) => exec(commands.build(src, argv)));
 
 prog
 	.command('watch [src]')
@@ -81,13 +78,13 @@ prog
 	.option('-c, --config', 'Path to custom CLI config', 'preact.config.js')
 	.option('-H, --host', 'Set server hostname', '0.0.0.0')
 	.option('-p, --port', 'Set server port (default 8080)')
-	.action(commands.watch);
+	.action((src, argv) => exec(commands.watch(src, argv)));
 
 prog
 	.command('info')
 	.describe('Print out debugging information about the local environment')
-	.action(() => {
-		envinfo
+	.action(() =>
+		exec(envinfo
 			.run({
 				System: ['OS', 'CPU'],
 				Binaries: ['Node', 'Yarn', 'npm'],
@@ -100,8 +97,8 @@ prog
 				],
 				npmGlobalPackages: ['preact-cli'],
 			})
-			.then(info => process.stdout.write(`\nEnvironment Info:${info}\n`));
-	});
+			.then(info => process.stdout.write(`\nEnvironment Info:${info}\n`))
+	));
 
 prog.parse(process.argv, {
 	alias: {
@@ -114,3 +111,20 @@ prog.parse(process.argv, {
 		);
 	},
 });
+
+function exec(cmd) {
+	cmd.catch(catchExceptions);
+}
+
+/**
+ * @param {Error | import('webpack').StatsError} err
+ */
+async function catchExceptions(err) {
+	// Webpack Stats Error
+	if ('moduleName' in err && 'loc' in err) {
+		error(`${err.moduleName} ${green(err.loc)}\n${err.message}\n\n`);
+	}
+	error(err.stack || err.message || err);
+}
+
+process.on('unhandledRejection', catchExceptions);
diff --git a/packages/cli/src/lib/webpack/prerender.js b/packages/cli/src/lib/webpack/prerender.js
@@ -5,7 +5,7 @@ const stackTrace = require('stack-trace');
 const URL = require('url');
 const { SourceMapConsumer } = require('source-map');
 
-module.exports = function (config, params) {
+module.exports = async function (config, params) {
 	params = params || {};
 
 	let entry = resolve(config.dest, './ssr-build/ssr-bundle.js');
@@ -36,7 +36,7 @@ module.exports = function (config, params) {
 			throw err;
 		}
 
-		handlePrerenderError(err, config, stack, entry);
+		await handlePrerenderError(err, config, stack, entry);
 	}
 };
 

diff --git a/packages/cli/src/lib/webpack/render-html-plugin.js b/packages/cli/src/lib/webpack/render-html-plugin.js
@@ -73,7 +73,7 @@ module.exports = async function renderHTMLPlugin(config, env) {
 			title,
 			filename,
 			template: `!!${require.resolve('ejs-loader')}?esModule=false!${template}`,
-			templateParameters: (compilation, assets, assetTags, options) => {
+			templateParameters: async (compilation, assets, assetTags, options) => {
 				let entrypoints = {};
 				compilation.entrypoints.forEach((entrypoint, name) => {
 					let entryFiles = entrypoint.getFiles();
@@ -92,7 +92,9 @@ module.exports = async function renderHTMLPlugin(config, env) {
 						env,
 						prerenderData: values,
 						CLI_DATA: { prerenderData: { url, ...routeData } },
-						ssr: config.prerender ? prerender(config, values) : '',
+						ssr: config.prerender
+							? await prerender(config, values)
+							: '',
 						entrypoints,
 					},
 					htmlWebpackPlugin: {

diff --git a/packages/cli/src/lib/webpack/run-webpack.js b/packages/cli/src/lib/webpack/run-webpack.js
@@ -2,12 +2,12 @@ const ip = require('ip');
 const webpack = require('webpack');
 const { resolve } = require('path');
 const clear = require('console-clear');
-const { bold, red, green, magenta } = require('kleur');
+const { bold, green, magenta } = require('kleur');
 const DevServer = require('webpack-dev-server');
 const clientConfig = require('./webpack-client-config');
 const serverConfig = require('./webpack-server-config');
 const transformConfig = require('./transform-config');
-const { error, isDir, warn } = require('../../util');
+const { isDir, warn } = require('../../util');
 
 /**
  * @param {import('../../../types').Env} env
@@ -34,16 +34,12 @@ async function devBuild(config, env) {
 				devServer.port
 			)}`;
 
-			if (stats.hasErrors()) {
-				process.stdout.write(red('Build failed!\n\n'));
-			} else {
+			if (!stats.hasErrors()) {
 				process.stdout.write(green('Compiled successfully!\n\n'));
 				process.stdout.write('You can view the application in browser.\n\n');
 				process.stdout.write(`${bold('Local:')}            ${serverAddr}\n`);
 				process.stdout.write(`${bold('On Your Network:')}  ${localIpAddr}\n`);
 			}
-
-			showStats(stats, false);
 		});
 
 		compiler.hooks.failed.tap('CliDevPlugin', rej);
@@ -70,100 +66,81 @@ async function prodBuild(config, env) {
 	const clientWebpackConfig = await clientConfig(config, env);
 	await transformConfig(clientWebpackConfig, config, env);
 	const clientCompiler = webpack(clientWebpackConfig);
+	await runCompiler(clientCompiler);
 
-	try {
-		let stats = await runCompiler(clientCompiler);
-
-		// Timeout for plugins that work on `after-emit` event of webpack
-		await new Promise(r => setTimeout(r, 20));
-
-		return showStats(stats, true);
-	} catch (err) {
-		// eslint-disable-next-line
-		console.log(err);
-	}
+	// Timeout for plugins that work on `after-emit` event of webpack
+	await new Promise(r => setTimeout(r, 20));
 }
 
+/**
+ * @param {import('webpack').Compiler} compiler
+ */
 function runCompiler(compiler) {
-	return new Promise(res => {
+	return new Promise((res, rej) => {
 		compiler.run((err, stats) => {
-			if (err) {
-				error(err, 1);
-			}
+			if (err) rej(err);
 
-			showStats(stats, true);
+			showCompilationIssues(stats);
 
-			res(stats);
+			compiler.close(closeErr => {
+				if (closeErr) rej(closeErr);
+				res();
+			});
 		});
 	});
 }
 
-function showStats(stats, isProd) {
+/**
+ * @param {import('webpack').Stats} stats
+ */
+function showCompilationIssues(stats) {
 	if (stats) {
-		if (stats.hasErrors()) {
-			allFields(stats, 'errors')
-				.map(stripLoaderPrefix)
-				.forEach(({ message }) => error(message, isProd ? 1 : 0));
+		if (stats.hasWarnings()) {
+			allFields(stats, 'warnings').forEach(({ message }) => warn(message));
 		}
 
-		if (stats.hasWarnings()) {
-			allFields(stats, 'warnings')
-				.map(stripLoaderPrefix)
-				.forEach(({ message }) => warn(message));
+		if (stats.hasErrors()) {
+			allFields(stats, 'errors').forEach(err => {
+				throw err;
+			});
 		}
 	}
-
-	return stats;
 }
 
+/**
+ * Recursively retrieve all errors or warnings from compilation
+ *
+ * @param {import('webpack').Stats} stats
+ * @param {'warnings' | 'errors'} field
+ * @returns {import('webpack').StatsError[]}
+ */
 function allFields(stats, field, fields = [], name = null) {
 	const info = stats.toJson({
 		errors: true,
 		warnings: true,
 		errorDetails: false,
 	});
+
 	const addCompilerPrefix = msg =>
 		name ? bold(magenta(name + ': ')) + msg : msg;
-	if (field === 'errors' && stats.hasErrors()) {
+
+	if (field === 'errors') {
 		fields = fields.concat(info.errors.map(addCompilerPrefix));
-	}
-	if (field === 'warnings' && stats.hasWarnings()) {
+	} else {
 		fields = fields.concat(info.warnings.map(addCompilerPrefix));
 	}
+
 	if (stats.compilation.children) {
 		stats.compilation.children.forEach((child, index) => {
 			const name = child.name || `Child Compiler ${index + 1}`;
 			const stats = child.getStats();
-			fields = allFields(stats, field, fields, name);
+			if (field === 'errors' ? stats.hasErrors() : stats.hasWarnings()) {
+				fields = allFields(stats, field, fields, name);
+			}
 		});
 	}
-	return fields;
-}
 
-/** Removes all loaders from any resource identifiers found in a string */
-function stripLoaderPrefix(str) {
-	if (typeof str === 'string') {
-		str = str.replace(
-			/(?:(\()|(^|\b|@))(\.\/~|\.{0,2}\/(?:[^\s]+\/)?node_modules)\/\w+-loader(\/[^?!]+)?(\?\?[\w_.-]+|\?({[\s\S]*?})?)?!/g,
-			'$1'
-		);
-		str = str.replace(/(\.?\.?(?:\/[^/ ]+)+)\s+\(\1\)/g, '$1');
-		str = replaceAll(str, process.cwd(), '.');
-		return str;
-	}
-	return str;
-}
-
-// https://gist.github.com/developit/1a40a6fee65361d1182aaa22ab8c334c
-function replaceAll(str, find, replace) {
-	let s = '',
-		index,
-		next;
-	while (~(next = str.indexOf(find, index))) {
-		s += str.substring(index, next) + replace;
-		index = next + find.length;
-	}
-	return s + str.substring(index);
+	return fields;
 }
 
 /**