diff --git a/Cargo.lock b/Cargo.lock index b03837fa2d..e631742d59 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1035,9 +1035,9 @@ dependencies = [ [[package]] name = "deno_doc" -version = "0.63.1" +version = "0.64.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "499936300106c3c67caae87e29def3df5ea9385db6ed7428f154972f70ed39fa" +checksum = "c25d38dc94e4c2190bc7a1851790dd142fd0068888b3a26a1acb958db5113829" dependencies = [ "cfg-if", "deno_ast", @@ -1053,9 +1053,9 @@ dependencies = [ [[package]] name = "deno_emit" -version = "0.24.0" +version = "0.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6bdc024d2c1e5ec56ef6f923be2c2fea4662d596b0a68074ccf89991b38a05e7" +checksum = "0ef00ad78bc75e7f35a01bd4b5c32a97018eebc55c311e4b287e158a29021348" dependencies = [ "anyhow", "base64 0.13.1", @@ -1121,9 +1121,9 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.49.0" +version = "0.50.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a7e07fdff6c7dc1a9a7c03ce69435fda4b53641d2d6d3d3ed6d29cf67fefd3ea" +checksum = "73502c4d93a17f259b6edee6d5a5ba063e2fcdcdaeb6ca1c6953129cc14be6a7" dependencies = [ "anyhow", "data-url", @@ -1330,9 +1330,9 @@ dependencies = [ [[package]] name = "deno_npm" -version = "0.9.1" +version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "371ef0398b5b5460d66b78a958d5015658e198ad3a29fb9ce329459272fd29aa" +checksum = "fa5d1097de53e8ce3316d3e44095e253719ae367cf7478263f83082f44dddabf" dependencies = [ "anyhow", "async-trait", @@ -1340,7 +1340,6 @@ dependencies = [ "futures", "log", "monch", - "once_cell", "serde", "thiserror", ] @@ -1424,11 +1423,12 @@ dependencies = [ [[package]] name = "deno_semver" -version = "0.2.2" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "242c8ad9f4ce614ec0fa2e6b3d834f2662ce024ca78e9ed4c58d812cbfc3e41d" +checksum = "96f99990457915af1f444900003ffd5a9d3ab2e5337b06d681e56ca371b3e11f" dependencies = [ "monch", + "once_cell", "serde", "thiserror", "url", @@ -1944,9 +1944,9 @@ dependencies = [ [[package]] name = "eszip" -version = "0.45.0" +version = "0.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bbf5a0f47c2e73cb7631accc282ebda9bc9ed9b2034abfddec983dc9c8f78e7a" +checksum = "58d2382a70396ccffd260adb879ed9f954c45457e8075830c7d4ec30c6dc8ad2" dependencies = [ "anyhow", "base64 0.21.0", diff --git a/Cargo.toml b/Cargo.toml index 491755d41e..c65da291c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,8 +51,8 @@ deno_bench_util = { version = "0.107.0", path = "./bench_util" } test_util = { path = "./test_util" } deno_lockfile = "0.14.1" deno_media_type = { version = "0.1.0", features = ["module_specifier"] } -deno_npm = "0.9.1" -deno_semver = "0.2.2" +deno_npm = "0.10.1" +deno_semver = "0.3.0" # exts deno_broadcast_channel = { version = "0.107.0", path = "./ext/broadcast_channel" } diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 28b4a0957b..4c000ea55d 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 a8c6eaad45..76a09eadd6 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 bfba1d67de..6992d875e0 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 { + 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 { 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); - } + 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..."); - 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? + // 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 d46b6da6ef..9ae84d7f90 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 { + 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 dfa71cf6f7..a090dd4fa0 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 1b00cadbee..04e5dedba8 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::(); + + // 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 0000000000..06dfef10db --- /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 0000000000..1aca4250ba --- /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 0000000000..6eabea3aff --- /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 0000000000..9a96451a56 --- /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 0000000000..57626060de --- /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 0000000000..ecd5970a46 --- /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 99d891e5d0..aa4b9db0a2 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, 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::>(); - 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::>(); + 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 52883a0b31..cfc9bd9522 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, @@ -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, diff --git a/test_util/src/fs.rs b/test_util/src/fs.rs index 2a64fb9c48..005c467a6c 100644 --- a/test_util/src/fs.rs +++ b/test_util/src/fs.rs @@ -10,6 +10,8 @@ use std::sync::Arc; use anyhow::Context; use lsp_types::Url; +use serde::de::DeserializeOwned; +use serde::Serialize; use crate::assertions::assert_wildcard_match; @@ -110,6 +112,10 @@ impl PathRef { .with_context(|| format!("Could not read file: {}", self)) } + pub fn read_json(&self) -> TValue { + serde_json::from_str(&self.read_to_string()).unwrap() + } + pub fn rename(&self, to: impl AsRef) { fs::rename(self, self.join(to)).unwrap(); } @@ -118,6 +124,11 @@ impl PathRef { fs::write(self, text.as_ref()).unwrap(); } + pub fn write_json(&self, value: &TValue) { + let text = serde_json::to_string_pretty(value).unwrap(); + self.write(text); + } + pub fn symlink_dir( &self, oldpath: impl AsRef,