feat: allow better control over minification, default to off

BREAKING-CHANGE: This changes the default behavior of minification to
always off. It also drops the --no-minification argument in favor of a
`--minify <value>` option.

Closes: trunk-rs#774

Trunk.toml

@@ -22,8 +22,8 @@ offline = false
 frozen = false
 # Require Cargo.lock is up to date
 locked = false
-# Allow disabling minification
-no_minification = false
+# Control minification
+minify = "never" # can be one of: never, on_release, always
 # Allow disabling sub-resource integrity (SRI)
 no_sri = false
 
@@ -45,7 +45,7 @@ no_autoreload = false
 # Disable error reporting
 no_error_reporting = false
 # Additional headers set for responses.
-#headers = { "test-header" = "header value", "test-header2" = "header value 2" }
+# headers = { "test-header" = "header value", "test-header2" = "header value 2" }
 # Protocol used for autoreload WebSockets connection.
 ws_protocol = "ws"
 # The certificate/private key pair to use for TLS, which is enabled if both are set.

site/content/assets.md

@@ -56,7 +56,7 @@ This will typically look like: `<link data-trunk rel="{type}" href="{path}" ..ot
 
   - In the future, Trunk will resolve local `@imports`, will handle minification (see [trunk#7](https://github.com/trunk-rs/trunk/issues/7)), and we may even look into a pattern where any CSS found in the source tree will be bundled, which would enable a nice zero-config "component styles" pattern. See [trunk#3](https://github.com/trunk-rs/trunk/issues/3) for more details.
   - `data-integrity`: (optional) the `integrity` digest type for code & script resources. Defaults to plain `sha384`.
-  - `data-no-minify`: (optional) by default, CSS files are minified in `--release` mode (unless building with `--no-minification`). Setting this attribute disables minification for that particular file. Defaults to false.
+  - `data-no-minify`: (optional) Opt-out of minification. Also see: [Minification](#minification).
   - `data-target-path`: (optional) Path where the output is placed inside the dist dir. If not present, the directory is placed in the dist root. The path must be a relative path without `..`.
 
 ## tailwind
@@ -65,13 +65,15 @@ This will typically look like: `<link data-trunk rel="{type}" href="{path}" ..ot
 
   - `data-inline`: (optional) this attribute will inline the compiled CSS from the tailwind compilation into a `<style>` tag instead of using a `<link rel="stylesheet">` tag.
   - `data-integrity`: (optional) the `integrity` digest type for code & script resources. Defaults to plain `sha384`.
+  - `data-no-minify`: (optional) Opt-out of minification. Also see: [Minification](#minification).
   - `data-target-path`: (optional) Path where the output is placed inside the dist dir. If not present, the directory is placed in the dist root. The path must be a relative path without `..`.
 
 ## icon
 
 ✅ `rel="icon"`: Trunk will copy the icon image specified in the `href` attribute to the `dist` dir. This content is hashed for cache control.
 
   - `data-integrity`: (optional) the `integrity` digest type for code & script resources. Defaults to plain `sha384`.
+  - `data-no-minify`: (optional) Opt-out of minification. Also see: [Minification](#minification).
   - `data-target-path`: (optional) Path where the output is placed inside the dist dir. If not present, the directory is placed in the dist root. The path must be a relative path without `..`.
 
 ## inline
@@ -111,7 +113,7 @@ This will typically look like: `<script data-trunk src="{path}" ..other options
 
 Trunk will copy script files found in the source HTML without content modification. This content is hashed for cache control. The `src` attribute must be included in the script pointing to the script file to be processed.
 
-  - `data-no-minify`: (optional) by default, scripts are minified in `--release` mode (unless building with `--no-minification`). Setting this attribute disables minification for that particular file. Defaults to false.
+  - `data-no-minify`: (optional) Opt-out of minification. Also see: [Minification](#minification).
   - `data-target-path`: (optional) Path where the output is placed inside the dist dir. If not present, the directory is placed in the dist root. The path must be a relative path without `..`.
 
 ## JS Snippets
@@ -184,3 +186,19 @@ All hooks are executed using the same `stdin` and `stdout` as trunk. The executa
 # Auto-Reload
 
 As of `v0.14.0`, Trunk now ships with the ability to automatically reload your web app as the Trunk build pipeline completes.
+
+# Minification
+
+Trunk supports minifying of assets. This is disabled by default and can be controlled on various levels.
+
+In any case, Trunk does not perform minification itself, but delegates the process to dependencies which do the actual
+implementation. In cases where minification breaks things, it will, most likely, be an issue with that dependency.
+
+Starting with Trunk 0.20.0, minification is disabled by default. It can be turned on from the command line using the
+`--minify` (or `-M`) switch. Alternatively, it can be controlled using the `build.minify` field in the `Trunk.toml`
+file. The value of this field is an enum, with the following possible values: `never` (default, never minify),
+`on_release` (minify when running Trunk with `--release`), `always` (always minify).
+
+When minification is enabled, all assets known to trunk (this excludes the `copy-dir` and `copy-file` opaque blobs to
+Trunk), will get minified. It is possible to opt out of this process on a per-asset basis using the `data-no-minify`
+attribute (see individual asset configuration). In this case, the asset will *never* get minified.

src/config/mod.rs

@@ -17,6 +17,7 @@ pub const STAGE_DIR: &str = ".stage";
 pub use manifest::CargoMetadata;
 pub use models::{
     BaseUrl, ConfigOpts, ConfigOptsBuild, ConfigOptsClean, ConfigOptsCore, ConfigOptsHook,
-    ConfigOptsProxy, ConfigOptsServe, ConfigOptsTools, ConfigOptsWatch, CrossOrigin, WsProtocol,
+    ConfigOptsProxy, ConfigOptsServe, ConfigOptsTools, ConfigOptsWatch, CrossOrigin, Minify,
+    WsProtocol,
 };
 pub use rt::{Features, RtcBuild, RtcClean, RtcCore, RtcServe, RtcWatch};

src/config/models/build.rs

@@ -1,4 +1,5 @@
 use crate::config::models::BaseUrl;
+use crate::config::Minify;
 use clap::Args;
 use serde::Deserialize;
 use std::collections::HashMap;
@@ -122,10 +123,20 @@ pub struct ConfigOptsBuild {
     #[arg(long)]
     pub accept_invalid_certs: Option<bool>,
 
-    /// Allows disabling minification
+    /// Enable minification.
+    ///
+    /// This overrides the value from the configuration file.
+    // cli version
     #[serde(default)]
-    #[arg(long)]
-    pub no_minification: bool,
+    #[arg(id = "minify", short = 'M', long)]
+    pub minify_cli: bool,
+
+    /// Control minification.
+    // toml version
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(rename = "minify")]
+    #[arg(skip)]
+    pub minify_toml: Option<Minify>,
 
     /// Allows disabling sub-resource integrity (SRI)
     #[serde(default)]

src/config/models/mod.rs

@@ -353,9 +353,10 @@ impl ConfigOpts {
                 g.pattern_preload = g.pattern_preload.or(l.pattern_preload);
                 g.pattern_script = g.pattern_script.or(l.pattern_script);
                 g.pattern_params = g.pattern_params.or(l.pattern_params);
-                // NOTE: this can not be disabled in the cascade.
-                if l.no_minification {
-                    g.no_minification = true;
+
+                g.minify_toml = g.minify_toml.or(l.minify_toml);
+                if l.minify_cli {
+                    g.minify_cli = true;
                 }
                 // NOTE: this can not be disabled in the cascade.
                 if l.no_sri {

src/config/models/types/duration.rs

@@ -2,11 +2,12 @@ use serde::{Deserialize, Deserializer};
 use std::str::FromStr;
 use std::time::Duration;
 
+/// A newtype to allow using humantime durations as clap and serde values.
 #[derive(Clone, Debug)]
 pub struct ConfigDuration(pub Duration);
 
 impl<'de> Deserialize<'de> for ConfigDuration {
-    fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
@@ -17,7 +18,7 @@ impl<'de> Deserialize<'de> for ConfigDuration {
 impl FromStr for ConfigDuration {
     type Err = humantime::DurationError;
 
-    fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
         Ok(Self(humantime::Duration::from_str(s)?.into()))
     }
 }

src/config/models/types/minify.rs

@@ -0,0 +1,11 @@
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
+#[serde(rename_all = "snake_case")]
+pub enum Minify {
+    /// Never minify
+    #[default]
+    Never,
+    /// Minify for release builds
+    OnRelease,
+    /// Minify for all builds
+    Always,
+}

src/config/models/types/mod.rs

@@ -1,9 +1,11 @@
 mod address_family;
 mod base_url;
 mod duration;
+mod minify;
 mod ws;
 
 pub use address_family::*;
 pub use base_url::*;
 pub use duration::*;
+pub use minify::*;
 pub use ws::*;

src/config/rt/build.rs

@@ -1,6 +1,8 @@
 use super::super::{DIST_DIR, STAGE_DIR};
-use crate::config::models::BaseUrl;
-use crate::config::{ConfigOptsBuild, ConfigOptsCore, ConfigOptsHook, ConfigOptsTools, RtcCore};
+use crate::config::{
+    models::{BaseUrl, Minify},
+    ConfigOptsBuild, ConfigOptsCore, ConfigOptsHook, ConfigOptsTools, RtcCore,
+};
 use anyhow::{ensure, Context};
 use std::collections::HashMap;
 use std::io::ErrorKind;
@@ -72,8 +74,8 @@ pub struct RtcBuild {
     ///
     /// **WARNING**: Setting this to true can make you vulnerable to man-in-the-middle attacks. Sometimes this is necessary when working behind corporate proxies.
     pub accept_invalid_certs: Option<bool>,
-    /// Allow disabling minification
-    pub no_minification: bool,
+    /// Control minification
+    pub minify: Minify,
     /// Allow disabling SRI
     pub no_sri: bool,
     /// Ignore error's due to self closed script tags, instead will issue a warning.
@@ -152,6 +154,13 @@ impl RtcBuild {
             public_url = public_url.fix_trailing_slash();
         }
 
+        let minify = match (opts.minify_cli, opts.minify_toml) {
+            // the CLI will override with "always"
+            (true, _) => Minify::Always,
+            // otherwise, we take the configuration value, or the default
+            (false, minify) => minify.unwrap_or_default(),
+        };
+
         Ok(Self {
             core,
             target,
@@ -174,7 +183,7 @@ impl RtcBuild {
             locked: opts.locked,
             root_certificate: opts.root_certificate.map(PathBuf::from),
             accept_invalid_certs: opts.accept_invalid_certs,
-            no_minification: opts.no_minification,
+            minify,
             no_sri: opts.no_sri,
             ignore_script_error: opts.ignore_script_error,
         })
@@ -217,16 +226,23 @@ impl RtcBuild {
             locked: false,
             root_certificate: None,
             accept_invalid_certs: None,
-            no_minification: false,
+            minify: Minify::Never,
             no_sri: false,
             ignore_script_error: false,
         })
     }
 
-    /// Check if minification is globally enabled
-    ///
-    /// An asset might have an additional configuration, overriding the global one.
+    /// Evaluate the minify state with an asset's no_minify setting.
+    pub fn minify_asset(&self, no_minify: bool) -> bool {
+        !no_minify && self.should_minify()
+    }
+
+    /// Evaluate a global minify state, assets might override this.
     pub fn should_minify(&self) -> bool {
-        self.release && !self.no_minification
+        match (self.minify, self.release) {
+            (Minify::Never, _) => false,
+            (Minify::OnRelease, release) => release,
+            (Minify::Always, _) => true,
+        }
     }
 }

src/pipelines/css.rs

@@ -28,7 +28,7 @@ pub struct Css {
     /// The required integrity setting
     integrity: IntegrityType,
     /// Whether to minify or not
-    minify: bool,
+    no_minify: bool,
     /// Optional target path inside the dist dir.
     target_path: Option<PathBuf>,
 }
@@ -51,7 +51,7 @@ impl Css {
         let asset = AssetFile::new(&html_dir, path).await?;
 
         let integrity = IntegrityType::from_attrs(&attrs, &cfg)?;
-        let minify = !attrs.contains_key(ATTR_NO_MINIFY);
+        let no_minify = attrs.contains_key(ATTR_NO_MINIFY);
         let target_path = data_target_path(&attrs)?;
 
         Ok(Self {
@@ -60,7 +60,7 @@ impl Css {
             asset,
             attrs,
             integrity,
-            minify,
+            no_minify,
             target_path,
         })
     }
@@ -76,7 +76,6 @@ impl Css {
     async fn run(self) -> Result<TrunkAssetPipelineOutput> {
         let rel_path = crate::common::strip_prefix(&self.asset.path);
         tracing::debug!(path = ?rel_path, "copying & hashing css");
-        let minify = self.cfg.should_minify() && self.minify;
 
         let result_path =
             target_path(&self.cfg.staging_dist, self.target_path.as_deref(), None).await?;
@@ -87,7 +86,7 @@ impl Css {
                 &self.cfg.staging_dist,
                 &result_path,
                 self.cfg.filehash,
-                minify,
+                self.cfg.minify_asset(self.no_minify),
                 AssetFileType::Css,
             )
             .await?;

src/pipelines/icon.rs

@@ -27,7 +27,7 @@ pub struct Icon {
     /// The required integrity setting
     integrity: IntegrityType,
     /// Whether to minify or not
-    minify: bool,
+    no_minify: bool,
     /// Optional target path inside the dist dir.
     target_path: Option<PathBuf>,
 }
@@ -50,15 +50,15 @@ impl Icon {
         let asset = AssetFile::new(&html_dir, path).await?;
 
         let integrity = IntegrityType::from_attrs(&attrs, &cfg)?;
-        let minify = !attrs.contains_key(ATTR_NO_MINIFY);
+        let no_minify = attrs.contains_key(ATTR_NO_MINIFY);
         let target_path = data_target_path(&attrs)?;
 
         Ok(Self {
             id,
             cfg,
             asset,
             integrity,
-            minify,
+            no_minify,
             target_path,
         })
     }
@@ -89,7 +89,7 @@ impl Icon {
                 &self.cfg.staging_dist,
                 &result_dir,
                 self.cfg.filehash,
-                self.cfg.should_minify() && self.minify,
+                self.cfg.minify_asset(self.no_minify),
                 AssetFileType::Icon(image_type),
             )
             .await?;

src/pipelines/js.rs

@@ -30,7 +30,7 @@ pub struct Js {
     /// If it's a JavaScript module (vs a classic script)
     module: bool,
     /// Whether to minify or not
-    minify: bool,
+    no_minify: bool,
     /// Optional target path inside the dist dir.
     target_path: Option<PathBuf>,
 }
@@ -52,7 +52,7 @@ impl Js {
 
         let integrity = IntegrityType::from_attrs(&attrs, &cfg)?;
         let module = attrs.get("type").map(|s| s.as_str()) == Some("module");
-        let minify = !attrs.contains_key(ATTR_NO_MINIFY);
+        let no_minify = attrs.contains_key(ATTR_NO_MINIFY);
         let target_path = data_target_path(&attrs)?;
 
         Ok(Self {
@@ -62,7 +62,7 @@ impl Js {
             module,
             attrs,
             integrity,
-            minify,
+            no_minify,
             target_path,
         })
     }
@@ -78,7 +78,6 @@ impl Js {
     async fn run(self) -> Result<TrunkAssetPipelineOutput> {
         let rel_path = crate::common::strip_prefix(&self.asset.path);
         tracing::debug!(path = ?rel_path, "copying & hashing js");
-        let minify = self.cfg.should_minify() && self.minify;
 
         let result_dir =
             target_path(&self.cfg.staging_dist, self.target_path.as_deref(), None).await?;
@@ -89,7 +88,7 @@ impl Js {
                 &self.cfg.staging_dist,
                 &result_dir,
                 self.cfg.filehash,
-                minify,
+                self.cfg.minify_asset(self.no_minify),
                 if self.module {
                     AssetFileType::Mjs
                 } else {

src/pipelines/mod.rs

@@ -48,6 +48,7 @@ const ATTR_TYPE: &str = "type";
 const ATTR_REL: &str = "rel";
 const ATTR_NO_MINIFY: &str = "data-no-minify";
 const ATTR_TARGET_PATH: &str = "data-target-path";
+
 const SNIPPETS_DIR: &str = "snippets";
 const TRUNK_ID: &str = "data-trunk-id";
 const PNG_OPTIMIZATION_LEVEL: u8 = 6;

src/pipelines/rust/mod.rs

@@ -800,7 +800,7 @@ impl RustApp {
             .await
             .context("error reading JS loader file")?;
 
-        let write_bytes = match self.cfg.release && !self.cfg.no_minification {
+        let write_bytes = match self.cfg.should_minify() {
             true => minify_js(bytes, mode),
             false => bytes,
         };