fix(repl): improve package.json support (#18497)

1. Fixes a cosmetic issue in the repl where it would display lsp warning
messages.
2. Lazily loads dependencies from the package.json on use.
3. Supports using bare specifiers from package.json in the REPL.

Closes #17929
Closes #18494

12 files changed, 124 insertions, 69 deletions

diff --git a/cli/args/flags.rs b/cli/args/flags.rs
index f3b92b4fb..7caa12ca8 100644
--- a/cli/args/flags.rs
+++ b/cli/args/flags.rs
@@ -527,7 +527,7 @@ impl Flags {
           .ok()
       }
       Task(_) | Check(_) | Coverage(_) | Cache(_) | Info(_) | Eval(_)
-      | Test(_) | Bench(_) => std::env::current_dir().ok(),
+      | Test(_) | Bench(_) | Repl(_) => std::env::current_dir().ok(),
       _ => None,
     }
   }
@@ -2245,7 +2245,7 @@ fn lock_arg() -> Arg {
   Arg::new("lock")
     .long("lock")
     .value_name("FILE")
-    .help("Check the specified lock file. 
+    .help("Check the specified lock file.
 
 If value is not provided, defaults to \"deno.lock\" in the current working directory.")
     .num_args(0..=1)
diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index d8a94e538..aa47faf62 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -834,8 +834,6 @@ pub struct Documents {
   /// A resolver that takes into account currently loaded import map and JSX
   /// settings.
   resolver: CliGraphResolver,
-  /// The npm package requirements found in a package.json file.
-  npm_package_json_reqs: Arc<Vec<NpmPackageReq>>,
   /// The npm package requirements found in npm specifiers.
   npm_specifier_reqs: Arc<Vec<NpmPackageReq>>,
   /// Gets if any document had a node: specifier such that a @types/node package
@@ -856,7 +854,6 @@ impl Documents {
       resolver_config_hash: 0,
       imports: Default::default(),
       resolver: CliGraphResolver::default(),
-      npm_package_json_reqs: Default::default(),
       npm_specifier_reqs: Default::default(),
       has_injected_types_node_package: false,
       specifier_resolver: Arc::new(SpecifierResolver::new(location)),
@@ -994,15 +991,9 @@ impl Documents {
   }
 
   /// Returns a collection of npm package requirements.
-  pub fn npm_package_reqs(&mut self) -> Vec<NpmPackageReq> {
+  pub fn npm_package_reqs(&mut self) -> Arc<Vec<NpmPackageReq>> {
     self.calculate_dependents_if_dirty();
-    let mut reqs = Vec::with_capacity(
-      self.npm_package_json_reqs.len() + self.npm_specifier_reqs.len(),
-    );
-    // resolve the package.json reqs first, then the npm specifiers
-    reqs.extend(self.npm_package_json_reqs.iter().cloned());
-    reqs.extend(self.npm_specifier_reqs.iter().cloned());
-    reqs
+    self.npm_specifier_reqs.clone()
   }
 
   /// Returns if a @types/node package was injected into the npm
@@ -1206,20 +1197,6 @@ impl Documents {
       maybe_jsx_config.as_ref(),
       maybe_package_json_deps.as_ref(),
     );
-    self.npm_package_json_reqs = Arc::new({
-      match &maybe_package_json_deps {
-        Some(deps) => {
-          let mut reqs = deps
-            .values()
-            .filter_map(|r| r.as_ref().ok())
-            .cloned()
-            .collect::<Vec<_>>();
-          reqs.sort();
-          reqs
-        }
-        None => Vec::new(),
-      }
-    });
     let deps_installer = PackageJsonDepsInstaller::new(
       npm_registry_api.clone(),
       npm_resolution.clone(),
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 372a1489d..2d0bbd140 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -13,7 +13,6 @@ use deno_runtime::deno_node::PackageJson;
 use deno_runtime::deno_web::BlobStore;
 use import_map::ImportMap;
 use log::error;
-use log::warn;
 use serde_json::from_value;
 use std::collections::HashMap;
 use std::collections::HashSet;
@@ -47,6 +46,7 @@ use super::documents::Documents;
 use super::documents::DocumentsFilter;
 use super::documents::LanguageId;
 use super::logging::lsp_log;
+use super::logging::lsp_warn;
 use super::lsp_custom;
 use super::parent_process_checker;
 use super::performance::Performance;
@@ -675,7 +675,7 @@ impl Inner {
       if let Some(ignored_options) = maybe_ignored_options {
         // TODO(@kitsonk) turn these into diagnostics that can be sent to the
         // client
-        warn!("{}", ignored_options);
+        lsp_warn!("{}", ignored_options);
       }
     }
 
@@ -1166,9 +1166,10 @@ impl Inner {
           LanguageId::Unknown
         });
     if language_id == LanguageId::Unknown {
-      warn!(
+      lsp_warn!(
         "Unsupported language id \"{}\" received for document \"{}\".",
-        params.text_document.language_id, params.text_document.uri
+        params.text_document.language_id,
+        params.text_document.uri
       );
     }
     let document = self.documents.open(
@@ -1209,8 +1210,12 @@ impl Inner {
 
   async fn refresh_npm_specifiers(&mut self) {
     let package_reqs = self.documents.npm_package_reqs();
-    if let Err(err) = self.npm_resolver.set_package_reqs(package_reqs).await {
-      warn!("Could not set npm package requirements. {:#}", err);
+    if let Err(err) = self
+      .npm_resolver
+      .set_package_reqs((*package_reqs).clone())
+      .await
+    {
+      lsp_warn!("Could not set npm package requirements. {:#}", err);
     }
   }
 
@@ -1463,7 +1468,7 @@ impl Inner {
       Ok(None) => Some(Vec::new()),
       Err(err) => {
         // TODO(lucacasonato): handle error properly
-        warn!("Format error: {:#}", err);
+        lsp_warn!("Format error: {:#}", err);
         None
       }
     };
@@ -2846,7 +2851,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
         .register_capability(vec![registration])
         .await
       {
-        warn!("Client errored on capabilities.\n{:#}", err);
+        lsp_warn!("Client errored on capabilities.\n{:#}", err);
       }
     }
 
diff --git a/cli/lsp/logging.rs b/cli/lsp/logging.rs
index 7099b08bc..8b703f2a5 100644
--- a/cli/lsp/logging.rs
+++ b/cli/lsp/logging.rs
@@ -6,6 +6,8 @@ use std::sync::atomic::Ordering;
 
 static LSP_DEBUG_FLAG: AtomicBool = AtomicBool::new(false);
 static LSP_LOG_LEVEL: AtomicUsize = AtomicUsize::new(log::Level::Info as usize);
+static LSP_WARN_LEVEL: AtomicUsize =
+  AtomicUsize::new(log::Level::Warn as usize);
 
 pub fn set_lsp_debug_flag(value: bool) {
   LSP_DEBUG_FLAG.store(value, Ordering::SeqCst)
@@ -15,6 +17,7 @@ pub fn lsp_debug_enabled() -> bool {
   LSP_DEBUG_FLAG.load(Ordering::SeqCst)
 }
 
+/// Change the lsp to log at the provided level.
 pub fn set_lsp_log_level(level: log::Level) {
   LSP_LOG_LEVEL.store(level as usize, Ordering::SeqCst)
 }
@@ -28,13 +31,40 @@ pub fn lsp_log_level() -> log::Level {
   }
 }
 
+/// Change the lsp to warn at the provided level.
+pub fn set_lsp_warn_level(level: log::Level) {
+  LSP_WARN_LEVEL.store(level as usize, Ordering::SeqCst)
+}
+
+pub fn lsp_warn_level() -> log::Level {
+  let level = LSP_LOG_LEVEL.load(Ordering::SeqCst);
+  // TODO(bartlomieju):
+  #[allow(clippy::undocumented_unsafe_blocks)]
+  unsafe {
+    std::mem::transmute(level)
+  }
+}
+
 /// Use this macro to do "info" logs in the lsp code. This allows
 /// for downgrading these logs to another log level in the REPL.
 macro_rules! lsp_log {
   ($($arg:tt)+) => (
-    let lsp_log_level = crate::lsp::logging::lsp_log_level();
+    let lsp_log_level = $crate::lsp::logging::lsp_log_level();
+    if lsp_log_level == log::Level::Debug {
+      $crate::lsp::logging::lsp_debug!($($arg)+)
+    } else {
+      log::log!(lsp_log_level, $($arg)+)
+    }
+  )
+}
+
+/// Use this macro to do "warn" logs in the lsp code. This allows
+/// for downgrading these logs to another log level in the REPL.
+macro_rules! lsp_warn {
+  ($($arg:tt)+) => (
+    let lsp_log_level = $crate::lsp::logging::lsp_warn_level();
     if lsp_log_level == log::Level::Debug {
-      crate::lsp::logging::lsp_debug!($($arg)+)
+      $crate::lsp::logging::lsp_debug!($($arg)+)
     } else {
       log::log!(lsp_log_level, $($arg)+)
     }
@@ -51,3 +81,4 @@ macro_rules! lsp_debug {
 
 pub(super) use lsp_debug;
 pub(super) use lsp_log;
+pub(super) use lsp_warn;
diff --git a/cli/lsp/repl.rs b/cli/lsp/repl.rs
index 41a3f993a..ada8b9404 100644
--- a/cli/lsp/repl.rs
+++ b/cli/lsp/repl.rs
@@ -53,7 +53,10 @@ pub struct ReplLanguageServer {
 
 impl ReplLanguageServer {
   pub async fn new_initialized() -> Result<ReplLanguageServer, AnyError> {
+    // downgrade info and warn lsp logging to debug
     super::logging::set_lsp_log_level(log::Level::Debug);
+    super::logging::set_lsp_warn_level(log::Level::Debug);
+
     let language_server =
       super::language_server::LanguageServer::new(Client::new_for_repl());
 
diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs
index 6ef9b1dc3..316436988 100644
--- a/cli/lsp/tsc.rs
+++ b/cli/lsp/tsc.rs
@@ -20,6 +20,7 @@ use super::urls::LspUrlMap;
 use super::urls::INVALID_SPECIFIER;
 
 use crate::args::TsConfig;
+use crate::lsp::logging::lsp_warn;
 use crate::tsc;
 use crate::tsc::ResolveArgs;
 use crate::util::path::relative_specifier;
@@ -43,7 +44,6 @@ use deno_core::ModuleSpecifier;
 use deno_core::OpState;
 use deno_core::RuntimeOptions;
 use deno_runtime::tokio_util::create_basic_runtime;
-use log::warn;
 use once_cell::sync::Lazy;
 use regex::Captures;
 use regex::Regex;
@@ -114,7 +114,7 @@ impl TsServer {
           }
           let value = request(&mut ts_runtime, state_snapshot, req, token);
           if tx.send(value).is_err() {
-            warn!("Unable to send result to client.");
+            lsp_warn!("Unable to send result to client.");
           }
         }
       })
diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs
index 95a747523..27a9b716c 100644
--- a/cli/tests/integration/repl_tests.rs
+++ b/cli/tests/integration/repl_tests.rs
@@ -5,6 +5,7 @@ use test_util::assert_contains;
 use test_util::assert_ends_with;
 use test_util::assert_not_contains;
 use util::TempDir;
+use util::TestContextBuilder;
 
 #[test]
 fn pty_multiline() {
@@ -884,3 +885,39 @@ fn pty_tab_indexable_props() {
     assert_not_contains!(output, "0", "1", "2");
   });
 }
+
+#[test]
+fn package_json_uncached_no_error() {
+  let test_context = TestContextBuilder::for_npm()
+    .use_temp_cwd()
+    .use_http_server()
+    .env("RUST_BACKTRACE", "1")
+    .build();
+  let temp_dir = test_context.temp_dir();
+  temp_dir.write(
+    "package.json",
+    r#"{
+  "dependencies": {
+    "@denotest/esm-basic": "1.0.0"
+  }
+}
+"#,
+  );
+  test_context.new_command().with_pty(|mut console| {
+    console.write_line("console.log(123 + 456);");
+    console.expect("579");
+    assert_not_contains!(
+      console.all_output(),
+      "Could not set npm package requirements",
+    );
+
+    // should support getting the package now though
+    console
+      .write_line("import { getValue, setValue } from '@denotest/esm-basic';");
+    console.expect("undefined");
+    console.write_line("setValue(12 + 30);");
+    console.expect("undefined");
+    console.write_line("getValue()");
+    console.expect("42")
+  });
+}
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index 4504c970d..003bc59fc 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/cli/tests/integration/run_tests.rs
@@ -4248,8 +4248,6 @@ itest!(permission_args_quiet {
 
 // Regression test for https://github.com/denoland/deno/issues/16772
 #[test]
-// todo(dsherret): getting a dns error on windows for some reason
-#[cfg(unix)]
 fn file_fetcher_preserves_permissions() {
   let _guard = util::http_server();
   util::with_pty(&["repl", "--quiet"], |mut console| {
diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs
index 350b38bb9..95233de05 100644
--- a/cli/tools/repl/session.rs
+++ b/cli/tools/repl/session.rs
@@ -106,7 +106,7 @@ pub fn result_to_evaluation_output(
   match r {
     Ok(value) => value,
     Err(err) => {
-      EvaluationOutput::Error(format!("{} {}", colors::red("error:"), err))
+      EvaluationOutput::Error(format!("{} {:#}", colors::red("error:"), err))
     }
   }
 }
diff --git a/test_util/src/builders.rs b/test_util/src/builders.rs
index 84befb57a..4997dac2c 100644
--- a/test_util/src/builders.rs
+++ b/test_util/src/builders.rs
@@ -312,6 +312,14 @@ impl TestCommandBuilder {
     .collect::<Vec<_>>()
   }
 
+  fn build_envs(&self) -> HashMap<String, String> {
+    let mut envs = self.context.envs.clone();
+    for (key, value) in &self.envs {
+      envs.insert(key.to_string(), value.to_string());
+    }
+    envs
+  }
+
   pub fn with_pty(&self, mut action: impl FnMut(Pty)) {
     if !Pty::is_supported() {
       return;
@@ -319,11 +327,20 @@ impl TestCommandBuilder {
 
     let args = self.build_args();
     let args = args.iter().map(|s| s.as_str()).collect::<Vec<_>>();
-    let mut envs = self.envs.clone();
+    let mut envs = self.build_envs();
     if !envs.contains_key("NO_COLOR") {
       // set this by default for pty tests
       envs.insert("NO_COLOR".to_string(), "1".to_string());
     }
+
+    // note(dsherret): for some reason I need to inject the current
+    // environment here for the pty tests or else I get dns errors
+    if !self.env_clear {
+      for (key, value) in std::env::vars() {
+        envs.entry(key).or_insert(value);
+      }
+    }
+
     action(Pty::new(
       &self.build_command_path(),
       &args,
@@ -361,13 +378,7 @@ impl TestCommandBuilder {
       command.env_clear();
     }
     command.env("DENO_DIR", self.context.deno_dir.path());
-    command.envs({
-      let mut envs = self.context.envs.clone();
-      for (key, value) in &self.envs {
-        envs.insert(key.to_string(), value.to_string());
-      }
-      envs
-    });
+    command.envs(self.build_envs());
     command.current_dir(cwd);
     command.stdin(Stdio::piped());
 
diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs
index b38d72cd9..c844e594f 100644
--- a/test_util/src/lib.rs
+++ b/test_util/src/lib.rs
@@ -2166,24 +2166,12 @@ pub fn pattern_match(pattern: &str, s: &str, wildcard: &str) -> bool {
   t.1.is_empty()
 }
 
-pub fn with_pty(deno_args: &[&str], mut action: impl FnMut(Pty)) {
-  if !Pty::is_supported() {
-    return;
-  }
-
-  let deno_dir = new_deno_dir();
-  let mut env_vars = std::collections::HashMap::new();
-  env_vars.insert("NO_COLOR".to_string(), "1".to_string());
-  env_vars.insert(
-    "DENO_DIR".to_string(),
-    deno_dir.path().to_string_lossy().to_string(),
-  );
-  action(Pty::new(
-    &deno_exe_path(),
-    deno_args,
-    &testdata_path(),
-    Some(env_vars),
-  ))
+pub fn with_pty(deno_args: &[&str], action: impl FnMut(Pty)) {
+  let context = TestContextBuilder::default().build();
+  context
+    .new_command()
+    .args_vec(deno_args.iter().map(ToString::to_string).collect())
+    .with_pty(action);
 }
 
 pub struct WrkOutput {
diff --git a/test_util/src/pty.rs b/test_util/src/pty.rs
index 80d06881e..2f89e481e 100644
--- a/test_util/src/pty.rs
+++ b/test_util/src/pty.rs
@@ -1,5 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
+use std::borrow::Cow;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::io::Read;
@@ -35,7 +36,7 @@ impl Pty {
       read_bytes: Vec::new(),
       last_index: 0,
     };
-    if args[0] == "repl" && !args.contains(&"--quiet") {
+    if args.is_empty() || args[0] == "repl" && !args.contains(&"--quiet") {
       // wait for the repl to start up before writing to it
       pty.expect("exit using ctrl+d, ctrl+c, or close()");
     }
@@ -151,6 +152,10 @@ impl Pty {
     });
   }
 
+  pub fn all_output(&self) -> Cow<str> {
+    String::from_utf8_lossy(&self.read_bytes)
+  }
+
   #[track_caller]
   fn read_until_with_advancing(
     &mut self,