From 2b46e583a505726dcc4be1ee16ebdee652fe796b Mon Sep 17 00:00:00 2001
From: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Date: Fri, 18 Nov 2022 01:35:02 -0600
Subject: [PATCH] refactor: Separate internal concepts of config and env
 (#1734)

* refactor: Separate internal concepts of config and env

* docs: Adding changeset
---
 .changeset/great-dryers-cross.md              |  9 +++
 packages/cli/src/commands/build.js            |  2 +-
 packages/cli/src/commands/watch.js            |  2 +-
 packages/cli/src/lib/babel-config.js          |  7 +-
 packages/cli/src/lib/webpack/prerender.js     | 15 ++--
 .../cli/src/lib/webpack/render-html-plugin.js |  9 ++-
 packages/cli/src/lib/webpack/run-webpack.js   | 76 +++++++++++--------
 .../cli/src/lib/webpack/transform-config.js   | 27 ++++---
 .../src/lib/webpack/webpack-base-config.js    | 19 +++--
 .../src/lib/webpack/webpack-client-config.js  | 59 +++++++-------
 .../src/lib/webpack/webpack-server-config.js  | 13 ++--
 .../subjects/custom-template/template.html    |  2 +-
 12 files changed, 141 insertions(+), 99 deletions(-)
 create mode 100644 .changeset/great-dryers-cross.md

diff --git a/.changeset/great-dryers-cross.md b/.changeset/great-dryers-cross.md
new file mode 100644
index 000000000..aeda7e193
--- /dev/null
+++ b/.changeset/great-dryers-cross.md
@@ -0,0 +1,9 @@
+---
+'preact-cli': major
+---
+
+Reduces the `env` parameter of `preact.config.js` to only contain 3 values: `isProd`, `isWatch`, and `isServer`.
+
+Previously, `env` contained many semi-duplicated values (`production` and `isProd`, etc) as well as values that were unlikely to be of much use to many users (what flags were set, for instance). Because of this, the signal-to-noise ratio was rather low which we didn't like. As such, we reduced `env` down to the most basic environment info: what type of build is `preact-cli` doing and for which environement?
+
+If you customize your Webpack config using a `preact.config.js`, please be aware that you may need to update which values you consume from `env`.
diff --git a/packages/cli/src/commands/build.js b/packages/cli/src/commands/build.js
index a2b11b037..f1d2d3c87 100644
--- a/packages/cli/src/commands/build.js
+++ b/packages/cli/src/commands/build.js
@@ -22,7 +22,7 @@ exports.build = async function buildCommand(src, argv) {
 		await promisify(rimraf)(dest);
 	}
 
-	let stats = await runWebpack(argv, false);
+	let stats = await runWebpack(argv, true);
 
 	if (argv.json) {
 		await runWebpack.writeJsonStats(cwd, stats);
diff --git a/packages/cli/src/commands/watch.js b/packages/cli/src/commands/watch.js
index fcc06dcdd..98bcdd1fe 100644
--- a/packages/cli/src/commands/watch.js
+++ b/packages/cli/src/commands/watch.js
@@ -37,7 +37,7 @@ exports.watch = async function watchCommand(src, argv) {
 		}
 	}
 
-	return runWebpack(argv, true);
+	return runWebpack(argv, false);
 };
 
 const determinePort = (exports.determinePort = async function (port) {
diff --git a/packages/cli/src/lib/babel-config.js b/packages/cli/src/lib/babel-config.js
index 5a4859787..c420a8da2 100644
--- a/packages/cli/src/lib/babel-config.js
+++ b/packages/cli/src/lib/babel-config.js
@@ -1,7 +1,10 @@
 const { tryResolveConfig } = require('../util');
 
-module.exports = function (env) {
-	const { babelConfig, cwd, isProd, refresh } = env;
+/**
+ * @param {boolean} isProd
+ */
+module.exports = function (config, isProd) {
+	const { babelConfig, cwd, refresh } = config;
 
 	const resolvedConfig =
 		babelConfig &&
diff --git a/packages/cli/src/lib/webpack/prerender.js b/packages/cli/src/lib/webpack/prerender.js
index 708d71926..a75e072ad 100644
--- a/packages/cli/src/lib/webpack/prerender.js
+++ b/packages/cli/src/lib/webpack/prerender.js
@@ -5,10 +5,10 @@ const stackTrace = require('stack-trace');
 const URL = require('url');
 const { SourceMapConsumer } = require('source-map');
 
-module.exports = function (env, params) {
+module.exports = function (config, params) {
 	params = params || {};
 
-	let entry = resolve(env.dest, './ssr-build/ssr-bundle.js');
+	let entry = resolve(config.dest, './ssr-build/ssr-bundle.js');
 	let url = params.url || '/';
 
 	global.history = {};
@@ -25,10 +25,9 @@ module.exports = function (env, params) {
 			);
 			return '';
 		}
-		const { cwd } = env;
-		const preact = require(require.resolve('preact', { paths: [cwd] }));
+		const preact = require(require.resolve('preact', { paths: [config.cwd] }));
 		const renderToString = require(require.resolve('preact-render-to-string', {
-			paths: [cwd],
+			paths: [config.cwd],
 		}));
 		return renderToString(preact.h(app, { ...params, url }));
 	} catch (err) {
@@ -37,11 +36,11 @@ module.exports = function (env, params) {
 			throw err;
 		}
 
-		handlePrerenderError(err, env, stack, entry);
+		handlePrerenderError(err, config, stack, entry);
 	}
 };
 
-async function handlePrerenderError(err, env, stack, entry) {
+async function handlePrerenderError(err, config, stack, entry) {
 	let errorMessage = err.toString();
 	let isReferenceError = errorMessage.startsWith('ReferenceError');
 	let methodName = stack.getMethodName();
@@ -70,7 +69,7 @@ async function handlePrerenderError(err, env, stack, entry) {
 						.replace(/^(.*?\/node_modules\/(@[^/]+\/)?[^/]+)(\/.*)$/, '$1')
 				);
 
-			sourcePath = resolve(env.src, position.source);
+			sourcePath = resolve(config.cwd, position.source);
 			sourceLines;
 			try {
 				sourceLines = readFileSync(sourcePath, 'utf-8').split('\n');
diff --git a/packages/cli/src/lib/webpack/render-html-plugin.js b/packages/cli/src/lib/webpack/render-html-plugin.js
index d6d4c2375..4f141ce23 100644
--- a/packages/cli/src/lib/webpack/render-html-plugin.js
+++ b/packages/cli/src/lib/webpack/render-html-plugin.js
@@ -17,8 +17,12 @@ function read(path) {
 	return readFileSync(resolve(__dirname, path), 'utf-8');
 }
 
-module.exports = async function renderHTMLPlugin(config) {
+/**
+ * @param {import('../../../types').Env} env
+ */
+module.exports = async function renderHTMLPlugin(config, env) {
 	const { cwd, dest, src } = config;
+
 	const inProjectTemplatePath = resolve(src, 'template.html');
 	let template = defaultTemplate;
 	if (existsSync(inProjectTemplatePath)) {
@@ -85,9 +89,10 @@ module.exports = async function renderHTMLPlugin(config) {
 						manifest: config.manifest,
 						inlineCss: config['inline-css'],
 						config,
+						env,
 						prerenderData: values,
 						CLI_DATA: { prerenderData: { url, ...routeData } },
-						ssr: config.prerender ? prerender({ cwd, dest, src }, values) : '',
+						ssr: config.prerender ? 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
index ad1e74caa..6354c2d59 100644
--- a/packages/cli/src/lib/webpack/run-webpack.js
+++ b/packages/cli/src/lib/webpack/run-webpack.js
@@ -10,25 +10,30 @@ const serverConfig = require('./webpack-server-config');
 const transformConfig = require('./transform-config');
 const { error, isDir, warn } = require('../../util');
 
-async function devBuild(env) {
-	let config = await clientConfig(env);
+/**
+ * @param {import('../../../types').Env} env
+ */
+async function devBuild(config, env) {
+	const webpackConfig = await clientConfig(config, env);
 
-	await transformConfig(env, config);
+	await transformConfig(webpackConfig, config, env);
 
-	let compiler = webpack(config);
+	let compiler = webpack(webpackConfig);
 	return new Promise((res, rej) => {
 		compiler.hooks.beforeCompile.tap('CliDevPlugin', () => {
-			if (env['clear']) clear(true);
+			if (config['clear']) clear(true);
 		});
 
 		compiler.hooks.done.tap('CliDevPlugin', stats => {
-			let devServer = config.devServer;
-			let protocol = devServer.https ? 'https' : 'http';
+			const devServer = webpackConfig.devServer;
+			const protocol = devServer.https ? 'https' : 'http';
 
-			let serverAddr = `${protocol}://${devServer.host}:${bold(
+			const serverAddr = `${protocol}://${devServer.host}:${bold(
+				devServer.port
+			)}`;
+			const localIpAddr = `${protocol}://${ip.address()}:${bold(
 				devServer.port
 			)}`;
-			let localIpAddr = `${protocol}://${ip.address()}:${bold(devServer.port)}`;
 
 			if (stats.hasErrors()) {
 				process.stdout.write(red('Build failed!\n\n'));
@@ -44,26 +49,28 @@ async function devBuild(env) {
 
 		compiler.hooks.failed.tap('CliDevPlugin', rej);
 
-		let server = new DevServer(config.devServer, compiler);
+		let server = new DevServer(webpackConfig.devServer, compiler);
 		server.start();
 		res(server);
 	});
 }
 
-async function prodBuild(env) {
-	env = { ...env, isServer: false, dev: !env.production, ssr: false };
-	let config = await clientConfig(env);
-	await transformConfig(env, config);
+/**
+ * @param {import('../../../types').Env} env
+ */
+async function prodBuild(config, env) {
+	if (config.prerender) {
+		const serverEnv = { ...env, isServer: true };
 
-	if (env.prerender) {
-		const serverEnv = Object.assign({}, env, { isServer: true, ssr: true });
-		let ssrConfig = serverConfig(serverEnv);
-		await transformConfig(serverEnv, ssrConfig);
-		let serverCompiler = webpack(ssrConfig);
+		const serverWebpackConfig = serverConfig(config, serverEnv);
+		await transformConfig(serverWebpackConfig, config, serverEnv);
+		const serverCompiler = webpack(serverWebpackConfig);
 		await runCompiler(serverCompiler);
 	}
 
-	let clientCompiler = webpack(config);
+	const clientWebpackConfig = await clientConfig(config, env);
+	await transformConfig(clientWebpackConfig, config, env);
+	const clientCompiler = webpack(clientWebpackConfig);
 
 	try {
 		let stats = await runCompiler(clientCompiler);
@@ -219,21 +226,26 @@ function stripLoaderFromModuleNames(m) {
 	return m;
 }
 
-module.exports = function (env, watch = false) {
-	env.production = !watch;
-	env.isProd = env.production; // shorthand
-	env.isWatch = !!watch; // use HMR?
-	env.cwd = resolve(env.cwd || process.cwd());
-
-	// env.src='src' via `build` default
-	let src = resolve(env.cwd, env.src);
-	env.src = isDir(src) ? src : env.cwd;
+/**
+ * @param {boolean} isProd
+ */
+module.exports = function (argv, isProd) {
+	const env = {
+		isProd,
+		isWatch: !isProd,
+		isServer: false,
+	};
+	const config = argv;
+	config.cwd = resolve(argv.cwd || process.cwd());
+
+	// config.src='src' via `build` default
+	const src = resolve(config.cwd, argv.src);
+	config.src = isDir(src) ? src : config.cwd;
 
 	// attach sourcing helper
-	env.source = dir => resolve(env.src, dir);
+	config.source = dir => resolve(config.src, dir);
 
-	// determine build-type to run
-	return (watch ? devBuild : prodBuild)(env);
+	return (isProd ? prodBuild : devBuild)(config, env);
 };
 
 module.exports.writeJsonStats = writeJsonStats;
diff --git a/packages/cli/src/lib/webpack/transform-config.js b/packages/cli/src/lib/webpack/transform-config.js
index 9a5b1f26f..d6dd8d26a 100644
--- a/packages/cli/src/lib/webpack/transform-config.js
+++ b/packages/cli/src/lib/webpack/transform-config.js
@@ -6,14 +6,14 @@ const { error, esmImport, tryResolveConfig, warn } = require('../../util');
 const FILE = 'preact.config';
 const EXTENSIONS = ['js', 'json'];
 
-async function findConfig(env) {
+async function findConfig(cwd) {
 	let idx = 0;
 	for (idx; idx < EXTENSIONS.length; idx++) {
-		let config = `${FILE}.${EXTENSIONS[idx]}`;
-		let path = resolve(env.cwd, config);
+		let configFile = `${FILE}.${EXTENSIONS[idx]}`;
+		let path = resolve(cwd, configFile);
 		try {
 			await stat(path);
-			return { configFile: config, isDefault: true };
+			return { configFile, isDefault: true };
 		} catch (e) {}
 	}
 
@@ -90,17 +90,20 @@ function parseConfig(config) {
 	return transformers;
 }
 
-module.exports = async function (env, webpackConfig) {
+/**
+ * @param {import('../../../types').Env} env
+ */
+module.exports = async function (webpackConfig, config, env) {
 	const { configFile, isDefault } =
-		env.config !== 'preact.config.js'
-			? { configFile: env.config, isDefault: false }
-			: await findConfig(env);
+		config.config !== 'preact.config.js'
+			? { configFile: config.config, isDefault: false }
+			: await findConfig(config.cwd);
 
 	const cliConfig = tryResolveConfig(
-		env.cwd,
+		config.cwd,
 		configFile,
 		isDefault,
-		env.verbose
+		config.verbose
 	);
 
 	if (!cliConfig) return;
@@ -111,7 +114,7 @@ module.exports = async function (env, webpackConfig) {
 	} catch (error) {
 		warn(
 			`Failed to load preact-cli config file, using default!\n${
-				env.verbose ? error.stack : error.message
+				config.verbose ? error.stack : error.message
 			}`
 		);
 		return;
@@ -119,7 +122,7 @@ module.exports = async function (env, webpackConfig) {
 
 	const transformers = parseConfig((m && m.default) || m);
 
-	const helpers = new WebpackConfigHelpers(env.cwd);
+	const helpers = new WebpackConfigHelpers(config.cwd);
 	for (let [transformer, options] of transformers) {
 		try {
 			await transformer(webpackConfig, env, helpers, options);
diff --git a/packages/cli/src/lib/webpack/webpack-base-config.js b/packages/cli/src/lib/webpack/webpack-base-config.js
index 500b348b1..ec2347318 100644
--- a/packages/cli/src/lib/webpack/webpack-base-config.js
+++ b/packages/cli/src/lib/webpack/webpack-base-config.js
@@ -44,14 +44,17 @@ function resolveTsconfig(cwd, isProd) {
 }
 
 /**
+ * @param {import('../../../types').Env} env
  * @returns {import('webpack').Configuration}
  */
-module.exports = function createBaseConfig(env) {
-	const { cwd, isProd, src, source } = env;
-	// Apply base-level `env` values
-	env.dest = resolve(cwd, env.dest || 'build');
-	env.manifest = readJson(source('manifest.json')) || {};
-	env.pkg = readJson(resolve(cwd, 'package.json')) || {};
+module.exports = function createBaseConfig(config, env) {
+	const { cwd, src, source } = config;
+	const { isProd, isServer } = env;
+
+	// Apply base-level `config` values
+	config.dest = resolve(cwd, config.dest || 'build');
+	config.manifest = readJson(source('manifest.json')) || {};
+	config.pkg = readJson(resolve(cwd, 'package.json')) || {};
 
 	// use browserslist config environment, config default, or default browsers
 	// default browsers are '> 0.5%, last 2 versions, Firefox ESR, not dead'
@@ -134,7 +137,7 @@ module.exports = function createBaseConfig(env) {
 					use: [
 						{
 							loader: require.resolve('babel-loader'),
-							options: createBabelConfig(env),
+							options: createBabelConfig(config, isProd),
 						},
 						require.resolve('source-map-loader'),
 					],
@@ -285,7 +288,7 @@ module.exports = function createBaseConfig(env) {
 			new RemoveEmptyScriptsPlugin(),
 			new MiniCssExtractPlugin({
 				filename:
-					isProd && !env.isServer ? '[name].[contenthash:5].css' : '[name].css',
+					isProd && !isServer ? '[name].[contenthash:5].css' : '[name].css',
 				chunkFilename: isProd
 					? '[name].chunk.[contenthash:5].css'
 					: '[name].chunk.css',
diff --git a/packages/cli/src/lib/webpack/webpack-client-config.js b/packages/cli/src/lib/webpack/webpack-client-config.js
index 99b4d7b0a..c4061c649 100644
--- a/packages/cli/src/lib/webpack/webpack-client-config.js
+++ b/packages/cli/src/lib/webpack/webpack-client-config.js
@@ -24,14 +24,16 @@ const cleanFilename = name =>
 	);
 
 /**
+ * @param {import('../../../types').Env} env
  * @returns {Promise<import('webpack').Configuration>}
  */
-async function clientConfig(env) {
-	const { source, src } = env;
+async function clientConfig(config, env) {
+	const { source, src } = config;
+	const { isProd } = env;
 	const asyncLoader = require.resolve('@preact/async-loader');
 
 	let swInjectManifest = [];
-	if (env.sw) {
+	if (config.sw) {
 		let swPath = join(__dirname, '..', '..', '..', 'sw', 'sw.js');
 		const userSwPath = join(src, 'sw.js');
 		if (existsSync(userSwPath)) {
@@ -58,7 +60,7 @@ async function clientConfig(env) {
 		// copy any static files
 		existsSync(source('assets')) && { from: 'assets', to: 'assets' },
 		// copy sw-debug
-		!env.isProd && {
+		!isProd && {
 			from: resolve(__dirname, '../../resources/sw-debug.js'),
 			to: 'sw-debug.js',
 		},
@@ -75,9 +77,9 @@ async function clientConfig(env) {
 			'dom-polyfills': resolve(__dirname, './polyfills'),
 		},
 		output: {
-			path: env.dest,
+			path: config.dest,
 			publicPath: '/',
-			filename: env.isProd ? '[name].[chunkhash:5].js' : '[name].js',
+			filename: isProd ? '[name].[chunkhash:5].js' : '[name].js',
 			chunkFilename: '[name].chunk.[chunkhash:5].js',
 		},
 
@@ -119,10 +121,10 @@ async function clientConfig(env) {
 
 		plugins: [
 			new webpack.DefinePlugin({
-				'process.env.ADD_SW': env.sw,
-				'process.env.PRERENDER': env.prerender,
+				'process.env.ADD_SW': config.sw,
+				'process.env.PRERENDER': config.prerender,
 			}),
-			...(await renderHTMLPlugin(env)),
+			...(await renderHTMLPlugin(config, env)),
 			copyPatterns.length !== 0 &&
 				new CopyWebpackPlugin({
 					patterns: copyPatterns,
@@ -135,7 +137,7 @@ async function clientConfig(env) {
 /**
  * @returns {import('webpack').Configuration}
  */
-function isProd(env) {
+function prodBuild(config) {
 	let limit = 200 * 1000; // 200kb
 	const prodConfig = {
 		performance: Object.assign(
@@ -144,7 +146,7 @@ function isProd(env) {
 				maxAssetSize: limit,
 				maxEntrypointSize: limit,
 			},
-			env.pkg.performance
+			config.pkg.performance
 		),
 
 		plugins: [
@@ -189,7 +191,7 @@ function isProd(env) {
 		},
 	};
 
-	if (env['inline-css']) {
+	if (config['inline-css']) {
 		prodConfig.plugins.push(
 			new CrittersPlugin({
 				preload: 'media',
@@ -200,11 +202,11 @@ function isProd(env) {
 		);
 	}
 
-	if (env.analyze) {
+	if (config.analyze) {
 		prodConfig.plugins.push(new BundleAnalyzerPlugin());
 	}
 
-	if (env.brotli) {
+	if (config.brotli) {
 		prodConfig.plugins.push(
 			new CompressionPlugin({
 				filename: '[path][base].br[query]',
@@ -257,22 +259,22 @@ function setupProxy(target) {
 /**
  * @returns {import('webpack').Configuration}
  */
-function isDev(env) {
-	const { cwd, src } = env;
+function devBuild(config) {
+	const { cwd, src } = config;
 
 	return {
 		infrastructureLogging: {
 			level: 'info',
 		},
-		plugins: [env.refresh && new RefreshPlugin()].filter(Boolean),
+		plugins: [config.refresh && new RefreshPlugin()].filter(Boolean),
 
 		optimization: {
 			moduleIds: 'named',
 		},
 
 		devServer: {
-			hot: env.refresh,
-			liveReload: !env.refresh,
+			hot: config.refresh,
+			liveReload: !config.refresh,
 			compress: true,
 			devMiddleware: {
 				publicPath: '/',
@@ -284,24 +286,27 @@ function isDev(env) {
 					ignored: [resolve(cwd, 'build'), resolve(cwd, 'node_modules')],
 				},
 			},
-			https: env.https,
-			port: env.port,
-			host: env.host,
+			https: config.https,
+			port: config.port,
+			host: config.host,
 			allowedHosts: 'all',
 			historyApiFallback: true,
 			client: {
 				logging: 'none',
 				overlay: false,
 			},
-			proxy: setupProxy(env.pkg.proxy),
+			proxy: setupProxy(config.pkg.proxy),
 		},
 	};
 }
 
-module.exports = async function createClientConfig(env) {
+/**
+ * @param {import('../../../types').Env} env
+ */
+module.exports = async function createClientConfig(config, env) {
 	return merge(
-		baseConfig(env),
-		await clientConfig(env),
-		(env.isProd ? isProd : isDev)(env)
+		baseConfig(config, env),
+		await clientConfig(config, env),
+		(env.isProd ? prodBuild : devBuild)(config)
 	);
 };
diff --git a/packages/cli/src/lib/webpack/webpack-server-config.js b/packages/cli/src/lib/webpack/webpack-server-config.js
index bc3dd80b2..9e52f6a85 100644
--- a/packages/cli/src/lib/webpack/webpack-server-config.js
+++ b/packages/cli/src/lib/webpack/webpack-server-config.js
@@ -5,15 +5,15 @@ const baseConfig = require('./webpack-base-config');
 /**
  * @returns {import('webpack').Configuration}
  */
-function serverConfig(env) {
+function serverConfig(config) {
 	return {
 		entry: {
-			'ssr-bundle': env.source('index'),
+			'ssr-bundle': config.source('index'),
 		},
 		output: {
 			publicPath: '/',
 			filename: 'ssr-bundle.js',
-			path: resolve(env.dest, 'ssr-build'),
+			path: resolve(config.dest, 'ssr-build'),
 			libraryTarget: 'commonjs2',
 		},
 		externals: {
@@ -31,6 +31,9 @@ function serverConfig(env) {
 	};
 }
 
-module.exports = function createServerConfig(env) {
-	return merge(baseConfig(env), serverConfig(env));
+/**
+ * @param {import('../../../types').Env} env
+ */
+module.exports = function createServerConfig(config, env) {
+	return merge(baseConfig(config, env), serverConfig(config));
 };
diff --git a/packages/cli/tests/subjects/custom-template/template.html b/packages/cli/tests/subjects/custom-template/template.html
index 1e611a086..8ad6c331f 100644
--- a/packages/cli/tests/subjects/custom-template/template.html
+++ b/packages/cli/tests/subjects/custom-template/template.html
@@ -3,7 +3,7 @@
 	<head>
 		<meta charset="utf-8">
 		<title><% preact.title %></title>
-		<% if (cli.config.isProd) { %>
+		<% if (cli.env.isProd) { %>
 			<meta name="example-meta" content="Hello Prod">
 		<% } else { %>
 			<meta name="example-meta" content="Hello Dev">