diff options
Diffstat (limited to 'cli')
14 files changed, 198 insertions, 44 deletions
diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 28b4a0957..4c000ea55 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -42,16 +42,16 @@ winres.workspace = true [dependencies] deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] } deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } -deno_doc = "=0.63.1" -deno_emit = "=0.24.0" -deno_graph = "=0.49.0" +deno_doc = "=0.64.0" +deno_emit = "=0.25.0" +deno_graph = "=0.50.0" deno_lint = { version = "=0.49.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm.workspace = true deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] } deno_semver.workspace = true deno_task_shell = "=0.13.1" -eszip = "=0.45.0" +eszip = "=0.48.0" napi_sym.workspace = true async-trait.workspace = true diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs index a8c6eaad4..76a09eadd 100644 --- a/cli/args/package_json.rs +++ b/cli/args/package_json.rs @@ -85,7 +85,7 @@ pub fn get_local_package_json_version_reqs( match result { Ok(version_req) => Ok(NpmPackageReq { name: name.to_string(), - version_req: Some(version_req), + version_req, }), Err(err) => Err(PackageJsonDepValueParseError::Specifier(err)), } diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs index bfba1d67d..6992d875e 100644 --- a/cli/npm/resolution.rs +++ b/cli/npm/resolution.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; @@ -249,6 +250,10 @@ impl NpmResolution { Ok(nv) } + pub fn package_reqs(&self) -> HashMap<NpmPackageReq, NpmPackageNv> { + self.snapshot.read().package_reqs().clone() + } + pub fn all_system_packages( &self, system_info: &NpmSystemInfo, @@ -303,45 +308,44 @@ async fn add_package_reqs_to_snapshot( get_new_snapshot: impl Fn() -> NpmResolutionSnapshot, ) -> Result<NpmResolutionSnapshot, AnyError> { let snapshot = get_new_snapshot(); - if !snapshot.has_pending() + let snapshot = if !snapshot.has_pending() && package_reqs .iter() .all(|req| snapshot.package_reqs().contains_key(req)) { log::debug!("Snapshot already up to date. Skipping pending resolution."); - return Ok(snapshot); - } - - let pending_resolver = get_npm_pending_resolver(api); - let result = pending_resolver - .resolve_pending(snapshot, package_reqs) - .await; - api.clear_memory_cache(); - let snapshot = match result { - Ok(snapshot) => snapshot, - Err(NpmResolutionError::Resolution(err)) if api.mark_force_reload() => { - log::debug!("{err:#}"); - log::debug!("npm resolution failed. Trying again..."); - - // try again - let snapshot = get_new_snapshot(); - let result = pending_resolver - .resolve_pending(snapshot, package_reqs) - .await; - api.clear_memory_cache(); - // now surface the result after clearing the cache - result? + snapshot + } else { + let pending_resolver = get_npm_pending_resolver(api); + let result = pending_resolver + .resolve_pending(snapshot, package_reqs) + .await; + api.clear_memory_cache(); + match result { + Ok(snapshot) => snapshot, + Err(NpmResolutionError::Resolution(err)) if api.mark_force_reload() => { + log::debug!("{err:#}"); + log::debug!("npm resolution failed. Trying again..."); + + // try again + let snapshot = get_new_snapshot(); + let result = pending_resolver + .resolve_pending(snapshot, package_reqs) + .await; + api.clear_memory_cache(); + // now surface the result after clearing the cache + result? + } + Err(err) => return Err(err.into()), } - Err(err) => return Err(err.into()), }; if let Some(lockfile_mutex) = maybe_lockfile { let mut lockfile = lockfile_mutex.lock(); populate_lockfile_from_snapshot(&mut lockfile, &snapshot)?; - Ok(snapshot) - } else { - Ok(snapshot) } + + Ok(snapshot) } fn get_npm_pending_resolver( diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index d46b6da6e..9ae84d7f9 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -4,6 +4,7 @@ mod common; mod global; mod local; +use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -153,7 +154,7 @@ impl CliNpmResolver { let Some(cache_folder_id) = self .fs_resolver .resolve_package_cache_folder_id_from_specifier(specifier)? else { -return Ok(None); + return Ok(None); }; Ok(Some( self @@ -229,6 +230,10 @@ return Ok(None); .unwrap() } + pub fn package_reqs(&self) -> HashMap<NpmPackageReq, NpmPackageNv> { + self.resolution.package_reqs() + } + pub fn snapshot(&self) -> NpmResolutionSnapshot { self.resolution.snapshot() } diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index dfa71cf6f..a090dd4fa 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -191,7 +191,7 @@ impl ModuleLoader for EmbeddedModuleLoader { } let module = module?; - let code = module.source().await.unwrap_or_default(); + let code = module.source().await.unwrap_or_else(|| Arc::new([])); let code = std::str::from_utf8(&code) .map_err(|_| type_error("Module source is not utf-8"))? .to_owned() @@ -204,6 +204,9 @@ impl ModuleLoader for EmbeddedModuleLoader { eszip::ModuleKind::Jsonc => { return Err(type_error("jsonc modules not supported")) } + eszip::ModuleKind::OpaqueData => { + unreachable!(); + } }, code, &module_specifier, diff --git a/cli/tests/integration/check_tests.rs b/cli/tests/integration/check_tests.rs index 1b00cadbe..04e5dedba 100644 --- a/cli/tests/integration/check_tests.rs +++ b/cli/tests/integration/check_tests.rs @@ -307,3 +307,80 @@ fn check_error_in_dep_then_fix() { output.assert_matches_text("Check [WILDCARD]main.ts\nerror: TS234[WILDCARD]"); output.assert_exit_code(1); } + +#[test] +fn json_module_check_then_error() { + let test_context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = test_context.temp_dir(); + let correct_code = "{ \"foo\": \"bar\" }"; + let incorrect_code = "{ \"foo2\": \"bar\" }"; + + temp_dir.write( + "main.ts", + "import test from './test.json' assert { type: 'json' }; console.log(test.foo);\n", + ); + temp_dir.write("test.json", correct_code); + + let check_command = test_context.new_command().args_vec(["check", "main.ts"]); + + check_command.run().assert_exit_code(0).skip_output_check(); + + temp_dir.write("test.json", incorrect_code); + check_command + .run() + .assert_matches_text("Check [WILDCARD]main.ts\nerror: TS2551[WILDCARD]") + .assert_exit_code(1); +} + +#[test] +fn npm_module_check_then_error() { + let test_context = TestContextBuilder::new() + .use_temp_cwd() + .add_npm_env_vars() + .use_http_server() + .build(); + let temp_dir = test_context.temp_dir(); + temp_dir.write("deno.json", "{}"); // so the lockfile gets loaded + + // get the lockfiles values first (this is necessary because the test + // server generates different tarballs based on the operating system) + test_context + .new_command() + .args_vec([ + "cache", + "npm:@denotest/breaking-change-between-versions@1.0.0", + "npm:@denotest/breaking-change-between-versions@2.0.0", + ]) + .run() + .skip_output_check(); + let lockfile = temp_dir.path().join("deno.lock"); + let mut lockfile_content = + lockfile.read_json::<deno_lockfile::LockfileContent>(); + + // make the specifier resolve to version 1 + lockfile_content.npm.specifiers.insert( + "@denotest/breaking-change-between-versions".to_string(), + "@denotest/breaking-change-between-versions@1.0.0".to_string(), + ); + lockfile.write_json(&lockfile_content); + temp_dir.write( + "main.ts", + "import { oldName } from 'npm:@denotest/breaking-change-between-versions'; console.log(oldName());\n", + ); + + let check_command = test_context.new_command().args_vec(["check", "main.ts"]); + check_command.run().assert_exit_code(0).skip_output_check(); + + // now update the lockfile to use version 2 instead, which should cause a + // type checking error because the oldName no longer exists + lockfile_content.npm.specifiers.insert( + "@denotest/breaking-change-between-versions".to_string(), + "@denotest/breaking-change-between-versions@2.0.0".to_string(), + ); + lockfile.write_json(&lockfile_content); + + check_command + .run() + .assert_matches_text("Check [WILDCARD]main.ts\nerror: TS2305[WILDCARD]has no exported member 'oldName'[WILDCARD]") + .assert_exit_code(1); +} diff --git a/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/index.d.ts b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/index.d.ts new file mode 100644 index 000000000..06dfef10d --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/index.d.ts @@ -0,0 +1 @@ +export function oldName(): 1;
\ No newline at end of file diff --git a/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/index.js b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/index.js new file mode 100644 index 000000000..1aca4250b --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/index.js @@ -0,0 +1,3 @@ +export function newName() { + return 1; +} diff --git a/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/package.json b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/package.json new file mode 100644 index 000000000..6eabea3af --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/1.0.0/package.json @@ -0,0 +1,6 @@ +{ + "name": "@denotest/breaking-change-between-versions", + "version": "1.0.0", + "type": "module", + "main": "index.js" +} diff --git a/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/index.d.ts b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/index.d.ts new file mode 100644 index 000000000..9a96451a5 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/index.d.ts @@ -0,0 +1 @@ +export function newName(): 2;
\ No newline at end of file diff --git a/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/index.js b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/index.js new file mode 100644 index 000000000..57626060d --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/index.js @@ -0,0 +1,3 @@ +export function newName() { + return 2; +} diff --git a/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/package.json b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/package.json new file mode 100644 index 000000000..ecd5970a4 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/breaking-change-between-versions/2.0.0/package.json @@ -0,0 +1,6 @@ +{ + "name": "@denotest/breaking-change-between-versions", + "version": "2.0.0", + "type": "module", + "main": "index.js" +} diff --git a/cli/tools/check.rs b/cli/tools/check.rs index 99d891e5d..aa4b9db0a 100644 --- a/cli/tools/check.rs +++ b/cli/tools/check.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; @@ -10,6 +11,8 @@ use deno_graph::Module; use deno_graph::ModuleGraph; use deno_runtime::colors; use deno_runtime::deno_node::NodeResolver; +use deno_semver::npm::NpmPackageNv; +use deno_semver::npm::NpmPackageReq; use once_cell::sync::Lazy; use regex::Regex; @@ -93,7 +96,12 @@ impl TypeChecker { let debug = self.cli_options.log_level() == Some(log::Level::Debug); let cache = TypeCheckCache::new(self.caches.type_checking_cache_db()); let check_js = ts_config.get_check_js(); - let check_hash = match get_check_hash(&graph, type_check_mode, &ts_config) { + let check_hash = match get_check_hash( + &graph, + self.npm_resolver.package_reqs(), + type_check_mode, + &ts_config, + ) { CheckHashResult::NoFiles => return Ok(()), CheckHashResult::Hash(hash) => hash, }; @@ -186,6 +194,7 @@ enum CheckHashResult { /// be used to tell fn get_check_hash( graph: &ModuleGraph, + package_reqs: HashMap<NpmPackageReq, NpmPackageNv>, type_check_mode: TypeCheckMode, ts_config: &TsConfig, ) -> CheckHashResult { @@ -198,11 +207,10 @@ fn get_check_hash( hasher.write(&ts_config.as_bytes()); let check_js = ts_config.get_check_js(); - let mut sorted_modules = graph.modules().collect::<Vec<_>>(); - sorted_modules.sort_by_key(|m| m.specifier().as_str()); // make it deterministic let mut has_file = false; let mut has_file_to_type_check = false; - for module in sorted_modules { + // this iterator of modules is already deterministic, so no need to sort it + for module in graph.modules() { match module { Module::Esm(module) => { let ts_check = has_ts_check(module.media_type, &module.source); @@ -240,15 +248,36 @@ fn get_check_hash( hasher.write_str(module.specifier.as_str()); hasher.write_str(&module.source); } - Module::Json(_) - | Module::External(_) - | Module::Node(_) - | Module::Npm(_) => { - // ignore + Module::Node(_) => { + // the @types/node package will be in the resolved + // snapshot below so don't bother including it here + } + Module::Npm(_) => { + // don't bother adding this specifier to the hash + // because what matters is the resolved npm snapshot, + // which is hashed below + } + Module::Json(module) => { + has_file_to_type_check = true; + hasher.write_str(module.specifier.as_str()); + hasher.write_str(&module.source); + } + Module::External(module) => { + hasher.write_str(module.specifier.as_str()); } } } + // Check if any of the top level npm pckages have changed. We could go + // further and check all the individual npm packages, but that's + // probably overkill. + let mut package_reqs = package_reqs.into_iter().collect::<Vec<_>>(); + package_reqs.sort_by(|a, b| a.0.cmp(&b.0)); // determinism + for (pkg_req, pkg_nv) in package_reqs { + hasher.write_hashable(&pkg_req); + hasher.write_hashable(&pkg_nv); + } + if !has_file || !check_js && !has_file_to_type_check { // no files to type check CheckHashResult::NoFiles diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 52883a0b3..cfc9bd952 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -28,6 +28,7 @@ use deno_core::ModuleSpecifier; use deno_core::OpState; use deno_core::RuntimeOptions; use deno_core::Snapshot; +use deno_graph::GraphKind; use deno_graph::Module; use deno_graph::ModuleGraph; use deno_graph::ResolutionResolved; @@ -319,7 +320,7 @@ pub struct Response { pub stats: Stats, } -#[derive(Debug, Default)] +#[derive(Debug)] struct State { hash_data: u64, graph: Arc<ModuleGraph>, @@ -331,6 +332,21 @@ struct State { current_dir: PathBuf, } +impl Default for State { + fn default() -> Self { + Self { + hash_data: Default::default(), + graph: Arc::new(ModuleGraph::new(GraphKind::All)), + maybe_tsbuildinfo: Default::default(), + maybe_response: Default::default(), + maybe_node_resolver: Default::default(), + remapped_specifiers: Default::default(), + root_map: Default::default(), + current_dir: Default::default(), + } + } +} + impl State { pub fn new( graph: Arc<ModuleGraph>, |