Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING: remove "emit" and "map" from deno info output #25468

Merged
merged 1 commit into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 0 additions & 13 deletions cli/cache/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,19 +82,6 @@ impl EmitCache {
Ok(())
}

/// Gets the filepath which stores the emit.
pub fn get_emit_filepath(
&self,
specifier: &ModuleSpecifier,
) -> Option<PathBuf> {
Some(
self
.disk_cache
.location
.join(self.get_emit_filename(specifier)?),
)
}

fn get_emit_filename(&self, specifier: &ModuleSpecifier) -> Option<PathBuf> {
self
.disk_cache
Expand Down
60 changes: 26 additions & 34 deletions cli/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<EmitCache>,
file_fetcher: Arc<FileFetcher>,
file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
global_http_cache: Arc<GlobalHttpCache>,
Expand All @@ -118,7 +118,6 @@ pub struct FetchCacher {

impl FetchCacher {
pub fn new(
emit_cache: Arc<EmitCache>,
file_fetcher: Arc<FileFetcher>,
file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
global_http_cache: Arc<GlobalHttpCache>,
Expand All @@ -127,7 +126,6 @@ impl FetchCacher {
permissions: PermissionsContainer,
) -> Self {
Self {
emit_cache,
file_fetcher,
file_header_overrides,
global_http_cache,
Expand All @@ -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<PathBuf> {
// TODO(@kitsonk) fix when deno_graph does not query cache for synthetic
// modules
Expand All @@ -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
}
Expand All @@ -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(),
},
))));
}
}
Expand Down Expand Up @@ -293,14 +284,15 @@ impl Loader for FetchCacher {
fn cache_module_info(
&self,
specifier: &ModuleSpecifier,
media_type: MediaType,
source: &Arc<[u8]>,
module_info: &deno_graph::ModuleInfo,
) {
log::debug!("Caching module info for {}", specifier);
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,
);
Expand Down
1 change: 0 additions & 1 deletion cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)))
Expand Down
4 changes: 0 additions & 4 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ pub struct ModuleGraphBuilder {
parsed_source_cache: Arc<ParsedSourceCache>,
lockfile: Option<Arc<CliLockfile>>,
maybe_file_watcher_reporter: Option<FileWatcherReporter>,
emit_cache: Arc<cache::EmitCache>,
file_fetcher: Arc<FileFetcher>,
global_http_cache: Arc<GlobalHttpCache>,
}
Expand All @@ -385,7 +384,6 @@ impl ModuleGraphBuilder {
parsed_source_cache: Arc<ParsedSourceCache>,
lockfile: Option<Arc<CliLockfile>>,
maybe_file_watcher_reporter: Option<FileWatcherReporter>,
emit_cache: Arc<cache::EmitCache>,
file_fetcher: Arc<FileFetcher>,
global_http_cache: Arc<GlobalHttpCache>,
) -> Self {
Expand All @@ -399,7 +397,6 @@ impl ModuleGraphBuilder {
parsed_source_cache,
lockfile,
maybe_file_watcher_reporter,
emit_cache,
file_fetcher,
global_http_cache,
}
Expand Down Expand Up @@ -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(),
Expand Down
10 changes: 7 additions & 3 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}
}

Expand Down
8 changes: 0 additions & 8 deletions cli/schemas/module-graph.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
16 changes: 0 additions & 16 deletions cli/tools/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
27 changes: 27 additions & 0 deletions cli/util/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ pub fn get_extension(file_path: &Path) -> Option<String> {
.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!(
Expand Down Expand Up @@ -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");
Expand Down
25 changes: 0 additions & 25 deletions tests/integration/info_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 0 additions & 4 deletions tests/specs/info/multiple_redirects/main.out
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,13 @@ 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"
},
{
"kind": "esm",
"local": "[WILDLINE]",
"emit": null,
"map": null,
"size": 27,
"mediaType": "JavaScript",
"specifier": "http://localhost:4545/subdir/redirects/redirect1.js"
Expand Down
2 changes: 0 additions & 2 deletions tests/testdata/npm/cjs_with_deps/main_info_json.out
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@
}
],
"local": "[WILDCARD]main.js",
"emit": null,
"map": null,
"size": 325,
"mediaType": "JavaScript",
"specifier": "[WILDCARD]/main.js"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@
}
],
"local": "[WILDCARD]main.ts",
"emit": null,
"map": null,
"size": 171,
"mediaType": "TypeScript",
"specifier": "file://[WILDCARD]/main.ts"
Expand Down