From bf4f13c67fda72f18575f5159668cc0e16fd38f0 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 5 Sep 2024 15:45:16 +0200 Subject: [PATCH] BREAKING: remove "emit" and "map" from deno info output --- Cargo.lock | 4 +- cli/Cargo.toml | 2 +- cli/cache/emit.rs | 13 ---- cli/cache/mod.rs | 60 ++++++++----------- cli/factory.rs | 1 - cli/graph_util.rs | 4 -- cli/lsp/documents.rs | 10 +++- cli/schemas/module-graph.json | 8 --- cli/tools/info.rs | 16 ----- cli/util/path.rs | 27 +++++++++ tests/integration/info_tests.rs | 25 -------- tests/specs/info/multiple_redirects/main.out | 4 -- .../npm/cjs_with_deps/main_info_json.out | 2 - .../main_info_json.out | 2 - 14 files changed, 63 insertions(+), 115 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd3cc350c97a70..4ba940ad5a3dc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1582,9 +1582,9 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.82.0" +version = "0.82.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "646757b109993751f618d20de9bafc17f8f886fa910fb82c2c89b9e1df220ac6" +checksum = "78b63015c73aa203da206b5d35b4c1eaa23bc7fed37ab325da62d525a5524a04" dependencies = [ "anyhow", "async-trait", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index d9c3e21ad3898f..efe3f2ce508df1 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -68,7 +68,7 @@ deno_cache_dir = { workspace = true } deno_config = { version = "=0.33.2", features = ["workspace", "sync"] } deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } deno_doc = { version = "0.148.0", features = ["html", "syntect"] } -deno_graph = { version = "=0.82.0" } +deno_graph = { version = "=0.82.1" } deno_lint = { version = "=0.64.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "=0.25.0" diff --git a/cli/cache/emit.rs b/cli/cache/emit.rs index 5e89f9a90a10e7..6807f06c1063e4 100644 --- a/cli/cache/emit.rs +++ b/cli/cache/emit.rs @@ -82,19 +82,6 @@ impl EmitCache { Ok(()) } - /// Gets the filepath which stores the emit. - pub fn get_emit_filepath( - &self, - specifier: &ModuleSpecifier, - ) -> Option { - Some( - self - .disk_cache - .location - .join(self.get_emit_filename(specifier)?), - ) - } - fn get_emit_filename(&self, specifier: &ModuleSpecifier) -> Option { self .disk_cache diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index 4500e3a4aced40..3b4e277607eb1f 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -8,6 +8,7 @@ use crate::file_fetcher::FileFetcher; use crate::file_fetcher::FileOrRedirect; use crate::npm::CliNpmResolver; use crate::util::fs::atomic_write_file_with_retries; +use crate::util::path::specifier_has_extension; use deno_ast::MediaType; use deno_core::futures; @@ -106,7 +107,6 @@ pub use deno_cache_dir::HttpCache; /// A "wrapper" for the FileFetcher and DiskCache for the Deno CLI that provides /// a concise interface to the DENO_DIR when building module graphs. pub struct FetchCacher { - emit_cache: Arc, file_fetcher: Arc, file_header_overrides: HashMap>, global_http_cache: Arc, @@ -118,7 +118,6 @@ pub struct FetchCacher { impl FetchCacher { pub fn new( - emit_cache: Arc, file_fetcher: Arc, file_header_overrides: HashMap>, global_http_cache: Arc, @@ -127,7 +126,6 @@ impl FetchCacher { permissions: PermissionsContainer, ) -> Self { Self { - emit_cache, file_fetcher, file_header_overrides, global_http_cache, @@ -144,15 +142,7 @@ impl FetchCacher { self.cache_info_enabled = true; } - // DEPRECATED: Where the file is stored and how it's stored should be an implementation - // detail of the cache. - // - // todo(dsheret): remove once implementing - // * https://github.com/denoland/deno/issues/17707 - // * https://github.com/denoland/deno/issues/17703 - #[deprecated( - note = "There should not be a way to do this because the file may not be cached at a local path in the future." - )] + /// Only use this for `deno info`. fn get_local_path(&self, specifier: &ModuleSpecifier) -> Option { // TODO(@kitsonk) fix when deno_graph does not query cache for synthetic // modules @@ -179,15 +169,7 @@ impl Loader for FetchCacher { #[allow(deprecated)] let local = self.get_local_path(specifier)?; if local.is_file() { - let emit = self - .emit_cache - .get_emit_filepath(specifier) - .filter(|p| p.is_file()); - Some(CacheInfo { - local: Some(local), - emit, - map: None, - }) + Some(CacheInfo { local: Some(local) }) } else { None } @@ -200,19 +182,28 @@ impl Loader for FetchCacher { ) -> LoadFuture { use deno_graph::source::CacheSetting as LoaderCacheSetting; - if specifier.scheme() == "file" - && specifier.path().contains("/node_modules/") - { - // The specifier might be in a completely different symlinked tree than - // what the node_modules url is in (ex. `/my-project-1/node_modules` - // symlinked to `/my-project-2/node_modules`), so first we checked if the path - // is in a node_modules dir to avoid needlessly canonicalizing, then now compare - // against the canonicalized specifier. - let specifier = - crate::node::resolve_specifier_into_node_modules(specifier); - if self.npm_resolver.in_npm_package(&specifier) { + if specifier.scheme() == "file" { + if specifier.path().contains("/node_modules/") { + // The specifier might be in a completely different symlinked tree than + // what the node_modules url is in (ex. `/my-project-1/node_modules` + // symlinked to `/my-project-2/node_modules`), so first we checked if the path + // is in a node_modules dir to avoid needlessly canonicalizing, then now compare + // against the canonicalized specifier. + let specifier = + crate::node::resolve_specifier_into_node_modules(specifier); + if self.npm_resolver.in_npm_package(&specifier) { + return Box::pin(futures::future::ready(Ok(Some( + LoadResponse::External { specifier }, + )))); + } + } + + // make local CJS modules external to the graph + if specifier_has_extension(specifier, "cjs") { return Box::pin(futures::future::ready(Ok(Some( - LoadResponse::External { specifier }, + LoadResponse::External { + specifier: specifier.clone(), + }, )))); } } @@ -293,6 +284,7 @@ impl Loader for FetchCacher { fn cache_module_info( &self, specifier: &ModuleSpecifier, + media_type: MediaType, source: &Arc<[u8]>, module_info: &deno_graph::ModuleInfo, ) { @@ -300,7 +292,7 @@ impl Loader for FetchCacher { let source_hash = CacheDBHash::from_source(source); let result = self.module_info_cache.set_module_info( specifier, - MediaType::from_specifier(specifier), + media_type, source_hash, module_info, ); diff --git a/cli/factory.rs b/cli/factory.rs index 81212c7885208a..a51bc771ead613 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -616,7 +616,6 @@ impl CliFactory { self.parsed_source_cache().clone(), cli_options.maybe_lockfile().cloned(), self.maybe_file_watcher_reporter().clone(), - self.emit_cache()?.clone(), self.file_fetcher()?.clone(), self.global_http_cache()?.clone(), ))) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index d7f007a7c37e37..d73733123f72ab 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -368,7 +368,6 @@ pub struct ModuleGraphBuilder { parsed_source_cache: Arc, lockfile: Option>, maybe_file_watcher_reporter: Option, - emit_cache: Arc, file_fetcher: Arc, global_http_cache: Arc, } @@ -385,7 +384,6 @@ impl ModuleGraphBuilder { parsed_source_cache: Arc, lockfile: Option>, maybe_file_watcher_reporter: Option, - emit_cache: Arc, file_fetcher: Arc, global_http_cache: Arc, ) -> Self { @@ -399,7 +397,6 @@ impl ModuleGraphBuilder { parsed_source_cache, lockfile, maybe_file_watcher_reporter, - emit_cache, file_fetcher, global_http_cache, } @@ -681,7 +678,6 @@ impl ModuleGraphBuilder { permissions: PermissionsContainer, ) -> cache::FetchCacher { cache::FetchCacher::new( - self.emit_cache.clone(), self.file_fetcher.clone(), self.options.resolve_file_header_overrides(), self.global_http_cache.clone(), diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index ee9a5229747d0b..d6af96b2380daf 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1524,12 +1524,16 @@ impl<'a> deno_graph::source::Loader for OpenDocumentsGraphLoader<'a> { fn cache_module_info( &self, specifier: &deno_ast::ModuleSpecifier, + media_type: MediaType, source: &Arc<[u8]>, module_info: &deno_graph::ModuleInfo, ) { - self - .inner_loader - .cache_module_info(specifier, source, module_info) + self.inner_loader.cache_module_info( + specifier, + media_type, + source, + module_info, + ) } } diff --git a/cli/schemas/module-graph.json b/cli/schemas/module-graph.json index b761baddedb3d7..ea32d843be28f5 100644 --- a/cli/schemas/module-graph.json +++ b/cli/schemas/module-graph.json @@ -83,14 +83,6 @@ "type": "string", "description": "The checksum of the local source file. This can be used to validate if the current on disk version matches the version described here." }, - "emit": { - "type": "string", - "description": "The path to an emitted version of the module, if the module requires transpilation to be loaded into the Deno runtime." - }, - "map": { - "type": "string", - "description": "The path to an optionally emitted source map between the original and emitted version of the file." - }, "error": { "type": "string", "description": "If when resolving the module, Deno encountered an error and the module is unavailable, the text of that error will be indicated here." diff --git a/cli/tools/info.rs b/cli/tools/info.rs index 6aa044a924d05a..d78b83cbe3bd0b 100644 --- a/cli/tools/info.rs +++ b/cli/tools/info.rs @@ -461,22 +461,6 @@ impl<'a> GraphDisplayContext<'a> { local.to_string_lossy() )?; } - if let Some(emit) = &cache_info.emit { - writeln!( - writer, - "{} {}", - colors::bold("emit:"), - emit.to_string_lossy() - )?; - } - if let Some(map) = &cache_info.map { - writeln!( - writer, - "{} {}", - colors::bold("map:"), - map.to_string_lossy() - )?; - } } if let Some(module) = root.js() { writeln!(writer, "{} {}", colors::bold("type:"), module.media_type)?; diff --git a/cli/util/path.rs b/cli/util/path.rs index 804b26f65f7219..6f09cf1eacb9f7 100644 --- a/cli/util/path.rs +++ b/cli/util/path.rs @@ -42,6 +42,21 @@ pub fn get_extension(file_path: &Path) -> Option { .map(|e| e.to_lowercase()); } +pub fn specifier_has_extension( + specifier: &ModuleSpecifier, + searching_ext: &str, +) -> bool { + let Some((_, ext)) = specifier.path().rsplit_once('.') else { + return false; + }; + let searching_ext = searching_ext.strip_prefix('.').unwrap_or(searching_ext); + debug_assert!(!searching_ext.contains('.')); // exts like .d.ts are not implemented here + if ext.len() != searching_ext.len() { + return false; + } + ext.eq_ignore_ascii_case(searching_ext) +} + pub fn get_atomic_dir_path(file_path: &Path) -> PathBuf { let rand = gen_rand_path_component(); let new_file_name = format!( @@ -377,6 +392,18 @@ mod test { } } + #[test] + fn test_specifier_has_extension() { + fn get(specifier: &str, ext: &str) -> bool { + specifier_has_extension(&ModuleSpecifier::parse(specifier).unwrap(), ext) + } + + assert!(get("file:///a/b/c.ts", "ts")); + assert!(get("file:///a/b/c.ts", ".ts")); + assert!(!get("file:///a/b/c.ts", ".cts")); + assert!(get("file:///a/b/c.CtS", ".cts")); + } + #[test] fn test_to_percent_decoded_str() { let str = to_percent_decoded_str("%F0%9F%A6%95"); diff --git a/tests/integration/info_tests.rs b/tests/integration/info_tests.rs index b22209a565d906..38dd9448f4c902 100644 --- a/tests/integration/info_tests.rs +++ b/tests/integration/info_tests.rs @@ -5,31 +5,6 @@ use test_util::itest; // use util::env_vars_for_npm_tests; use util::TestContextBuilder; -#[test] -fn info_with_compiled_source() { - let context = TestContextBuilder::new().use_http_server().build(); - let module_path = "http://127.0.0.1:4545/run/048_media_types_jsx.ts"; - - let output = context - .new_command() - .current_dir(util::testdata_path()) - .args_vec(["cache", module_path]) - .run(); - output.assert_exit_code(0); - output.skip_output_check(); - - let output = context - .new_command() - .current_dir(util::testdata_path()) - .args_vec(["info", module_path]) - .split_output() - .run(); - - // check the output of the test.ts program. - assert!(output.stdout().trim().contains("emit: ")); - assert_eq!(output.stderr(), ""); -} - #[test] fn info_lock_write() { let context = TestContextBuilder::new().use_http_server().build(); diff --git a/tests/specs/info/multiple_redirects/main.out b/tests/specs/info/multiple_redirects/main.out index 31123be778dc60..9cc1066a93b23c 100644 --- a/tests/specs/info/multiple_redirects/main.out +++ b/tests/specs/info/multiple_redirects/main.out @@ -27,8 +27,6 @@ Download http://localhost:4545/subdir/redirects/redirect1.js } ], "local": "[WILDLINE]main.ts", - "emit": null, - "map": null, "size": 97, "mediaType": "TypeScript", "specifier": "file:///[WILDLINE]/multiple_redirects/main.ts" @@ -36,8 +34,6 @@ Download http://localhost:4545/subdir/redirects/redirect1.js { "kind": "esm", "local": "[WILDLINE]", - "emit": null, - "map": null, "size": 27, "mediaType": "JavaScript", "specifier": "http://localhost:4545/subdir/redirects/redirect1.js" diff --git a/tests/testdata/npm/cjs_with_deps/main_info_json.out b/tests/testdata/npm/cjs_with_deps/main_info_json.out index 4d8c1a5bea5986..af1ef1351880a7 100644 --- a/tests/testdata/npm/cjs_with_deps/main_info_json.out +++ b/tests/testdata/npm/cjs_with_deps/main_info_json.out @@ -42,8 +42,6 @@ } ], "local": "[WILDCARD]main.js", - "emit": null, - "map": null, "size": 325, "mediaType": "JavaScript", "specifier": "[WILDCARD]/main.js" diff --git a/tests/testdata/npm/peer_deps_with_copied_folders/main_info_json.out b/tests/testdata/npm/peer_deps_with_copied_folders/main_info_json.out index 48cb1f992037c3..df6541aa8e83ad 100644 --- a/tests/testdata/npm/peer_deps_with_copied_folders/main_info_json.out +++ b/tests/testdata/npm/peer_deps_with_copied_folders/main_info_json.out @@ -42,8 +42,6 @@ } ], "local": "[WILDCARD]main.ts", - "emit": null, - "map": null, "size": 171, "mediaType": "TypeScript", "specifier": "file://[WILDCARD]/main.ts"