feat: improve warnings for deprecations and lifecycle script for npm …

…packages (#25694) This commit improves warning messages for deprecated npm packages and packages that rely on lifecycle script.
denoland · Sep 18, 2024 · 5b14c71 · 5b14c71
1 parent fd86026
commit 5b14c71
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 28 deletions.
diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
@@ -19,13 +19,15 @@ use std::sync::Arc;
 
 use crate::args::LifecycleScriptsConfig;
 use crate::args::PackagesAllowedScripts;
+use crate::colors;
 use async_trait::async_trait;
 use deno_ast::ModuleSpecifier;
 use deno_core::anyhow;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_core::futures::stream::FuturesUnordered;
 use deno_core::futures::StreamExt;
+use deno_core::parking_lot::Mutex;
 use deno_core::url::Url;
 use deno_npm::resolution::NpmResolutionSnapshot;
 use deno_npm::NpmPackageCacheFolderId;
@@ -461,6 +463,7 @@ async fn sync_resolution_with_fs(
   let bin_entries = Rc::new(RefCell::new(bin_entries::BinEntries::new()));
   let mut packages_with_scripts = Vec::with_capacity(2);
   let mut packages_with_scripts_not_run = Vec::new();
+  let packages_with_deprecation_warnings = Arc::new(Mutex::new(Vec::new()));
   for package in &package_partitions.packages {
     if let Some(current_pkg) =
       newest_packages_by_name.get_mut(&package.id.nv.name)
@@ -487,6 +490,8 @@ async fn sync_resolution_with_fs(
 
       let folder_path = folder_path.clone();
       let bin_entries_to_setup = bin_entries.clone();
+      let packages_with_deprecation_warnings =
+        packages_with_deprecation_warnings.clone();
       cache_futures.push(async move {
         tarball_cache
           .ensure_package(&package.id.nv, &package.dist)
@@ -519,12 +524,9 @@ async fn sync_resolution_with_fs(
         }
 
         if let Some(deprecated) = &package.deprecated {
-          log::info!(
-            "{} {:?} is deprecated: {}",
-            crate::colors::yellow("Warning"),
-            package.id,
-            crate::colors::gray(deprecated),
-          );
+          packages_with_deprecation_warnings
+            .lock()
+            .push((package.id.clone(), deprecated.clone()));
         }
 
         // finally stop showing the progress bar
@@ -849,15 +851,59 @@ async fn sync_resolution_with_fs(
     }
   }
 
+  {
+    let packages_with_deprecation_warnings =
+      packages_with_deprecation_warnings.lock();
+    if !packages_with_deprecation_warnings.is_empty() {
+      log::warn!(
+        "{} Following packages are deprecated:",
+        colors::yellow("Warning")
+      );
+      let len = packages_with_deprecation_warnings.len();
+      for (idx, (package_id, msg)) in
+        packages_with_deprecation_warnings.iter().enumerate()
+      {
+        if idx != len - 1 {
+          log::warn!(
+            "┠─ {}",
+            colors::gray(format!("npm:{:?} ({})", package_id, msg))
+          );
+        } else {
+          log::warn!(
+            "┗─ {}",
+            colors::gray(format!("npm:{:?} ({})", package_id, msg))
+          );
+        }
+      }
+    }
+  }
+
   if !packages_with_scripts_not_run.is_empty() {
-    let packages = packages_with_scripts_not_run
+    log::warn!("{} Following packages contained npm lifecycle scripts ({}) that were not executed:", colors::yellow("Warning"), colors::gray("preinstall/install/postinstall"));
+
+    for (_, package_nv) in packages_with_scripts_not_run.iter() {
+      log::warn!("┠─ {}", colors::gray(format!("npm:{package_nv}")));
+    }
+
+    log::warn!("┃");
+    log::warn!(
+      "┠─ {}",
+      colors::italic("This may cause the packages to not work correctly.")
+    );
+    log::warn!("┗─ {}", colors::italic("To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:"));
+    let packages_comma_separated = packages_with_scripts_not_run
       .iter()
       .map(|(_, p)| format!("npm:{p}"))
       .collect::<Vec<_>>()
-      .join(", ");
-    log::warn!("{} Packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed.
-    This may cause the packages to not work correctly. To run them, use the `--allow-scripts` flag with `deno cache` or `deno install`
-    (e.g. `deno cache --allow-scripts=pkg1,pkg2 <entrypoint>` or `deno install --allow-scripts=pkg1,pkg2`):\n      {packages}", crate::colors::yellow("Warning"));
+      .join(",");
+    log::warn!(
+      "   {}",
+      colors::bold(format!(
+        "deno install --allow-scripts={}",
+        packages_comma_separated
+      ))
+    );
+
     for (scripts_warned_path, _) in packages_with_scripts_not_run {
       let _ignore_err = fs::write(scripts_warned_path, "");
     }

diff --git a/tests/specs/install/install_deprecated_package/install.out b/tests/specs/install/install_deprecated_package/install.out
@@ -2,4 +2,5 @@ Add npm:@denotest/deprecated-package@1.0.0
 Download http://localhost:4260/@denotest/deprecated-package
 Download http://localhost:4260/@denotest/deprecated-package/1.0.0.tgz
 Initialize @denotest/deprecated-package@1.0.0
-Warning @denotest/deprecated-package@1.0.0 is deprecated: Deprecated version
+Warning Following packages are deprecated:
+┗─ npm:@denotest/deprecated-package@1.0.0 (Deprecated version)
diff --git a/tests/specs/npm/lifecycle_scripts/all_lifecycles_not_run.out b/tests/specs/npm/lifecycle_scripts/all_lifecycles_not_run.out
@@ -6,7 +6,9 @@ Download http://localhost:4260/@denotest/bin/1.0.0.tgz
 Initialize @denotest/node-lifecycle-scripts@1.0.0
 Initialize @denotest/bin@1.0.0
 [UNORDERED_END]
-Warning Packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed.
-    This may cause the packages to not work correctly. To run them, use the `--allow-scripts` flag with `deno cache` or `deno install`
-    (e.g. `deno cache --allow-scripts=pkg1,pkg2 <entrypoint>` or `deno install --allow-scripts=pkg1,pkg2`):
-      npm:@denotest/node-lifecycle-scripts@1.0.0
+Warning Following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+┠─ npm:@denotest/node-lifecycle-scripts@1.0.0
+┃
+┠─ This may cause the packages to not work correctly.
+┗─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
+   deno install --allow-scripts=npm:@denotest/node-lifecycle-scripts@1.0.0
diff --git a/tests/specs/npm/lifecycle_scripts/future_install_all_lifecycles_not_run.out b/tests/specs/npm/lifecycle_scripts/future_install_all_lifecycles_not_run.out
@@ -6,7 +6,9 @@ Download http://localhost:4260/@denotest/bin/1.0.0.tgz
 Initialize @denotest/node-lifecycle-scripts@1.0.0
 Initialize @denotest/bin@1.0.0
 [UNORDERED_END]
-Warning Packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed.
-    This may cause the packages to not work correctly. To run them, use the `--allow-scripts` flag with `deno cache` or `deno install`
-    (e.g. `deno cache --allow-scripts=pkg1,pkg2 <entrypoint>` or `deno install --allow-scripts=pkg1,pkg2`):
-      npm:@denotest/node-lifecycle-scripts@1.0.0
+Warning Following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+┠─ npm:@denotest/node-lifecycle-scripts@1.0.0
+┃
+┠─ This may cause the packages to not work correctly.
+┗─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
+   deno install --allow-scripts=npm:@denotest/node-lifecycle-scripts@1.0.0
diff --git a/tests/specs/npm/lifecycle_scripts/node_gyp_not_run.out b/tests/specs/npm/lifecycle_scripts/node_gyp_not_run.out
@@ -1,9 +1,11 @@
 Download http://localhost:4260/@denotest/node-addon
 Download http://localhost:4260/node-gyp
 [WILDCARD]
-Warning Packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed.
-    This may cause the packages to not work correctly. To run them, use the `--allow-scripts` flag with `deno cache` or `deno install`
-    (e.g. `deno cache --allow-scripts=pkg1,pkg2 <entrypoint>` or `deno install --allow-scripts=pkg1,pkg2`):
-      npm:@denotest/node-addon@1.0.0
+Warning Following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+┠─ npm:@denotest/node-addon@1.0.0
+┃
+┠─ This may cause the packages to not work correctly.
+┗─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
+   deno install --allow-scripts=npm:@denotest/node-addon@1.0.0
 error: Uncaught (in promise) Error: Cannot find module './build/Release/node_addon'
 [WILDCARD]
diff --git a/tests/specs/npm/lifecycle_scripts/only_warns_first1.out b/tests/specs/npm/lifecycle_scripts/only_warns_first1.out
@@ -6,9 +6,11 @@ Download http://localhost:4260/@denotest/bin/1.0.0.tgz
 Initialize @denotest/node-lifecycle-scripts@1.0.0
 Initialize @denotest/bin@1.0.0
 [UNORDERED_END]
-Warning Packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed.
-    This may cause the packages to not work correctly. To run them, use the `--allow-scripts` flag with `deno cache` or `deno install`
-    (e.g. `deno cache --allow-scripts=pkg1,pkg2 <entrypoint>` or `deno install --allow-scripts=pkg1,pkg2`):
-      npm:@denotest/node-lifecycle-scripts@1.0.0
+Warning Following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+┠─ npm:@denotest/node-lifecycle-scripts@1.0.0
+┃
+┠─ This may cause the packages to not work correctly.
+┗─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
+   deno install --allow-scripts=npm:@denotest/node-lifecycle-scripts@1.0.0
 error: Uncaught SyntaxError: The requested module 'npm:@denotest/node-lifecycle-scripts' does not provide an export named 'value'
 [WILDCARD]