From a47b7c4391964f7eaeea763acb5a12865c8bdfe8 Mon Sep 17 00:00:00 2001
From: Matt Jones <mattjones701@gmail.com>
Date: Tue, 22 Oct 2024 08:51:40 +1100
Subject: [PATCH] [V3] Handle should optimize false correctly (#139)

---
 Cargo.lock                                     |  1 +
 crates/atlaspack/Cargo.toml                    |  1 +
 .../atlaspack/src/requests/target_request.rs   | 18 +++++++++---------
 .../src/types/atlaspack_options.rs             |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index d36441d5a..f9837db96 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -189,6 +189,7 @@ dependencies = [
  "num_cpus",
  "pathdiff",
  "petgraph",
+ "pretty_assertions",
  "rand 0.8.5",
  "rayon",
  "regex",
diff --git a/crates/atlaspack/Cargo.toml b/crates/atlaspack/Cargo.toml
index 342d644ab..2fa468b1c 100644
--- a/crates/atlaspack/Cargo.toml
+++ b/crates/atlaspack/Cargo.toml
@@ -43,3 +43,4 @@ xxhash-rust = { version = "0.8.2", features = ["xxh3"] }
 
 [dev-dependencies]
 mockall = "0.12.1"
+pretty_assertions = "1.4.0"
diff --git a/crates/atlaspack/src/requests/target_request.rs b/crates/atlaspack/src/requests/target_request.rs
index a1ce04a8c..468961d34 100644
--- a/crates/atlaspack/src/requests/target_request.rs
+++ b/crates/atlaspack/src/requests/target_request.rs
@@ -384,7 +384,10 @@ impl TargetRequest {
             .default_target_options
             .output_format
             .unwrap_or_else(|| fallback_output_format(context)),
-          should_optimize: self.default_target_options.should_optimize,
+          should_optimize: self
+            .default_target_options
+            .should_optimize
+            .unwrap_or_else(|| self.mode == BuildMode::Production && !is_library),
           should_scope_hoist: self
             .default_target_options
             .should_scope_hoist
@@ -530,14 +533,10 @@ impl TargetRequest {
         is_library,
         loc: None, // TODO
         output_format,
-        should_optimize: self.default_target_options.should_optimize
-          || if is_library {
-            // Libraries are not optimized by default, users must explicitly configure this.
-            target_descriptor.optimize.is_some_and(|o| o == true)
-          } else {
-            target_descriptor.optimize.is_none()
-              || target_descriptor.optimize.is_some_and(|o| o != false)
-          },
+        should_optimize: self.default_target_options.should_optimize.map_or_else(
+          || target_descriptor.optimize.unwrap_or(!is_library),
+          |o| o && target_descriptor.optimize.unwrap_or(!is_library),
+        ),
         should_scope_hoist: (is_library
           || self
             .default_target_options
@@ -654,6 +653,7 @@ mod tests {
   use atlaspack_filesystem::in_memory_file_system::InMemoryFileSystem;
 
   use crate::test_utils::{request_tracker, RequestTrackerTestOptions};
+  use pretty_assertions::assert_eq;
 
   use super::*;
 
diff --git a/crates/atlaspack_core/src/types/atlaspack_options.rs b/crates/atlaspack_core/src/types/atlaspack_options.rs
index 6eed51b6b..e0e1e6401 100644
--- a/crates/atlaspack_core/src/types/atlaspack_options.rs
+++ b/crates/atlaspack_core/src/types/atlaspack_options.rs
@@ -78,7 +78,7 @@ pub struct DefaultTargetOptions {
   pub is_library: Option<bool>,
   pub output_format: Option<OutputFormat>,
   pub public_url: String,
-  pub should_optimize: bool,
+  pub should_optimize: Option<bool>,
   pub should_scope_hoist: Option<bool>,
   pub source_maps: bool,
 }
@@ -91,7 +91,7 @@ impl Default for DefaultTargetOptions {
       is_library: None,
       output_format: None,
       public_url: String::from("/"),
-      should_optimize: false,
+      should_optimize: None,
       should_scope_hoist: None,
       source_maps: false,
     }