1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-22 15:24:46 -05:00

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
This commit is contained in:
David Sherret 2023-03-30 10:43:16 -04:00 committed by GitHub
parent 3deade4b14
commit e0429e2ad6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 124 additions and 69 deletions

View file

@ -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)

View file

@ -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(),

View file

@ -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);
}
}

View file

@ -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)+)
$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)+)
} 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;

View file

@ -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());

View file

@ -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.");
}
}
})

View file

@ -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")
});
}

View file

@ -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| {

View file

@ -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))
}
}
}

View file

@ -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());

View file

@ -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 {

View file

@ -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,