From fadf48aecdf5c351f411f2457b102719586afc24 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 3 Feb 2022 18:25:54 -0500 Subject: [PATCH 01/33] Committing for backup purposes. --- cli/flags.rs | 50 +++++ cli/main.rs | 13 ++ cli/tools/mod.rs | 1 + cli/tools/vendor/mod.rs | 133 ++++++++++++ cli/tools/vendor/specifiers.rs | 380 +++++++++++++++++++++++++++++++++ 5 files changed, 577 insertions(+) create mode 100644 cli/tools/vendor/mod.rs create mode 100644 cli/tools/vendor/specifiers.rs diff --git a/cli/flags.rs b/cli/flags.rs index d4ba685fb87ed..0c26018367a5b 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -161,6 +161,13 @@ pub struct UpgradeFlags { pub ca_file: Option, } +#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] +pub struct VendorFlags { + pub entry_points: Vec, + pub output_path: Option, + pub force: bool, +} + #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] pub enum DenoSubcommand { Bundle(BundleFlags), @@ -181,6 +188,7 @@ pub enum DenoSubcommand { Test(TestFlags), Types, Upgrade(UpgradeFlags), + Vendor(VendorFlags), } impl Default for DenoSubcommand { @@ -482,6 +490,7 @@ pub fn flags_from_vec(args: Vec) -> clap::Result { Some(("lint", m)) => lint_parse(&mut flags, m), Some(("compile", m)) => compile_parse(&mut flags, m), Some(("lsp", m)) => lsp_parse(&mut flags, m), + Some(("vendor", m)) => vendor_parse(&mut flags, m), _ => handle_repl_flags(&mut flags, ReplFlags { eval: None }), } @@ -553,6 +562,7 @@ If the flag is set, restrict these messages to errors.", .subcommand(test_subcommand()) .subcommand(types_subcommand()) .subcommand(upgrade_subcommand()) + .subcommand(vendor_subcommand()) .long_about(DENO_HELP) .after_help(ENV_VARIABLES_HELP) } @@ -1402,6 +1412,35 @@ update to a different location, use the --output flag .arg(ca_file_arg()) } +fn vendor_subcommand<'a>() -> App<'a> { + // todo(THIS PR): write tests for these once finalized + App::new("vendor") + .about("Vendor remote modules into a local directory") + .long_about("") + .arg( + Arg::new("entry_points") + .takes_value(true) + .multiple_values(true) + .multiple_occurrences(true) + .required(true), + ) + .arg( + Arg::new("output") + .long("output") + .help("The directory to output the vendored modules to") + .takes_value(true), + ) + .arg( + Arg::new("force") + .long("force") + .short('f') + .help( + "Forcefully overwrite conflicting files in existing output directory", + ) + .takes_value(false), + ) +} + fn compile_args(app: App) -> App { app .arg(import_map_arg()) @@ -2221,6 +2260,17 @@ fn upgrade_parse(flags: &mut Flags, matches: &clap::ArgMatches) { }); } +fn vendor_parse(flags: &mut Flags, matches: &clap::ArgMatches) { + flags.subcommand = DenoSubcommand::Vendor(VendorFlags { + entry_points: matches + .values_of("entry_points") + .map(|p| p.map(ToString::to_string).collect()) + .unwrap_or_default(), + output_path: matches.value_of("output").map(PathBuf::from), + force: matches.is_present("force"), + }); +} + fn compile_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) { import_map_arg_parse(flags, matches); no_remote_arg_parse(flags, matches); diff --git a/cli/main.rs b/cli/main.rs index 0e9a57382c9fa..3d3297bea8add 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -57,6 +57,7 @@ use crate::flags::RunFlags; use crate::flags::TestFlags; use crate::flags::UninstallFlags; use crate::flags::UpgradeFlags; +use crate::flags::VendorFlags; use crate::fmt_errors::PrettyJsError; use crate::graph_util::graph_lock_or_exit; use crate::graph_util::graph_valid; @@ -1272,6 +1273,15 @@ async fn upgrade_command( Ok(0) } +async fn vendor_command( + flags: Flags, + vendor_flags: VendorFlags, +) -> Result { + let ps = ProcState::build(flags).await?; + tools::vendor::vendor(ps, vendor_flags).await?; + Ok(0) +} + fn init_v8_flags(v8_flags: &[String]) { let v8_flags_includes_help = v8_flags .iter() @@ -1350,6 +1360,9 @@ fn get_subcommand( DenoSubcommand::Upgrade(upgrade_flags) => { upgrade_command(flags, upgrade_flags).boxed_local() } + DenoSubcommand::Vendor(vendor_flags) => { + vendor_command(flags, vendor_flags).boxed_local() + } } } diff --git a/cli/tools/mod.rs b/cli/tools/mod.rs index 83c501ddebd0b..ffea76e1d938d 100644 --- a/cli/tools/mod.rs +++ b/cli/tools/mod.rs @@ -9,3 +9,4 @@ pub mod repl; pub mod standalone; pub mod test; pub mod upgrade; +pub mod vendor; diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs new file mode 100644 index 0000000000000..52e97be42da06 --- /dev/null +++ b/cli/tools/vendor/mod.rs @@ -0,0 +1,133 @@ +use std::collections::HashMap; +use std::collections::HashSet; +use std::path::PathBuf; + +use deno_core::anyhow::bail; +use deno_core::error::AnyError; +use deno_core::resolve_url_or_path; +use deno_runtime::permissions::Permissions; + +use crate::flags::VendorFlags; +use crate::lockfile; +use crate::proc_state::ProcState; +use crate::resolver::ImportMapResolver; +use crate::resolver::JsxResolver; + +use self::specifiers::dir_name_for_root; +use self::specifiers::get_unique_path; +use self::specifiers::make_url_relative; +use self::specifiers::partition_by_root_specifiers; +use self::specifiers::sanitize_filepath; + +mod specifiers; + +pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { + let output_dir = resolve_and_validate_output_dir(&flags)?; + let graph = create_graph(&ps, &flags).await?; + let remote_modules = graph + .modules() + .into_iter() + .filter(|m| m.specifier.scheme().starts_with("http")) + .collect::>(); + let partitioned_specifiers = + partition_by_root_specifiers(remote_modules.iter().map(|m| &m.specifier)); + let mut mapped_paths = HashSet::new(); + let mut mappings = HashMap::new(); + + for (root, specifiers) in partitioned_specifiers.into_iter() { + let base_dir = get_unique_path( + output_dir.join(dir_name_for_root(&root, &specifiers)), + &mut mapped_paths, + ); + for specifier in specifiers { + let media_type = graph.get(&specifier).unwrap().media_type; + let relative = base_dir + .join(sanitize_filepath(&make_url_relative(&root, &specifier)?)) + .with_extension(&media_type.as_ts_extension()[1..]); + mappings.insert(specifier, get_unique_path(relative, &mut mapped_paths)); + } + } + + Ok(()) +} + +fn resolve_and_validate_output_dir( + flags: &VendorFlags, +) -> Result { + let output_dir = match &flags.output_path { + Some(output_path) => output_path.clone(), + None => std::env::current_dir()?.join("vendor"), + }; + if !flags.force { + if let Ok(mut dir) = std::fs::read_dir(&output_dir) { + if dir.next().is_some() { + bail!("Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", output_dir.display()); + } + } + } + Ok(output_dir) +} + +async fn create_graph( + ps: &ProcState, + flags: &VendorFlags, +) -> Result { + let entry_points = flags + .entry_points + .iter() + .map(|p| { + let url = resolve_url_or_path(p)?; + Ok((url, deno_graph::ModuleKind::Esm)) + }) + .collect::, AnyError>>()?; + + // todo(dsherret): there is a lot of copy and paste here from + // other parts of the codebase and we should resolve this + // code duplication in a future PR + let mut cache = crate::cache::FetchCacher::new( + ps.dir.gen_cache.clone(), + ps.file_fetcher.clone(), + Permissions::allow_all(), + Permissions::allow_all(), + ); + let maybe_locker = lockfile::as_maybe_locker(ps.lockfile.clone()); + let maybe_imports = if let Some(config_file) = &ps.maybe_config_file { + config_file.to_maybe_imports()? + } else { + None + }; + let maybe_import_map_resolver = + ps.maybe_import_map.clone().map(ImportMapResolver::new); + let maybe_jsx_resolver = ps + .maybe_config_file + .as_ref() + .map(|cf| { + cf.to_maybe_jsx_import_source_module() + .map(|im| JsxResolver::new(im, maybe_import_map_resolver.clone())) + }) + .flatten(); + let maybe_resolver = if maybe_jsx_resolver.is_some() { + maybe_jsx_resolver.as_ref().map(|jr| jr.as_resolver()) + } else { + maybe_import_map_resolver + .as_ref() + .map(|im| im.as_resolver()) + }; + + let graph = deno_graph::create_graph( + entry_points, + false, + maybe_imports, + &mut cache, + maybe_resolver, + maybe_locker, + None, + None, + ) + .await; + + graph.lock()?; + graph.valid()?; + + Ok(graph) +} diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs new file mode 100644 index 0000000000000..f823977a71a05 --- /dev/null +++ b/cli/tools/vendor/specifiers.rs @@ -0,0 +1,380 @@ +use std::collections::HashSet; +use std::path::Path; +use std::path::PathBuf; + +use deno_ast::ModuleSpecifier; +use deno_core::anyhow::anyhow; +use deno_core::error::AnyError; + +/// Partitions the provided specifiers by specifiers that do not have a +/// parent specifier. +pub fn partition_by_root_specifiers<'a>( + specifiers: impl Iterator, +) -> Vec<(ModuleSpecifier, Vec)> { + let mut root_specifiers: Vec<(ModuleSpecifier, Vec)> = + Vec::new(); + for remote_specifier in specifiers { + let mut found = false; + for (root_specifier, specifiers) in root_specifiers.iter_mut() { + if let Some(relative_url) = root_specifier.make_relative(remote_specifier) + { + // found a new root + if relative_url.starts_with("../") { + let end_ancestor_index = + relative_url.len() - relative_url.trim_start_matches("../").len(); + *root_specifier = root_specifier + .join(&relative_url[..end_ancestor_index]) + .unwrap(); + } + + specifiers.push(remote_specifier.clone()); + found = true; + break; + } + } + if !found { + // get the specifier without the directory + let root_specifier = remote_specifier + .join("./") + .unwrap_or_else(|_| remote_specifier.clone()); + root_specifiers.push((root_specifier, vec![remote_specifier.clone()])); + } + } + root_specifiers +} + +/// Gets the flattened directory name to use for the provided root +/// specifier and its descendant specifiers. We use the descendant +/// specifiers to estimate the maximum directory path length in +/// order to truncate the root directory name if necessary due to +/// the 260 character max path length on Windows. +pub fn dir_name_for_root( + root: &ModuleSpecifier, + specifiers: &[ModuleSpecifier], +) -> PathBuf { + // all the provided specifiers should be descendants of the root + debug_assert!(specifiers + .iter() + .all(|s| s.as_str().starts_with(root.as_str()))); + + let mut result = String::new(); + if let Some(domain) = root.domain() { + result.push_str(&sanitize_segment(domain)); + } + if let Some(port) = root.port() { + if !result.is_empty() { + result.push('_'); + } + result.push_str(&port.to_string()); + } + if let Some(segments) = root.path_segments() { + for segment in segments.filter(|s| !s.is_empty()) { + if !result.is_empty() { + result.push('_'); + } + result.push_str(&sanitize_segment(segment)); + } + } + + PathBuf::from(if result.is_empty() { + "unknown".to_string() + } else { + // Limit the size of the directory to reduce the chance of max path + // errors on Windows. This uses bytes instead of chars because it's + // faster, but the approximation should be good enough. + let root_len = root.as_str().len(); + let max_specifier_len = specifiers + .iter() + .map(|s| s.as_str().len()) + .max() + .unwrap_or(root_len); + let sub_path_len = max_specifier_len - root_len; + let max_win_path = 260; + // This is the approximate length that a path might be before the root directory. + // It should be a hardcoded number and not calculated based on the system as the + // produced code will most likely not stay only on the system that produced it. + let approx_path_prefix_len = 80; + let truncate_len = std::cmp::max( + 10, + max_win_path as isize + - approx_path_prefix_len as isize + - sub_path_len as isize, + ) as usize; + + truncate_str(&result, truncate_len) + .trim_end_matches('_') + .trim_end_matches('.') + .to_string() + }) +} + +/// Gets a path with the specified file stem suffix. +pub fn path_with_stem_suffix(path: &Path, suffix: &str) -> PathBuf { + if let Some(file_name) = path.file_name().map(|f| f.to_string_lossy()) { + if let Some(file_stem) = path.file_stem().map(|f| f.to_string_lossy()) { + if let Some(ext) = path.extension().map(|f| f.to_string_lossy()) { + return path + .with_file_name(format!("{}_{}.{}", file_stem, suffix, ext)); + } + } + + path.with_file_name(format!("{}_{}", file_name, suffix)) + } else { + path.with_file_name(suffix) + } +} + +/// Gets a unique file path given the provided file path +/// and the set of existing file paths. Inserts to the +/// set when finding a unique path. +pub fn get_unique_path( + mut path: PathBuf, + unique_set: &mut HashSet, +) -> PathBuf { + let original_path = path.clone(); + let mut count = 2; + // case insensitive comparison for case insensitive file systems + while !unique_set.insert(path.to_string_lossy().to_lowercase()) { + path = path_with_stem_suffix(&original_path, &count.to_string()); + count += 1; + } + path +} + +pub fn make_url_relative( + root: &ModuleSpecifier, + url: &ModuleSpecifier, +) -> Result { + let mut url = url.clone(); + url.set_query(None); + root.make_relative(&url).ok_or_else(|| { + anyhow!( + "Error making url ({}) relative to root: {}", + url.to_string(), + root.to_string() + ) + }) +} + +fn truncate_str(text: &str, max: usize) -> &str { + match text.char_indices().nth(max) { + Some((i, _)) => &text[..i], + None => text, + } +} + +pub fn sanitize_filepath(text: &str) -> String { + text + .chars() + .map(|c| if is_banned_path_char(c) { '_' } else { c }) + .collect() +} + +fn is_banned_path_char(c: char) -> bool { + matches!(c, '<' | '>' | ':' | '"' | '|' | '?' | '*') +} + +fn sanitize_segment(text: &str) -> String { + text + .chars() + .map(|c| if is_banned_segment_char(c) { '_' } else { c }) + .collect() +} + +fn is_banned_segment_char(c: char) -> bool { + matches!(c, '/' | '\\') || is_banned_path_char(c) +} + +#[cfg(test)] +mod test { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn partition_by_root_specifiers_same_sub_folder() { + run_partition_by_root_specifiers_test( + vec![ + "https://deno.land/x/mod/A.ts", + "https://deno.land/x/mod/other/A.ts", + ], + vec![( + "https://deno.land/x/mod/", + vec![ + "https://deno.land/x/mod/A.ts", + "https://deno.land/x/mod/other/A.ts", + ], + )], + ); + } + + #[test] + fn partition_by_root_specifiers_different_sub_folder() { + run_partition_by_root_specifiers_test( + vec![ + "https://deno.land/x/mod/A.ts", + "https://deno.land/x/other/A.ts", + ], + vec![( + "https://deno.land/x/", + vec![ + "https://deno.land/x/mod/A.ts", + "https://deno.land/x/other/A.ts", + ], + )], + ); + } + + #[test] + fn partition_by_root_specifiers_different_hosts() { + run_partition_by_root_specifiers_test( + vec![ + "https://deno.land/mod/A.ts", + "https://localhost/mod/A.ts", + "https://other/A.ts", + ], + vec![ + ("https://deno.land/mod/", vec!["https://deno.land/mod/A.ts"]), + ("https://localhost/mod/", vec!["https://localhost/mod/A.ts"]), + ("https://other/", vec!["https://other/A.ts"]), + ], + ); + } + + fn run_partition_by_root_specifiers_test( + input: Vec<&str>, + expected: Vec<(&str, Vec<&str>)>, + ) { + let input = input + .iter() + .map(|s| ModuleSpecifier::parse(s).unwrap()) + .collect::>(); + let output = partition_by_root_specifiers(input.iter()); + // the assertion is much easier to compare when everything is strings + let output = output + .into_iter() + .map(|(s, vec)| { + ( + s.to_string(), + vec.into_iter().map(|s| s.to_string()).collect::>(), + ) + }) + .collect::>(); + let expected = expected + .into_iter() + .map(|(s, vec)| { + ( + s.to_string(), + vec.into_iter().map(|s| s.to_string()).collect::>(), + ) + }) + .collect::>(); + assert_eq!(output, expected); + } + + #[test] + fn should_get_dir_name_root() { + run_test( + "http://deno.land/x/test", + &["http://deno.land/x/test/mod.ts"], + "deno.land_x_test", + ); + run_test( + "http://localhost", + &["http://localhost/test.mod"], + "localhost", + ); + run_test( + "http://localhost/test%20test", + &["http://localhost/test%20test/asdf"], + "localhost_test%20test", + ); + // will truncate + run_test( + // length of 45 + "http://localhost/testtestestingtestingtesting", + // length of 210 + &["http://localhost/testtestestingtestingtesting/testingthisoutwithaverlongspecifiertestingtasfasdfasdfasdfadsfasdfasfasdfasfasdfasdfasfasdfasfdasdfasdfasdfasdfasdfsdafasdfasdasdfasdfasdfasdfasdfasdfaasdfasdfas.ts"], + // Max(10, 260 - 80 - (210 - 45)) = 15 chars + "localhost_testt", + ); + // will truncate + run_test( + // length of 45 + "http://localhost/testtestestingtestingtesting", + // length of 220 + &["http://localhost/testtestestingtestingtesting/testingthisoutwithaverlongspecifiertestingtasfasdfasdfasdfadsfasdfasfasdfasfasdfasdfasfasdfasfdasdfasdfasdfasdfasdfsdafasdfasdasdfasdfasdfasdfasdfasdfaasdfasdfasteststttts.ts"], + // Max(10, 260 - 80 - (210 - 45)) = 10 and trim the trailing underscore + "localhost", + ); + + fn run_test(specifier: &str, specifiers: &[&str], expected: &str) { + assert_eq!( + dir_name_for_root( + &ModuleSpecifier::parse(specifier).unwrap(), + &specifiers + .iter() + .map(|s| ModuleSpecifier::parse(s).unwrap()) + .collect::>(), + ), + PathBuf::from(expected) + ); + } + } + + #[test] + fn test_path_with_stem_suffix() { + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/"), "2"), + PathBuf::from("/2") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test"), "2"), + PathBuf::from("/test_2") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.txt"), "2"), + PathBuf::from("/test_2.txt") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test/subdir"), "2"), + PathBuf::from("/test/subdir_2") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test/subdir.other.txt"), "2"), + PathBuf::from("/test/subdir.other_2.txt") + ); + } + + #[test] + fn test_unique_path() { + let mut paths = HashSet::new(); + assert_eq!( + get_unique_path(PathBuf::from("/test"), &mut paths), + PathBuf::from("/test") + ); + assert_eq!( + get_unique_path(PathBuf::from("/test"), &mut paths), + PathBuf::from("/test_2") + ); + assert_eq!( + get_unique_path(PathBuf::from("/test"), &mut paths), + PathBuf::from("/test_3") + ); + assert_eq!( + get_unique_path(PathBuf::from("/TEST"), &mut paths), + PathBuf::from("/TEST_4") + ); + assert_eq!( + get_unique_path(PathBuf::from("/test.txt"), &mut paths), + PathBuf::from("/test.txt") + ); + assert_eq!( + get_unique_path(PathBuf::from("/test.txt"), &mut paths), + PathBuf::from("/test_2.txt") + ); + assert_eq!( + get_unique_path(PathBuf::from("/TEST.TXT"), &mut paths), + PathBuf::from("/TEST_3.TXT") + ); + } +} From 8532d19ef5a9bd5dc800ad9a327e63831ae567a0 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 5 Feb 2022 16:05:35 -0500 Subject: [PATCH 02/33] Add copyrights. --- cli/tools/vendor/mod.rs | 2 ++ cli/tools/vendor/specifiers.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 52e97be42da06..c6befe006f88c 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -1,3 +1,5 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index f823977a71a05..a58636d802f73 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -1,3 +1,5 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; From e0db996cf96c1281406bdbe2c835ca2ce8a68553 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 7 Feb 2022 12:40:43 -0500 Subject: [PATCH 03/33] Starting to add text changes. --- cli/tools/vendor/analyze.rs | 108 +++++++++++++++++++++++++++++++ cli/tools/vendor/mappings.rs | 52 +++++++++++++++ cli/tools/vendor/mod.rs | 57 +++++++++------- cli/tools/vendor/text_changes.rs | 71 ++++++++++++++++++++ 4 files changed, 265 insertions(+), 23 deletions(-) create mode 100644 cli/tools/vendor/analyze.rs create mode 100644 cli/tools/vendor/mappings.rs create mode 100644 cli/tools/vendor/text_changes.rs diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs new file mode 100644 index 0000000000000..ff83f0f46ac00 --- /dev/null +++ b/cli/tools/vendor/analyze.rs @@ -0,0 +1,108 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + +use std::path::Path; +use std::path::PathBuf; + +use deno_ast::LineAndColumnIndex; +use deno_ast::ModuleSpecifier; +use deno_ast::SourceTextInfo; +use deno_ast::swc::common::BytePos; +use deno_ast::swc::common::Span; +use deno_graph::Module; +use deno_graph::ModuleGraph; +use deno_graph::Position; +use deno_graph::Range; +use deno_graph::Resolved; + +use super::mappings::Mappings; +use super::text_changes::TextChange; + +pub struct CollectSpecifierTextChangesParams<'a> { + pub mappings: &'a Mappings, + pub module: &'a Module, + pub graph: &'a ModuleGraph, +} + +struct Context<'a> { + mappings: &'a Mappings, + module: &'a Module, + graph: &'a ModuleGraph, + text_info: &'a SourceTextInfo, + text_changes: Vec, + local_path: PathBuf, +} + +impl<'a> Context<'a> { + pub fn byte_pos(&self, pos: &Position) -> BytePos { + // todo(https://github.com/denoland/deno_graph/issues/79): use byte indexes all the way down + self.text_info.byte_index(LineAndColumnIndex { + line_index: pos.line, + column_index: pos.character, + }) + } + + pub fn span(&self, range: &Range) -> Span { + let start = self.byte_pos(&range.start); + let end = self.byte_pos(&range.end); + Span::new(start, end, Default::default()) + } +} + +pub fn collect_specifier_text_changes(params: &CollectSpecifierTextChangesParams) -> Vec { + let text_info = match ¶ms.module.maybe_parsed_source { + Some(source) => source.source(), + None => return Vec::new(), + }; + let mut context = Context { + mappings: params.mappings, + module: params.module, + graph: params.graph, + text_info, + text_changes: Vec::new(), + local_path: params.mappings.local_path(¶ms.module.specifier).to_owned(), + }; + + // todo(dsherret): this is may not good enough because it only includes what deno_graph has resolved + // and may not include everything in the source file + for (specifier, dep) in ¶ms.module.dependencies { + handle_maybe_resolved(&dep.maybe_code, &mut context); + handle_maybe_resolved(&dep.maybe_type, &mut context); + } + + context.text_changes +} + +fn handle_maybe_resolved(maybe_resolved: &Resolved, context: &mut Context<'_>) { + if let Resolved::Ok { specifier, range, .. } = maybe_resolved { + let span = context.span(range); + let local_path = context.mappings.local_path(specifier); + let new_specifier = get_relative_specifier(&context.local_path, &local_path); + context.text_changes.push(TextChange::from_span_and_text( + Span::new(span.lo + BytePos(1), span.hi - BytePos(1), Default::default()), + new_specifier, + )); + } +} + +fn get_relative_specifier( + from: &Path, + to: &Path, +) -> String { + let relative_path = get_relative_path(from, to); + + if relative_path.starts_with("../") || relative_path.starts_with("./") + { + relative_path + } else { + format!("./{}", relative_path) + } +} + +pub fn get_relative_path( + from: &Path, + to: &Path, +) -> String { + let from_path = ModuleSpecifier::from_file_path(from).unwrap(); + let to_path = ModuleSpecifier::from_file_path(to).unwrap(); + from_path.make_relative(&to_path).unwrap() +} diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs new file mode 100644 index 0000000000000..e216c074dea68 --- /dev/null +++ b/cli/tools/vendor/mappings.rs @@ -0,0 +1,52 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + +use std::collections::HashMap; +use std::collections::HashSet; +use std::path::Path; +use std::path::PathBuf; + +use deno_ast::ModuleSpecifier; +use deno_core::error::AnyError; +use deno_graph::Module; +use deno_graph::ModuleGraph; + +use super::specifiers::dir_name_for_root; +use super::specifiers::get_unique_path; +use super::specifiers::make_url_relative; +use super::specifiers::partition_by_root_specifiers; +use super::specifiers::sanitize_filepath; + +/// Constructs and holds the remote specifier to local path mappings. +pub struct Mappings(HashMap); + +impl Mappings { + pub fn from_remote_modules( + graph: &ModuleGraph, + remote_modules: &[&Module], + output_dir: &Path, + ) -> Result { + let partitioned_specifiers = partition_by_root_specifiers(remote_modules.iter().map(|m| &m.specifier)); + let mut mapped_paths = HashSet::new(); + let mut mappings = HashMap::new(); + + for (root, specifiers) in partitioned_specifiers.into_iter() { + let base_dir = get_unique_path( + output_dir.join(dir_name_for_root(&root, &specifiers)), + &mut mapped_paths, + ); + for specifier in specifiers { + let media_type = graph.get(&specifier).unwrap().media_type; + let relative = base_dir + .join(sanitize_filepath(&make_url_relative(&root, &specifier)?)) + .with_extension(&media_type.as_ts_extension()[1..]); + mappings.insert(specifier, get_unique_path(relative, &mut mapped_paths)); + } + } + + Ok(Self(mappings)) + } + + pub fn local_path(&self, specifier: &ModuleSpecifier) -> &PathBuf { + self.0.get(specifier).as_ref().unwrap_or_else(|| panic!("Could not find local path for {}", specifier)) + } +} \ No newline at end of file diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index c6befe006f88c..3bee7242c4b24 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -1,12 +1,11 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. -use std::collections::HashMap; -use std::collections::HashSet; use std::path::PathBuf; use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::resolve_url_or_path; +use deno_graph::ModuleKind; use deno_runtime::permissions::Permissions; use crate::flags::VendorFlags; @@ -15,13 +14,15 @@ use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; -use self::specifiers::dir_name_for_root; -use self::specifiers::get_unique_path; -use self::specifiers::make_url_relative; -use self::specifiers::partition_by_root_specifiers; -use self::specifiers::sanitize_filepath; +use self::analyze::CollectSpecifierTextChangesParams; +use self::analyze::collect_specifier_text_changes; +use self::mappings::Mappings; +use self::text_changes::apply_text_changes; +mod analyze; mod specifiers; +mod text_changes; +mod mappings; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { let output_dir = resolve_and_validate_output_dir(&flags)?; @@ -31,23 +32,33 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { .into_iter() .filter(|m| m.specifier.scheme().starts_with("http")) .collect::>(); - let partitioned_specifiers = - partition_by_root_specifiers(remote_modules.iter().map(|m| &m.specifier)); - let mut mapped_paths = HashSet::new(); - let mut mappings = HashMap::new(); + let mappings = Mappings::from_remote_modules(&graph, &remote_modules, &output_dir)?; - for (root, specifiers) in partitioned_specifiers.into_iter() { - let base_dir = get_unique_path( - output_dir.join(dir_name_for_root(&root, &specifiers)), - &mut mapped_paths, - ); - for specifier in specifiers { - let media_type = graph.get(&specifier).unwrap().media_type; - let relative = base_dir - .join(sanitize_filepath(&make_url_relative(&root, &specifier)?)) - .with_extension(&media_type.as_ts_extension()[1..]); - mappings.insert(specifier, get_unique_path(relative, &mut mapped_paths)); - } + // collect text changes + for module in &remote_modules { + let source = match &module.maybe_source { + Some(source) => source, + None => continue, + }; + let local_path = mappings.local_path(&module.specifier); + let file_text = match module.kind { + ModuleKind::Esm => { + let text_changes = collect_specifier_text_changes(&CollectSpecifierTextChangesParams { + graph: &graph, + mappings: &mappings, + module, + }); + apply_text_changes(source, text_changes) + }, + ModuleKind::Asserted => { + source.to_string() + }, + _ => { + log::warn!("Unsupported module kind {:?} for {}", module.kind, module.specifier); + continue; + } + }; + std::fs::write(local_path, file_text)?; } Ok(()) diff --git a/cli/tools/vendor/text_changes.rs b/cli/tools/vendor/text_changes.rs new file mode 100644 index 0000000000000..9198caaf1b81f --- /dev/null +++ b/cli/tools/vendor/text_changes.rs @@ -0,0 +1,71 @@ +// todo: before merge, delete this file as it's now in deno_ast + +use std::ops::Range; + +use deno_ast::swc::common::BytePos; +use deno_ast::swc::common::Span; + +#[derive(Clone, Debug)] +pub struct TextChange { + /// Range start to end byte index. + pub range: Range, + /// New text to insert or replace at the provided range. + pub new_text: String, +} + +impl TextChange { + pub fn new(start: usize, end: usize, new_text: String) -> Self { + Self { + range: start..end, + new_text, + } + } + + pub fn from_span_and_text(span: Span, new_text: String) -> Self { + TextChange::new(span.lo.0 as usize, span.hi.0 as usize, new_text) + } + + /// Gets an swc span for the provided text change. + pub fn as_span(&self) -> Span { + Span::new( + BytePos(self.range.start as u32), + BytePos(self.range.end as u32), + Default::default(), + ) + } +} + +/// Applies the text changes to the given source text. +pub fn apply_text_changes( + source: &str, + mut changes: Vec, +) -> String { + changes.sort_by(|a, b| a.range.start.cmp(&b.range.start)); + + let mut last_index = 0; + let mut final_text = String::new(); + + for change in changes { + if change.range.start > change.range.end { + panic!( + "Text change had start index {} greater than end index {}.", + change.range.start, change.range.end + ) + } + if change.range.start < last_index { + panic!("Text changes were overlapping. Past index was {}, but new change had index {}.", last_index, change.range.start); + } else if change.range.start > last_index && last_index < source.len() { + final_text.push_str( + &source[last_index..std::cmp::min(source.len(), change.range.start)], + ); + } + final_text.push_str(&change.new_text); + last_index = change.range.end; + } + + if last_index < source.len() { + final_text.push_str(&source[last_index..]); + } + + final_text +} From 383a962fdbef6e11c9db9c599b530abb7996861f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 7 Feb 2022 16:42:22 -0500 Subject: [PATCH 04/33] Somewhat working. --- cli/tools/vendor/analyze.rs | 83 ++++++++++--------- cli/tools/vendor/mappings.rs | 22 +++-- cli/tools/vendor/mod.rs | 134 +++++++++++++++++++++++++------ cli/tools/vendor/specifiers.rs | 26 +++--- cli/tools/vendor/text_changes.rs | 18 +---- 5 files changed, 182 insertions(+), 101 deletions(-) diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs index ff83f0f46ac00..27acead5ea59e 100644 --- a/cli/tools/vendor/analyze.rs +++ b/cli/tools/vendor/analyze.rs @@ -3,13 +3,12 @@ use std::path::Path; use std::path::PathBuf; +use deno_ast::swc::common::BytePos; +use deno_ast::swc::common::Span; use deno_ast::LineAndColumnIndex; use deno_ast::ModuleSpecifier; use deno_ast::SourceTextInfo; -use deno_ast::swc::common::BytePos; -use deno_ast::swc::common::Span; use deno_graph::Module; -use deno_graph::ModuleGraph; use deno_graph::Position; use deno_graph::Range; use deno_graph::Resolved; @@ -17,16 +16,9 @@ use deno_graph::Resolved; use super::mappings::Mappings; use super::text_changes::TextChange; -pub struct CollectSpecifierTextChangesParams<'a> { - pub mappings: &'a Mappings, - pub module: &'a Module, - pub graph: &'a ModuleGraph, -} - struct Context<'a> { mappings: &'a Mappings, module: &'a Module, - graph: &'a ModuleGraph, text_info: &'a SourceTextInfo, text_changes: Vec, local_path: PathBuf, @@ -36,8 +28,8 @@ impl<'a> Context<'a> { pub fn byte_pos(&self, pos: &Position) -> BytePos { // todo(https://github.com/denoland/deno_graph/issues/79): use byte indexes all the way down self.text_info.byte_index(LineAndColumnIndex { - line_index: pos.line, - column_index: pos.character, + line_index: pos.line, + column_index: pos.character, }) } @@ -46,62 +38,79 @@ impl<'a> Context<'a> { let end = self.byte_pos(&range.end); Span::new(start, end, Default::default()) } + + pub fn relative_specifier_text(&self, specifier: &ModuleSpecifier) -> String { + let local_path = self.mappings.local_path(specifier); + get_relative_specifier_text(&self.local_path, local_path) + } } -pub fn collect_specifier_text_changes(params: &CollectSpecifierTextChangesParams) -> Vec { - let text_info = match ¶ms.module.maybe_parsed_source { +pub fn collect_remote_module_text_changes<'a>( + mappings: &'a Mappings, + module: &'a Module, +) -> Vec { + let text_info = match &module.maybe_parsed_source { Some(source) => source.source(), None => return Vec::new(), }; let mut context = Context { - mappings: params.mappings, - module: params.module, - graph: params.graph, + mappings, + module, text_info, text_changes: Vec::new(), - local_path: params.mappings.local_path(¶ms.module.specifier).to_owned(), + local_path: mappings.local_path(&module.specifier).to_owned(), }; - // todo(dsherret): this is may not good enough because it only includes what deno_graph has resolved - // and may not include everything in the source file - for (specifier, dep) in ¶ms.module.dependencies { + // todo(THIS PR): this is may not good enough because it only includes + // what deno_graph has resolved and may not include everything in the source file? + for dep in context.module.dependencies.values() { handle_maybe_resolved(&dep.maybe_code, &mut context); handle_maybe_resolved(&dep.maybe_type, &mut context); } + // todo(THIS PR): does this contain more than just the header? I think so? + + // resolve x-typescript-types header and inject it as a types directive + if let Some((_, Resolved::Ok { specifier, .. })) = + &context.module.maybe_types_dependency + { + let new_specifier_text = context.relative_specifier_text(specifier); + context.text_changes.push(TextChange::new( + 0, + 0, + format!("/// \n", new_specifier_text), + )) + } + context.text_changes } fn handle_maybe_resolved(maybe_resolved: &Resolved, context: &mut Context<'_>) { - if let Resolved::Ok { specifier, range, .. } = maybe_resolved { + if let Resolved::Ok { + specifier, range, .. + } = maybe_resolved + { let span = context.span(range); - let local_path = context.mappings.local_path(specifier); - let new_specifier = get_relative_specifier(&context.local_path, &local_path); - context.text_changes.push(TextChange::from_span_and_text( - Span::new(span.lo + BytePos(1), span.hi - BytePos(1), Default::default()), - new_specifier, + let new_specifier_text = context.relative_specifier_text(specifier); + context.text_changes.push(TextChange::new( + span.lo.0 as usize + 1, + span.hi.0 as usize - 1, + new_specifier_text, )); } } -fn get_relative_specifier( - from: &Path, - to: &Path, -) -> String { +fn get_relative_specifier_text(from: &Path, to: &Path) -> String { let relative_path = get_relative_path(from, to); - if relative_path.starts_with("../") || relative_path.starts_with("./") - { + if relative_path.starts_with("../") || relative_path.starts_with("./") { relative_path } else { format!("./{}", relative_path) } } -pub fn get_relative_path( - from: &Path, - to: &Path, -) -> String { +fn get_relative_path(from: &Path, to: &Path) -> String { let from_path = ModuleSpecifier::from_file_path(from).unwrap(); let to_path = ModuleSpecifier::from_file_path(to).unwrap(); from_path.make_relative(&to_path).unwrap() diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index e216c074dea68..f06a5ca604a2d 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -21,11 +21,12 @@ pub struct Mappings(HashMap); impl Mappings { pub fn from_remote_modules( - graph: &ModuleGraph, - remote_modules: &[&Module], - output_dir: &Path, - ) -> Result { - let partitioned_specifiers = partition_by_root_specifiers(remote_modules.iter().map(|m| &m.specifier)); + graph: &ModuleGraph, + remote_modules: &[&Module], + output_dir: &Path, + ) -> Result { + let partitioned_specifiers = + partition_by_root_specifiers(remote_modules.iter().map(|m| &m.specifier)); let mut mapped_paths = HashSet::new(); let mut mappings = HashMap::new(); @@ -39,7 +40,8 @@ impl Mappings { let relative = base_dir .join(sanitize_filepath(&make_url_relative(&root, &specifier)?)) .with_extension(&media_type.as_ts_extension()[1..]); - mappings.insert(specifier, get_unique_path(relative, &mut mapped_paths)); + mappings + .insert(specifier, get_unique_path(relative, &mut mapped_paths)); } } @@ -47,6 +49,10 @@ impl Mappings { } pub fn local_path(&self, specifier: &ModuleSpecifier) -> &PathBuf { - self.0.get(specifier).as_ref().unwrap_or_else(|| panic!("Could not find local path for {}", specifier)) + self + .0 + .get(specifier) + .as_ref() + .unwrap_or_else(|| panic!("Could not find local path for {}", specifier)) } -} \ No newline at end of file +} diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 3bee7242c4b24..ef638a0f5a0cb 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -1,11 +1,16 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +use std::collections::HashSet; +use std::path::Path; use std::path::PathBuf; +use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::resolve_url_or_path; +use deno_graph::Module; use deno_graph::ModuleKind; +use deno_graph::Resolved; use deno_runtime::permissions::Permissions; use crate::flags::VendorFlags; @@ -14,27 +19,26 @@ use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; -use self::analyze::CollectSpecifierTextChangesParams; -use self::analyze::collect_specifier_text_changes; +use self::analyze::collect_remote_module_text_changes; use self::mappings::Mappings; use self::text_changes::apply_text_changes; mod analyze; +mod mappings; mod specifiers; mod text_changes; -mod mappings; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { let output_dir = resolve_and_validate_output_dir(&flags)?; let graph = create_graph(&ps, &flags).await?; - let remote_modules = graph + let (remote_modules, local_modules) = graph .modules() .into_iter() - .filter(|m| m.specifier.scheme().starts_with("http")) - .collect::>(); - let mappings = Mappings::from_remote_modules(&graph, &remote_modules, &output_dir)?; + .partition::, _>(|m| is_remote_specifier(&m.specifier)); + let mappings = + Mappings::from_remote_modules(&graph, &remote_modules, &output_dir)?; - // collect text changes + // collect and write out all the text changes for module in &remote_modules { let source = match &module.maybe_source { Some(source) => source, @@ -43,24 +47,31 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { let local_path = mappings.local_path(&module.specifier); let file_text = match module.kind { ModuleKind::Esm => { - let text_changes = collect_specifier_text_changes(&CollectSpecifierTextChangesParams { - graph: &graph, - mappings: &mappings, - module, - }); + let text_changes = + collect_remote_module_text_changes(&mappings, module); apply_text_changes(source, text_changes) - }, - ModuleKind::Asserted => { - source.to_string() - }, + } + ModuleKind::Asserted => source.to_string(), _ => { - log::warn!("Unsupported module kind {:?} for {}", module.kind, module.specifier); + log::warn!( + "Unsupported module kind {:?} for {}", + module.kind, + module.specifier + ); continue; } }; + std::fs::create_dir_all(local_path.parent().unwrap())?; std::fs::write(local_path, file_text)?; } + // create the import map + if let Some(import_map_text) = + build_import_map(&output_dir, &local_modules, &mappings) + { + std::fs::write(output_dir.join("import_map.json"), import_map_text)?; + } + Ok(()) } @@ -71,16 +82,22 @@ fn resolve_and_validate_output_dir( Some(output_path) => output_path.clone(), None => std::env::current_dir()?.join("vendor"), }; - if !flags.force { - if let Ok(mut dir) = std::fs::read_dir(&output_dir) { - if dir.next().is_some() { - bail!("Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", output_dir.display()); - } - } + if !flags.force && !is_dir_empty(&output_dir)? { + bail!("Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", output_dir.display()); } Ok(output_dir) } +fn is_dir_empty(dir_path: &Path) -> Result { + match std::fs::read_dir(&dir_path) { + Ok(mut dir) => Ok(dir.next().is_none()), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(true), + Err(err) => { + bail!("Error reading directory {}: {}", dir_path.display(), err) + } + } +} + async fn create_graph( ps: &ProcState, flags: &VendorFlags, @@ -144,3 +161,72 @@ async fn create_graph( Ok(graph) } + +fn build_import_map( + output_dir: &Path, + local_modules: &[&Module], + mappings: &Mappings, +) -> Option { + let key_values = collect_import_map_key_values(local_modules); + if key_values.is_empty() { + return None; + } + + let output_dir = ModuleSpecifier::from_directory_path(&output_dir).unwrap(); + + // purposefully includes duplicate keys... the user should then select which to delete + let mut text = "{\n".to_string(); + text.push_str(" \"imports\": {\n"); + for (i, (key, value)) in key_values.iter().enumerate() { + if i > 0 { + text.push_str(",\n"); + } + let local_path = mappings.local_path(value); + let local_uri = ModuleSpecifier::from_file_path(&local_path).unwrap(); + let relative_path = output_dir.make_relative(&local_uri).unwrap(); + text.push_str(&format!(" \"{}\": \"./{}\"", key, relative_path)); + } + text.push_str("\n }\n"); + text.push_str("}\n"); + + Some(text) +} + +fn collect_import_map_key_values( + local_modules: &[&Module], +) -> Vec<(String, ModuleSpecifier)> { + fn add_if_remote( + specifiers: &mut HashSet<(String, ModuleSpecifier)>, + text: &str, + specifier: &ModuleSpecifier, + ) { + if is_remote_specifier(specifier) { + specifiers.insert((text.to_string(), specifier.clone())); + } + } + + let mut result = HashSet::new(); + for module in local_modules { + for (text, dep) in &module.dependencies { + if let Some(specifier) = dep.get_code() { + add_if_remote(&mut result, text, specifier); + } + if let Some(specifier) = dep.get_type() { + add_if_remote(&mut result, text, specifier); + } + } + if let Some((text, Resolved::Ok { specifier, .. })) = + &module.maybe_types_dependency + { + add_if_remote(&mut result, text, specifier); + } + } + + let mut result = result.into_iter().collect::>(); + result.sort(); + result +} + +fn is_remote_specifier(specifier: &ModuleSpecifier) -> bool { + specifier.scheme().to_lowercase().starts_with("http") +} diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index a58636d802f73..4334e9fe011c5 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -72,7 +72,7 @@ pub fn dir_name_for_root( if let Some(segments) = root.path_segments() { for segment in segments.filter(|s| !s.is_empty()) { if !result.is_empty() { - result.push('_'); + result.push('/'); } result.push_str(&sanitize_segment(segment)); } @@ -103,10 +103,13 @@ pub fn dir_name_for_root( - sub_path_len as isize, ) as usize; - truncate_str(&result, truncate_len) - .trim_end_matches('_') - .trim_end_matches('.') - .to_string() + // if the final text should be truncated, then truncate and + // flatten it to a single folder name + let text = match result.char_indices().nth(truncate_len) { + Some((i, _)) => (&result[..i]).replace('/', "_"), + None => result, + }; + text.trim_end_matches('_').trim_end_matches('.').to_string() }) } @@ -158,13 +161,6 @@ pub fn make_url_relative( }) } -fn truncate_str(text: &str, max: usize) -> &str { - match text.char_indices().nth(max) { - Some((i, _)) => &text[..i], - None => text, - } -} - pub fn sanitize_filepath(text: &str) -> String { text .chars() @@ -278,7 +274,7 @@ mod test { run_test( "http://deno.land/x/test", &["http://deno.land/x/test/mod.ts"], - "deno.land_x_test", + "deno.land/x/test", ); run_test( "http://localhost", @@ -288,9 +284,9 @@ mod test { run_test( "http://localhost/test%20test", &["http://localhost/test%20test/asdf"], - "localhost_test%20test", + "localhost/test%20test", ); - // will truncate + // will flatten and truncate run_test( // length of 45 "http://localhost/testtestestingtestingtesting", diff --git a/cli/tools/vendor/text_changes.rs b/cli/tools/vendor/text_changes.rs index 9198caaf1b81f..75eab44f006a8 100644 --- a/cli/tools/vendor/text_changes.rs +++ b/cli/tools/vendor/text_changes.rs @@ -1,10 +1,7 @@ -// todo: before merge, delete this file as it's now in deno_ast +// todo(THIS PR): before merge, delete this file as this functionality is now in deno_ast use std::ops::Range; -use deno_ast::swc::common::BytePos; -use deno_ast::swc::common::Span; - #[derive(Clone, Debug)] pub struct TextChange { /// Range start to end byte index. @@ -20,19 +17,6 @@ impl TextChange { new_text, } } - - pub fn from_span_and_text(span: Span, new_text: String) -> Self { - TextChange::new(span.lo.0 as usize, span.hi.0 as usize, new_text) - } - - /// Gets an swc span for the provided text change. - pub fn as_span(&self) -> Span { - Span::new( - BytePos(self.range.start as u32), - BytePos(self.range.end as u32), - Default::default(), - ) - } } /// Applies the text changes to the given source text. From 85ab7ed8a1c5256c6579bee9fa79caaec29293a6 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 7 Feb 2022 18:10:29 -0500 Subject: [PATCH 05/33] Slight fix. --- cli/tools/vendor/mod.rs | 13 +++++++------ cli/tools/vendor/specifiers.rs | 3 ++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index ef638a0f5a0cb..1a7eef0dc1d2e 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -1,6 +1,6 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. -use std::collections::HashSet; +use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; @@ -35,6 +35,7 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { .modules() .into_iter() .partition::, _>(|m| is_remote_specifier(&m.specifier)); + // todo(THIS PR): change this to leave absolute http(s) urls as-is in the vendored files let mappings = Mappings::from_remote_modules(&graph, &remote_modules, &output_dir)?; @@ -174,7 +175,7 @@ fn build_import_map( let output_dir = ModuleSpecifier::from_directory_path(&output_dir).unwrap(); - // purposefully includes duplicate keys... the user should then select which to delete + // todo: use serde_json to print htis out? let mut text = "{\n".to_string(); text.push_str(" \"imports\": {\n"); for (i, (key, value)) in key_values.iter().enumerate() { @@ -196,16 +197,16 @@ fn collect_import_map_key_values( local_modules: &[&Module], ) -> Vec<(String, ModuleSpecifier)> { fn add_if_remote( - specifiers: &mut HashSet<(String, ModuleSpecifier)>, + specifiers: &mut HashMap, text: &str, specifier: &ModuleSpecifier, ) { if is_remote_specifier(specifier) { - specifiers.insert((text.to_string(), specifier.clone())); + specifiers.insert(text.to_string(), specifier.clone()); } } - let mut result = HashSet::new(); + let mut result = HashMap::new(); for module in local_modules { for (text, dep) in &module.dependencies { if let Some(specifier) = dep.get_code() { @@ -223,7 +224,7 @@ fn collect_import_map_key_values( } let mut result = result.into_iter().collect::>(); - result.sort(); + result.sort_by(|a, b| a.0.cmp(&b.0)); result } diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index 4334e9fe011c5..8fe49515a2c64 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -94,7 +94,8 @@ pub fn dir_name_for_root( let max_win_path = 260; // This is the approximate length that a path might be before the root directory. // It should be a hardcoded number and not calculated based on the system as the - // produced code will most likely not stay only on the system that produced it. + // produced code will most likely not stay only on the system or directory that + // produced it. let approx_path_prefix_len = 80; let truncate_len = std::cmp::max( 10, From 31c499699f7d85d884ad6a4d391bede6225ad4fb Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 8 Feb 2022 12:29:39 -0500 Subject: [PATCH 06/33] Better import map and leave output files the same more --- cli/tools/vendor/analyze.rs | 54 ++++++++++++++++++++++------------ cli/tools/vendor/mappings.rs | 48 +++++++++++++++++++++++++++--- cli/tools/vendor/mod.rs | 31 ++++++++++--------- cli/tools/vendor/specifiers.rs | 36 +++++++++++++++++++++-- 4 files changed, 129 insertions(+), 40 deletions(-) diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs index 27acead5ea59e..5b59fdd5e1295 100644 --- a/cli/tools/vendor/analyze.rs +++ b/cli/tools/vendor/analyze.rs @@ -3,8 +3,6 @@ use std::path::Path; use std::path::PathBuf; -use deno_ast::swc::common::BytePos; -use deno_ast::swc::common::Span; use deno_ast::LineAndColumnIndex; use deno_ast::ModuleSpecifier; use deno_ast::SourceTextInfo; @@ -14,29 +12,34 @@ use deno_graph::Range; use deno_graph::Resolved; use super::mappings::Mappings; +use super::specifiers::is_remote_specifier_text; use super::text_changes::TextChange; struct Context<'a> { mappings: &'a Mappings, module: &'a Module, text_info: &'a SourceTextInfo, + text: &'a str, text_changes: Vec, local_path: PathBuf, } impl<'a> Context<'a> { - pub fn byte_pos(&self, pos: &Position) -> BytePos { + pub fn byte_index(&self, pos: &Position) -> usize { // todo(https://github.com/denoland/deno_graph/issues/79): use byte indexes all the way down - self.text_info.byte_index(LineAndColumnIndex { - line_index: pos.line, - column_index: pos.character, - }) + self + .text_info + .byte_index(LineAndColumnIndex { + line_index: pos.line, + column_index: pos.character, + }) + .0 as usize } - pub fn span(&self, range: &Range) -> Span { - let start = self.byte_pos(&range.start); - let end = self.byte_pos(&range.end); - Span::new(start, end, Default::default()) + pub fn byte_range(&self, range: &Range) -> std::ops::Range { + let start = self.byte_index(&range.start); + let end = self.byte_index(&range.end); + start..end } pub fn relative_specifier_text(&self, specifier: &ModuleSpecifier) -> String { @@ -53,10 +56,15 @@ pub fn collect_remote_module_text_changes<'a>( Some(source) => source.source(), None => return Vec::new(), }; + let text = match &module.maybe_source { + Some(source) => source, + None => return Vec::new(), + }; let mut context = Context { mappings, module, text_info, + text, text_changes: Vec::new(), local_path: mappings.local_path(&module.specifier).to_owned(), }; @@ -90,13 +98,23 @@ fn handle_maybe_resolved(maybe_resolved: &Resolved, context: &mut Context<'_>) { specifier, range, .. } = maybe_resolved { - let span = context.span(range); - let new_specifier_text = context.relative_specifier_text(specifier); - context.text_changes.push(TextChange::new( - span.lo.0 as usize + 1, - span.hi.0 as usize - 1, - new_specifier_text, - )); + let mut byte_range = context.byte_range(range); + let mut current_text = &context.text[byte_range.clone()]; + if current_text.starts_with('"') || current_text.starts_with('\'') { + // remove the quotes + byte_range = (byte_range.start + 1)..(byte_range.end - 1); + current_text = &context.text[byte_range.clone()]; + }; + + // leave remote specifiers as-is as they will be handled by the import map + if !is_remote_specifier_text(current_text) { + let new_specifier_text = context.relative_specifier_text(specifier); + context.text_changes.push(TextChange::new( + byte_range.start, + byte_range.end, + new_specifier_text, + )); + } } } diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index f06a5ca604a2d..c50adb8fb3b49 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -37,11 +37,13 @@ impl Mappings { ); for specifier in specifiers { let media_type = graph.get(&specifier).unwrap().media_type; - let relative = base_dir - .join(sanitize_filepath(&make_url_relative(&root, &specifier)?)) - .with_extension(&media_type.as_ts_extension()[1..]); + let new_path = path_with_extension( + &base_dir + .join(sanitize_filepath(&make_url_relative(&root, &specifier)?)), + &media_type.as_ts_extension()[1..], + ); mappings - .insert(specifier, get_unique_path(relative, &mut mapped_paths)); + .insert(specifier, get_unique_path(new_path, &mut mapped_paths)); } } @@ -56,3 +58,41 @@ impl Mappings { .unwrap_or_else(|| panic!("Could not find local path for {}", specifier)) } } + +fn path_with_extension(path: &Path, ext: &str) -> PathBuf { + if let Some(file_stem) = path.file_stem().map(|f| f.to_string_lossy()) { + if path.extension().is_some() && file_stem.to_lowercase().ends_with(".d") { + return path.with_file_name(format!( + "{}.{}", + &file_stem[..file_stem.len() - ".d".len()], + ext + )); + } + } + path.with_extension(ext) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_path_with_extension() { + assert_eq!( + path_with_extension(&PathBuf::from("/test.D.TS"), "ts"), + PathBuf::from("/test.ts") + ); + assert_eq!( + path_with_extension(&PathBuf::from("/test.D.MTS"), "js"), + PathBuf::from("/test.js") + ); + assert_eq!( + path_with_extension(&PathBuf::from("/test.D.TS"), "d.ts"), + PathBuf::from("/test.d.ts") + ); + assert_eq!( + path_with_extension(&PathBuf::from("/test.js"), "js"), + PathBuf::from("/test.ts") + ); + } +} diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 1a7eef0dc1d2e..155abf9f83461 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -21,6 +21,8 @@ use crate::resolver::JsxResolver; use self::analyze::collect_remote_module_text_changes; use self::mappings::Mappings; +use self::specifiers::is_remote_specifier; +use self::specifiers::is_remote_specifier_text; use self::text_changes::apply_text_changes; mod analyze; @@ -31,11 +33,12 @@ mod text_changes; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { let output_dir = resolve_and_validate_output_dir(&flags)?; let graph = create_graph(&ps, &flags).await?; - let (remote_modules, local_modules) = graph - .modules() - .into_iter() - .partition::, _>(|m| is_remote_specifier(&m.specifier)); - // todo(THIS PR): change this to leave absolute http(s) urls as-is in the vendored files + let all_modules = graph.modules(); + let remote_modules = all_modules + .iter() + .filter(|m| is_remote_specifier(&m.specifier)) + .copied() + .collect::>(); let mappings = Mappings::from_remote_modules(&graph, &remote_modules, &output_dir)?; @@ -68,7 +71,7 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { // create the import map if let Some(import_map_text) = - build_import_map(&output_dir, &local_modules, &mappings) + build_import_map(&output_dir, &all_modules, &mappings) { std::fs::write(output_dir.join("import_map.json"), import_map_text)?; } @@ -165,17 +168,17 @@ async fn create_graph( fn build_import_map( output_dir: &Path, - local_modules: &[&Module], + modules: &[&Module], mappings: &Mappings, ) -> Option { - let key_values = collect_import_map_key_values(local_modules); + let key_values = collect_import_map_key_values(modules); if key_values.is_empty() { return None; } let output_dir = ModuleSpecifier::from_directory_path(&output_dir).unwrap(); - // todo: use serde_json to print htis out? + // todo: use serde_json to print this out? let mut text = "{\n".to_string(); text.push_str(" \"imports\": {\n"); for (i, (key, value)) in key_values.iter().enumerate() { @@ -197,12 +200,12 @@ fn collect_import_map_key_values( local_modules: &[&Module], ) -> Vec<(String, ModuleSpecifier)> { fn add_if_remote( - specifiers: &mut HashMap, + imports: &mut HashMap, text: &str, specifier: &ModuleSpecifier, ) { - if is_remote_specifier(specifier) { - specifiers.insert(text.to_string(), specifier.clone()); + if is_remote_specifier_text(text) { + imports.insert(text.to_string(), specifier.clone()); } } @@ -227,7 +230,3 @@ fn collect_import_map_key_values( result.sort_by(|a, b| a.0.cmp(&b.0)); result } - -fn is_remote_specifier(specifier: &ModuleSpecifier) -> bool { - specifier.scheme().to_lowercase().starts_with("http") -} diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index 8fe49515a2c64..bfd24894ddf9c 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -119,8 +119,16 @@ pub fn path_with_stem_suffix(path: &Path, suffix: &str) -> PathBuf { if let Some(file_name) = path.file_name().map(|f| f.to_string_lossy()) { if let Some(file_stem) = path.file_stem().map(|f| f.to_string_lossy()) { if let Some(ext) = path.extension().map(|f| f.to_string_lossy()) { - return path - .with_file_name(format!("{}_{}.{}", file_stem, suffix, ext)); + return if file_stem.to_lowercase().ends_with(".d") { + path.with_file_name(format!( + "{}_{}.d.{}", + &file_stem[..file_stem.len() - ".d".len()], + suffix, + ext + )) + } else { + path.with_file_name(format!("{}_{}.{}", file_stem, suffix, ext)) + }; } } @@ -162,6 +170,14 @@ pub fn make_url_relative( }) } +pub fn is_remote_specifier(specifier: &ModuleSpecifier) -> bool { + specifier.scheme().to_lowercase().starts_with("http") +} + +pub fn is_remote_specifier_text(text: &str) -> bool { + text.trim_start().to_lowercase().starts_with("http") +} + pub fn sanitize_filepath(text: &str) -> String { text .chars() @@ -342,6 +358,22 @@ mod test { path_with_stem_suffix(&PathBuf::from("/test/subdir.other.txt"), "2"), PathBuf::from("/test/subdir.other_2.txt") ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.d.ts"), "2"), + PathBuf::from("/test_2.d.ts") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.D.TS"), "2"), + PathBuf::from("/test_2.D.TS") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.d.mts"), "2"), + PathBuf::from("/test_2.d.mts") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.d.cts"), "2"), + PathBuf::from("/test_2.d.cts") + ); } #[test] From 591042d455447b7ea75502306d266df1c515d271 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 8 Feb 2022 15:31:33 -0500 Subject: [PATCH 07/33] Small fixes. Need to resolve the todos... might be a bit of work. --- cli/tools/vendor/mappings.rs | 6 +++++- cli/tools/vendor/mod.rs | 3 +-- cli/tools/vendor/specifiers.rs | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index c50adb8fb3b49..6e0ec6d52ff99 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -90,9 +90,13 @@ mod test { path_with_extension(&PathBuf::from("/test.D.TS"), "d.ts"), PathBuf::from("/test.d.ts") ); + assert_eq!( + path_with_extension(&PathBuf::from("/test.ts"), "js"), + PathBuf::from("/test.js") + ); assert_eq!( path_with_extension(&PathBuf::from("/test.js"), "js"), - PathBuf::from("/test.ts") + PathBuf::from("/test.js") ); } } diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 155abf9f83461..ea95b03aeb46b 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -116,8 +116,7 @@ async fn create_graph( .collect::, AnyError>>()?; // todo(dsherret): there is a lot of copy and paste here from - // other parts of the codebase and we should resolve this - // code duplication in a future PR + // other parts of the codebase let mut cache = crate::cache::FetchCacher::new( ps.dir.gen_cache.clone(), ps.file_fetcher.clone(), diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index bfd24894ddf9c..063dcc461dbcf 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -364,7 +364,8 @@ mod test { ); assert_eq!( path_with_stem_suffix(&PathBuf::from("/test.D.TS"), "2"), - PathBuf::from("/test_2.D.TS") + // good enough + PathBuf::from("/test_2.d.TS") ); assert_eq!( path_with_stem_suffix(&PathBuf::from("/test.d.mts"), "2"), From 5e6022a63d90105a1359e420ea6640e10821a21f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 9 Feb 2022 19:15:39 -0500 Subject: [PATCH 08/33] Lots of bugs, but slowly getting there. Will need deno_graph improvements though. --- cli/tools/vendor/analyze.rs | 13 ++- cli/tools/vendor/import_map.rs | 188 +++++++++++++++++++++++++++++++++ cli/tools/vendor/mappings.rs | 140 +++++++++++++++++++++--- cli/tools/vendor/mod.rs | 85 ++------------- cli/tools/vendor/specifiers.rs | 10 +- 5 files changed, 336 insertions(+), 100 deletions(-) create mode 100644 cli/tools/vendor/import_map.rs diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs index 5b59fdd5e1295..a4d9c4c2b6561 100644 --- a/cli/tools/vendor/analyze.rs +++ b/cli/tools/vendor/analyze.rs @@ -12,9 +12,11 @@ use deno_graph::Range; use deno_graph::Resolved; use super::mappings::Mappings; -use super::specifiers::is_remote_specifier_text; +use super::specifiers::is_absolute_specifier_text; use super::text_changes::TextChange; +// todo: delete... not used anymore + struct Context<'a> { mappings: &'a Mappings, module: &'a Module, @@ -44,7 +46,7 @@ impl<'a> Context<'a> { pub fn relative_specifier_text(&self, specifier: &ModuleSpecifier) -> String { let local_path = self.mappings.local_path(specifier); - get_relative_specifier_text(&self.local_path, local_path) + get_relative_specifier_text(&self.local_path, &local_path) } } @@ -66,11 +68,12 @@ pub fn collect_remote_module_text_changes<'a>( text_info, text, text_changes: Vec::new(), - local_path: mappings.local_path(&module.specifier).to_owned(), + local_path: mappings.local_path(&module.specifier), }; // todo(THIS PR): this is may not good enough because it only includes // what deno_graph has resolved and may not include everything in the source file? + // Yeah, not enough. for dep in context.module.dependencies.values() { handle_maybe_resolved(&dep.maybe_code, &mut context); handle_maybe_resolved(&dep.maybe_type, &mut context); @@ -107,7 +110,7 @@ fn handle_maybe_resolved(maybe_resolved: &Resolved, context: &mut Context<'_>) { }; // leave remote specifiers as-is as they will be handled by the import map - if !is_remote_specifier_text(current_text) { + if !is_absolute_specifier_text(current_text) { let new_specifier_text = context.relative_specifier_text(specifier); context.text_changes.push(TextChange::new( byte_range.start, @@ -118,6 +121,7 @@ fn handle_maybe_resolved(maybe_resolved: &Resolved, context: &mut Context<'_>) { } } +// todo: delete, I moved it elsewhere fn get_relative_specifier_text(from: &Path, to: &Path) -> String { let relative_path = get_relative_path(from, to); @@ -128,6 +132,7 @@ fn get_relative_specifier_text(from: &Path, to: &Path) -> String { } } +// todo: delete fn get_relative_path(from: &Path, to: &Path) -> String { let from_path = ModuleSpecifier::from_file_path(from).unwrap(); let to_path = ModuleSpecifier::from_file_path(to).unwrap(); diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs new file mode 100644 index 0000000000000..b4f9910d4cd20 --- /dev/null +++ b/cli/tools/vendor/import_map.rs @@ -0,0 +1,188 @@ +use std::collections::BTreeMap; + +use deno_ast::ModuleSpecifier; +use deno_core::serde_json; +use deno_graph::Module; +use deno_graph::Resolved; +use serde::Serialize; + +use super::mappings::Mappings; +use super::specifiers::is_absolute_specifier_text; +use super::specifiers::is_remote_specifier; + +#[derive(Serialize)] +struct SerializableImportMap { + imports: BTreeMap, + #[serde(skip_serializing_if = "BTreeMap::is_empty")] + scopes: BTreeMap>, +} + +struct ImportMapBuilder<'a> { + mappings: &'a Mappings, + imports: ImportsBuilder<'a>, + scopes: BTreeMap>, +} + +impl<'a> ImportMapBuilder<'a> { + pub fn new(mappings: &'a Mappings) -> Self { + ImportMapBuilder { + mappings, + imports: ImportsBuilder::new(mappings), + scopes: Default::default(), + } + } + + pub fn scope( + &mut self, + base_specifier: &ModuleSpecifier, + ) -> &mut ImportsBuilder<'a> { + self + .scopes + .entry( + self + .mappings + .relative_specifier_text(self.mappings.output_dir(), base_specifier), + ) + .or_insert_with(|| ImportsBuilder::new(self.mappings)) + } + + pub fn into_serializable(self) -> SerializableImportMap { + SerializableImportMap { + imports: self.imports.imports, + scopes: self + .scopes + .into_iter() + .map(|(key, value)| (key, value.imports)) + .collect(), + } + } + + pub fn into_file_text(self) -> String { + serde_json::to_string_pretty(&self.into_serializable()).unwrap() + } +} + +struct ImportsBuilder<'a> { + mappings: &'a Mappings, + imports: BTreeMap, +} + +impl<'a> ImportsBuilder<'a> { + pub fn new(mappings: &'a Mappings) -> Self { + Self { + mappings, + imports: Default::default(), + } + } + + pub fn add(&mut self, key: String, specifier: &ModuleSpecifier) { + self.imports.insert( + key, + self + .mappings + .relative_specifier_text(self.mappings.output_dir(), specifier), + ); + } +} + +pub fn build_import_map(modules: &[&Module], mappings: &Mappings) -> String { + let mut import_map = ImportMapBuilder::new(mappings); + fill_scopes(modules, mappings, &mut import_map); + + for base_specifier in mappings.base_specifiers() { + import_map + .imports + .add(base_specifier.to_string(), base_specifier); + } + + import_map.into_file_text() +} + +fn fill_scopes( + modules: &[&Module], + mappings: &Mappings, + import_map: &mut ImportMapBuilder, +) { + for module in modules { + for (text, dep) in &module.dependencies { + if let Some(specifier) = dep.get_code() { + handle_dep_specifier( + import_map, + &module.specifier, + text, + specifier, + mappings, + ); + } + if let Some(specifier) = dep.get_type() { + handle_dep_specifier( + import_map, + &module.specifier, + text, + specifier, + mappings, + ); + } + } + if let Some((text, Resolved::Ok { specifier, .. })) = + &module.maybe_types_dependency + { + handle_dep_specifier( + import_map, + &module.specifier, + text, + specifier, + mappings, + ); + } + } +} + +fn handle_dep_specifier( + import_map: &mut ImportMapBuilder, + referrer: &ModuleSpecifier, + text: &str, + specifier: &ModuleSpecifier, + mappings: &Mappings, +) { + // do not handle specifiers pointing at local modules + if !is_remote_specifier(specifier) { + return; + } + + let base_specifier = mappings.base_specifier(specifier); + if is_absolute_specifier_text(text) { + if !text.starts_with(base_specifier.as_str()) { + panic!("Expected {} to start with {}", text, base_specifier); + } + + let sub_path = &text[base_specifier.as_str().len()..]; + let expected_relative_specifier_text = + mappings.relative_path(base_specifier, specifier); + if expected_relative_specifier_text == sub_path { + return; + } + println!( + "File system text: {} || Sub path: {}", + expected_relative_specifier_text, sub_path + ); + + if !is_remote_specifier(referrer) { + import_map.imports.add(text.to_string(), specifier); + } else { + let imports = import_map.scope(base_specifier); + imports.add(sub_path.to_string(), specifier); + } + } else { + let expected_relative_specifier_text = + mappings.relative_specifier_text(referrer, specifier); + if expected_relative_specifier_text == text { + return; + } + // println!("File system text: {} || Actual: {}", file_system_specifier_text, text); + + let imports = import_map.scope(base_specifier); + // todo: wrong + imports.add(text.to_string(), specifier); + } +} diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index 6e0ec6d52ff99..d0647f84ddb34 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -5,6 +5,7 @@ use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; +use deno_ast::MediaType; use deno_ast::ModuleSpecifier; use deno_core::error::AnyError; use deno_graph::Module; @@ -17,7 +18,11 @@ use super::specifiers::partition_by_root_specifiers; use super::specifiers::sanitize_filepath; /// Constructs and holds the remote specifier to local path mappings. -pub struct Mappings(HashMap); +pub struct Mappings { + output_dir: ModuleSpecifier, + mappings: HashMap, + base_specifiers: Vec, +} impl Mappings { pub fn from_remote_modules( @@ -29,6 +34,7 @@ impl Mappings { partition_by_root_specifiers(remote_modules.iter().map(|m| &m.specifier)); let mut mapped_paths = HashSet::new(); let mut mappings = HashMap::new(); + let mut base_specifiers = Vec::new(); for (root, specifiers) in partitioned_specifiers.into_iter() { let base_dir = get_unique_path( @@ -37,36 +43,136 @@ impl Mappings { ); for specifier in specifiers { let media_type = graph.get(&specifier).unwrap().media_type; + let sub_path = sanitize_filepath(&make_url_relative(&root, &{ + let mut specifier = specifier.clone(); + specifier.set_query(None); + specifier + })?); let new_path = path_with_extension( - &base_dir - .join(sanitize_filepath(&make_url_relative(&root, &specifier)?)), + &base_dir.join(if cfg!(windows) { + sub_path.replace('/', "\\") + } else { + sub_path + }), &media_type.as_ts_extension()[1..], ); mappings .insert(specifier, get_unique_path(new_path, &mut mapped_paths)); } + base_specifiers.push(root.clone()); + mappings.insert(root, base_dir); } - Ok(Self(mappings)) + Ok(Self { + output_dir: ModuleSpecifier::from_directory_path(output_dir).unwrap(), + mappings, + base_specifiers, + }) + } + + pub fn output_dir(&self) -> &ModuleSpecifier { + &self.output_dir + } + + pub fn local_uri(&self, specifier: &ModuleSpecifier) -> ModuleSpecifier { + if specifier.scheme() == "file" { + specifier.clone() + } else { + let local_path = self.local_path(specifier); + if specifier.path().ends_with('/') { + ModuleSpecifier::from_directory_path(&local_path) + } else { + ModuleSpecifier::from_file_path(&local_path) + } + .unwrap_or_else(|_| { + panic!("Could not convert {} to uri.", local_path.display()) + }) + } } - pub fn local_path(&self, specifier: &ModuleSpecifier) -> &PathBuf { + pub fn local_path(&self, specifier: &ModuleSpecifier) -> PathBuf { + if specifier.scheme() == "file" { + specifier.to_file_path().unwrap() + } else { + self + .mappings + .get(specifier) + .as_ref() + .unwrap_or_else(|| { + panic!("Could not find local path for {}", specifier) + }) + .to_path_buf() + } + } + + pub fn relative_path( + &self, + from: &ModuleSpecifier, + to: &ModuleSpecifier, + ) -> String { + let mut from = self.local_uri(from); + let to = self.local_uri(to); + + // workaround using parent directory until https://github.com/servo/rust-url/pull/754 is merged + if !from.path().ends_with('/') { + let local_path = self.local_path(&from); + from = ModuleSpecifier::from_directory_path(local_path.parent().unwrap()) + .unwrap(); + } + + from.make_relative(&to).unwrap() + } + + pub fn relative_specifier_text( + &self, + from: &ModuleSpecifier, + to: &ModuleSpecifier, + ) -> String { + let relative_path = self.relative_path(from, to); + + if relative_path.starts_with("../") || relative_path.starts_with("./") { + relative_path + } else { + format!("./{}", relative_path) + } + } + + pub fn base_specifiers(&self) -> &Vec { + &self.base_specifiers + } + + pub fn base_specifier( + &self, + child_specifier: &ModuleSpecifier, + ) -> &ModuleSpecifier { self - .0 - .get(specifier) - .as_ref() - .unwrap_or_else(|| panic!("Could not find local path for {}", specifier)) + .base_specifiers + .iter() + .find(|s| child_specifier.as_str().starts_with(s.as_str())) + .unwrap_or_else(|| { + panic!("Could not find base specifier for {}", child_specifier) + }) } } fn path_with_extension(path: &Path, ext: &str) -> PathBuf { if let Some(file_stem) = path.file_stem().map(|f| f.to_string_lossy()) { - if path.extension().is_some() && file_stem.to_lowercase().ends_with(".d") { - return path.with_file_name(format!( - "{}.{}", - &file_stem[..file_stem.len() - ".d".len()], - ext - )); + if path.extension().is_some() { + if file_stem.to_lowercase().ends_with(".d") { + return path.with_file_name(format!( + "{}.{}", + &file_stem[..file_stem.len() - ".d".len()], + ext + )); + } + let media_type: MediaType = path.into(); + if media_type == MediaType::Unknown { + return path.with_file_name(format!( + "{}.{}", + path.file_name().unwrap().to_string_lossy(), + ext + )); + } } } path.with_extension(ext) @@ -98,5 +204,9 @@ mod test { path_with_extension(&PathBuf::from("/test.js"), "js"), PathBuf::from("/test.js") ); + assert_eq!( + path_with_extension(&PathBuf::from("/chai@1.2.3"), "js"), + PathBuf::from("/chai@1.2.3.js") + ); } } diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index ea95b03aeb46b..8054447d64372 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -1,16 +1,12 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. -use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; -use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::resolve_url_or_path; -use deno_graph::Module; use deno_graph::ModuleKind; -use deno_graph::Resolved; use deno_runtime::permissions::Permissions; use crate::flags::VendorFlags; @@ -19,18 +15,18 @@ use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; -use self::analyze::collect_remote_module_text_changes; +use self::import_map::build_import_map; use self::mappings::Mappings; use self::specifiers::is_remote_specifier; -use self::specifiers::is_remote_specifier_text; -use self::text_changes::apply_text_changes; mod analyze; +mod import_map; mod mappings; mod specifiers; mod text_changes; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { + // todo: error when someone uses an import map in the vendor folder let output_dir = resolve_and_validate_output_dir(&flags)?; let graph = create_graph(&ps, &flags).await?; let all_modules = graph.modules(); @@ -51,9 +47,10 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { let local_path = mappings.local_path(&module.specifier); let file_text = match module.kind { ModuleKind::Esm => { - let text_changes = + /*let text_changes = collect_remote_module_text_changes(&mappings, module); - apply_text_changes(source, text_changes) + apply_text_changes(source, text_changes)*/ + source.to_string() } ModuleKind::Asserted => source.to_string(), _ => { @@ -70,9 +67,8 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { } // create the import map - if let Some(import_map_text) = - build_import_map(&output_dir, &all_modules, &mappings) - { + if !mappings.base_specifiers().is_empty() { + let import_map_text = build_import_map(&all_modules, &mappings); std::fs::write(output_dir.join("import_map.json"), import_map_text)?; } @@ -164,68 +160,3 @@ async fn create_graph( Ok(graph) } - -fn build_import_map( - output_dir: &Path, - modules: &[&Module], - mappings: &Mappings, -) -> Option { - let key_values = collect_import_map_key_values(modules); - if key_values.is_empty() { - return None; - } - - let output_dir = ModuleSpecifier::from_directory_path(&output_dir).unwrap(); - - // todo: use serde_json to print this out? - let mut text = "{\n".to_string(); - text.push_str(" \"imports\": {\n"); - for (i, (key, value)) in key_values.iter().enumerate() { - if i > 0 { - text.push_str(",\n"); - } - let local_path = mappings.local_path(value); - let local_uri = ModuleSpecifier::from_file_path(&local_path).unwrap(); - let relative_path = output_dir.make_relative(&local_uri).unwrap(); - text.push_str(&format!(" \"{}\": \"./{}\"", key, relative_path)); - } - text.push_str("\n }\n"); - text.push_str("}\n"); - - Some(text) -} - -fn collect_import_map_key_values( - local_modules: &[&Module], -) -> Vec<(String, ModuleSpecifier)> { - fn add_if_remote( - imports: &mut HashMap, - text: &str, - specifier: &ModuleSpecifier, - ) { - if is_remote_specifier_text(text) { - imports.insert(text.to_string(), specifier.clone()); - } - } - - let mut result = HashMap::new(); - for module in local_modules { - for (text, dep) in &module.dependencies { - if let Some(specifier) = dep.get_code() { - add_if_remote(&mut result, text, specifier); - } - if let Some(specifier) = dep.get_type() { - add_if_remote(&mut result, text, specifier); - } - } - if let Some((text, Resolved::Ok { specifier, .. })) = - &module.maybe_types_dependency - { - add_if_remote(&mut result, text, specifier); - } - } - - let mut result = result.into_iter().collect::>(); - result.sort_by(|a, b| a.0.cmp(&b.0)); - result -} diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index 063dcc461dbcf..1fc432ace542b 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -13,6 +13,10 @@ use deno_core::error::AnyError; pub fn partition_by_root_specifiers<'a>( specifiers: impl Iterator, ) -> Vec<(ModuleSpecifier, Vec)> { + // todo: this should do an initial pass where it partitions + // even within paths in domains, then a second pass where it collapses + // them while mapping + let mut root_specifiers: Vec<(ModuleSpecifier, Vec)> = Vec::new(); for remote_specifier in specifiers { @@ -159,9 +163,7 @@ pub fn make_url_relative( root: &ModuleSpecifier, url: &ModuleSpecifier, ) -> Result { - let mut url = url.clone(); - url.set_query(None); - root.make_relative(&url).ok_or_else(|| { + root.make_relative(url).ok_or_else(|| { anyhow!( "Error making url ({}) relative to root: {}", url.to_string(), @@ -174,7 +176,7 @@ pub fn is_remote_specifier(specifier: &ModuleSpecifier) -> bool { specifier.scheme().to_lowercase().starts_with("http") } -pub fn is_remote_specifier_text(text: &str) -> bool { +pub fn is_absolute_specifier_text(text: &str) -> bool { text.trim_start().to_lowercase().starts_with("http") } From 52c78183287f2a837d04ee875267cac3261f8abe Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 10 Feb 2022 09:48:09 -0500 Subject: [PATCH 09/33] - Inspect the source code itself to get the specifier text - Remove windows 260 limit code because it was terrible and I thought of a better solution that can come after the initial implementation. - Remove other code that's no longer used --- cli/tools/vendor/analyze.rs | 140 ------------------------------- cli/tools/vendor/import_map.rs | 114 +++++++++++++++++++------ cli/tools/vendor/mappings.rs | 2 +- cli/tools/vendor/mod.rs | 4 +- cli/tools/vendor/specifiers.rs | 96 ++------------------- cli/tools/vendor/text_changes.rs | 55 ------------ 6 files changed, 101 insertions(+), 310 deletions(-) delete mode 100644 cli/tools/vendor/analyze.rs delete mode 100644 cli/tools/vendor/text_changes.rs diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs deleted file mode 100644 index a4d9c4c2b6561..0000000000000 --- a/cli/tools/vendor/analyze.rs +++ /dev/null @@ -1,140 +0,0 @@ -// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. - -use std::path::Path; -use std::path::PathBuf; - -use deno_ast::LineAndColumnIndex; -use deno_ast::ModuleSpecifier; -use deno_ast::SourceTextInfo; -use deno_graph::Module; -use deno_graph::Position; -use deno_graph::Range; -use deno_graph::Resolved; - -use super::mappings::Mappings; -use super::specifiers::is_absolute_specifier_text; -use super::text_changes::TextChange; - -// todo: delete... not used anymore - -struct Context<'a> { - mappings: &'a Mappings, - module: &'a Module, - text_info: &'a SourceTextInfo, - text: &'a str, - text_changes: Vec, - local_path: PathBuf, -} - -impl<'a> Context<'a> { - pub fn byte_index(&self, pos: &Position) -> usize { - // todo(https://github.com/denoland/deno_graph/issues/79): use byte indexes all the way down - self - .text_info - .byte_index(LineAndColumnIndex { - line_index: pos.line, - column_index: pos.character, - }) - .0 as usize - } - - pub fn byte_range(&self, range: &Range) -> std::ops::Range { - let start = self.byte_index(&range.start); - let end = self.byte_index(&range.end); - start..end - } - - pub fn relative_specifier_text(&self, specifier: &ModuleSpecifier) -> String { - let local_path = self.mappings.local_path(specifier); - get_relative_specifier_text(&self.local_path, &local_path) - } -} - -pub fn collect_remote_module_text_changes<'a>( - mappings: &'a Mappings, - module: &'a Module, -) -> Vec { - let text_info = match &module.maybe_parsed_source { - Some(source) => source.source(), - None => return Vec::new(), - }; - let text = match &module.maybe_source { - Some(source) => source, - None => return Vec::new(), - }; - let mut context = Context { - mappings, - module, - text_info, - text, - text_changes: Vec::new(), - local_path: mappings.local_path(&module.specifier), - }; - - // todo(THIS PR): this is may not good enough because it only includes - // what deno_graph has resolved and may not include everything in the source file? - // Yeah, not enough. - for dep in context.module.dependencies.values() { - handle_maybe_resolved(&dep.maybe_code, &mut context); - handle_maybe_resolved(&dep.maybe_type, &mut context); - } - - // todo(THIS PR): does this contain more than just the header? I think so? - - // resolve x-typescript-types header and inject it as a types directive - if let Some((_, Resolved::Ok { specifier, .. })) = - &context.module.maybe_types_dependency - { - let new_specifier_text = context.relative_specifier_text(specifier); - context.text_changes.push(TextChange::new( - 0, - 0, - format!("/// \n", new_specifier_text), - )) - } - - context.text_changes -} - -fn handle_maybe_resolved(maybe_resolved: &Resolved, context: &mut Context<'_>) { - if let Resolved::Ok { - specifier, range, .. - } = maybe_resolved - { - let mut byte_range = context.byte_range(range); - let mut current_text = &context.text[byte_range.clone()]; - if current_text.starts_with('"') || current_text.starts_with('\'') { - // remove the quotes - byte_range = (byte_range.start + 1)..(byte_range.end - 1); - current_text = &context.text[byte_range.clone()]; - }; - - // leave remote specifiers as-is as they will be handled by the import map - if !is_absolute_specifier_text(current_text) { - let new_specifier_text = context.relative_specifier_text(specifier); - context.text_changes.push(TextChange::new( - byte_range.start, - byte_range.end, - new_specifier_text, - )); - } - } -} - -// todo: delete, I moved it elsewhere -fn get_relative_specifier_text(from: &Path, to: &Path) -> String { - let relative_path = get_relative_path(from, to); - - if relative_path.starts_with("../") || relative_path.starts_with("./") { - relative_path - } else { - format!("./{}", relative_path) - } -} - -// todo: delete -fn get_relative_path(from: &Path, to: &Path) -> String { - let from_path = ModuleSpecifier::from_file_path(from).unwrap(); - let to_path = ModuleSpecifier::from_file_path(to).unwrap(); - from_path.make_relative(&to_path).unwrap() -} diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index b4f9910d4cd20..5fa961d53e452 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -1,8 +1,12 @@ use std::collections::BTreeMap; +use deno_ast::LineAndColumnIndex; use deno_ast::ModuleSpecifier; +use deno_ast::SourceTextInfo; use deno_core::serde_json; use deno_graph::Module; +use deno_graph::Position; +use deno_graph::Range; use deno_graph::Resolved; use serde::Serialize; @@ -104,40 +108,67 @@ fn fill_scopes( import_map: &mut ImportMapBuilder, ) { for module in modules { - for (text, dep) in &module.dependencies { - if let Some(specifier) = dep.get_code() { - handle_dep_specifier( - import_map, - &module.specifier, - text, - specifier, - mappings, - ); - } - if let Some(specifier) = dep.get_type() { - handle_dep_specifier( - import_map, - &module.specifier, - text, - specifier, - mappings, - ); - } + let text_info = match &module.maybe_parsed_source { + Some(source) => source.source(), + None => continue, + }; + let source_text = match &module.maybe_source { + Some(source) => source, + None => continue, + }; + + for dep in module.dependencies.values() { + handle_maybe_resolved( + &dep.maybe_code, + import_map, + &module.specifier, + mappings, + text_info, + source_text, + ); + handle_maybe_resolved( + &dep.maybe_type, + import_map, + &module.specifier, + mappings, + text_info, + source_text, + ); } - if let Some((text, Resolved::Ok { specifier, .. })) = - &module.maybe_types_dependency - { - handle_dep_specifier( + + if let Some((_, maybe_resolved)) = &module.maybe_types_dependency { + handle_maybe_resolved( + maybe_resolved, import_map, &module.specifier, - text, - specifier, mappings, + text_info, + source_text, ); } } } +fn handle_maybe_resolved( + maybe_resolved: &Resolved, + import_map: &mut ImportMapBuilder, + referrer: &ModuleSpecifier, + mappings: &Mappings, + text_info: &SourceTextInfo, + source_text: &str, +) { + if let Resolved::Ok { + specifier, range, .. + } = maybe_resolved + { + let text = text_from_range(text_info, source_text, range); + // if the text is empty then it's probably an x-TypeScript-types + if !text.is_empty() { + handle_dep_specifier(import_map, referrer, text, specifier, mappings); + } + } +} + fn handle_dep_specifier( import_map: &mut ImportMapBuilder, referrer: &ModuleSpecifier, @@ -186,3 +217,36 @@ fn handle_dep_specifier( imports.add(text.to_string(), specifier); } } + +fn text_from_range<'a>( + text_info: &SourceTextInfo, + text: &'a str, + range: &Range, +) -> &'a str { + let result = &text[byte_range(text_info, range)]; + if result.starts_with('"') || result.starts_with('\'') { + // remove the quotes + &result[1..result.len() - 1] + } else { + result + } +} + +fn byte_range( + text_info: &SourceTextInfo, + range: &Range, +) -> std::ops::Range { + let start = byte_index(text_info, &range.start); + let end = byte_index(text_info, &range.end); + start..end +} + +fn byte_index(text_info: &SourceTextInfo, pos: &Position) -> usize { + // todo(https://github.com/denoland/deno_graph/issues/79): use byte indexes all the way down + text_info + .byte_index(LineAndColumnIndex { + line_index: pos.line, + column_index: pos.character, + }) + .0 as usize +} diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index d0647f84ddb34..f604e3116021e 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -38,7 +38,7 @@ impl Mappings { for (root, specifiers) in partitioned_specifiers.into_iter() { let base_dir = get_unique_path( - output_dir.join(dir_name_for_root(&root, &specifiers)), + output_dir.join(dir_name_for_root(&root)), &mut mapped_paths, ); for specifier in specifiers { diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 8054447d64372..e49657ab7dec9 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -19,14 +19,14 @@ use self::import_map::build_import_map; use self::mappings::Mappings; use self::specifiers::is_remote_specifier; -mod analyze; mod import_map; mod mappings; mod specifiers; -mod text_changes; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { // todo: error when someone uses an import map in the vendor folder + // todo: need to handle rewriting out the current import map to the new location? Though probably not possible. + // I think people will need to manually update let output_dir = resolve_and_validate_output_dir(&flags)?; let graph = create_graph(&ps, &flags).await?; let all_modules = graph.modules(); diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index 1fc432ace542b..eb00d211b4d34 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -13,10 +13,6 @@ use deno_core::error::AnyError; pub fn partition_by_root_specifiers<'a>( specifiers: impl Iterator, ) -> Vec<(ModuleSpecifier, Vec)> { - // todo: this should do an initial pass where it partitions - // even within paths in domains, then a second pass where it collapses - // them while mapping - let mut root_specifiers: Vec<(ModuleSpecifier, Vec)> = Vec::new(); for remote_specifier in specifiers { @@ -49,20 +45,8 @@ pub fn partition_by_root_specifiers<'a>( root_specifiers } -/// Gets the flattened directory name to use for the provided root -/// specifier and its descendant specifiers. We use the descendant -/// specifiers to estimate the maximum directory path length in -/// order to truncate the root directory name if necessary due to -/// the 260 character max path length on Windows. -pub fn dir_name_for_root( - root: &ModuleSpecifier, - specifiers: &[ModuleSpecifier], -) -> PathBuf { - // all the provided specifiers should be descendants of the root - debug_assert!(specifiers - .iter() - .all(|s| s.as_str().starts_with(root.as_str()))); - +/// Gets the directory name to use for the provided root. +pub fn dir_name_for_root(root: &ModuleSpecifier) -> PathBuf { let mut result = String::new(); if let Some(domain) = root.domain() { result.push_str(&sanitize_segment(domain)); @@ -73,49 +57,14 @@ pub fn dir_name_for_root( } result.push_str(&port.to_string()); } + let mut result = PathBuf::from(result); if let Some(segments) = root.path_segments() { for segment in segments.filter(|s| !s.is_empty()) { - if !result.is_empty() { - result.push('/'); - } - result.push_str(&sanitize_segment(segment)); + result = result.join(sanitize_segment(segment)); } } - PathBuf::from(if result.is_empty() { - "unknown".to_string() - } else { - // Limit the size of the directory to reduce the chance of max path - // errors on Windows. This uses bytes instead of chars because it's - // faster, but the approximation should be good enough. - let root_len = root.as_str().len(); - let max_specifier_len = specifiers - .iter() - .map(|s| s.as_str().len()) - .max() - .unwrap_or(root_len); - let sub_path_len = max_specifier_len - root_len; - let max_win_path = 260; - // This is the approximate length that a path might be before the root directory. - // It should be a hardcoded number and not calculated based on the system as the - // produced code will most likely not stay only on the system or directory that - // produced it. - let approx_path_prefix_len = 80; - let truncate_len = std::cmp::max( - 10, - max_win_path as isize - - approx_path_prefix_len as isize - - sub_path_len as isize, - ) as usize; - - // if the final text should be truncated, then truncate and - // flatten it to a single folder name - let text = match result.char_indices().nth(truncate_len) { - Some((i, _)) => (&result[..i]).replace('/', "_"), - None => result, - }; - text.trim_end_matches('_').trim_end_matches('.').to_string() - }) + result } /// Gets a path with the specified file stem suffix. @@ -292,47 +241,20 @@ mod test { fn should_get_dir_name_root() { run_test( "http://deno.land/x/test", - &["http://deno.land/x/test/mod.ts"], "deno.land/x/test", ); run_test( "http://localhost", - &["http://localhost/test.mod"], "localhost", ); run_test( - "http://localhost/test%20test", - &["http://localhost/test%20test/asdf"], - "localhost/test%20test", - ); - // will flatten and truncate - run_test( - // length of 45 - "http://localhost/testtestestingtestingtesting", - // length of 210 - &["http://localhost/testtestestingtestingtesting/testingthisoutwithaverlongspecifiertestingtasfasdfasdfasdfadsfasdfasfasdfasfasdfasdfasfasdfasfdasdfasdfasdfasdfasdfsdafasdfasdasdfasdfasdfasdfasdfasdfaasdfasdfas.ts"], - // Max(10, 260 - 80 - (210 - 45)) = 15 chars - "localhost_testt", - ); - // will truncate - run_test( - // length of 45 - "http://localhost/testtestestingtestingtesting", - // length of 220 - &["http://localhost/testtestestingtestingtesting/testingthisoutwithaverlongspecifiertestingtasfasdfasdfasdfadsfasdfasfasdfasfasdfasdfasfasdfasfdasdfasdfasdfasdfasdfsdafasdfasdasdfasdfasdfasdfasdfasdfaasdfasdfasteststttts.ts"], - // Max(10, 260 - 80 - (210 - 45)) = 10 and trim the trailing underscore - "localhost", + "http://localhost/test%20:test", + "localhost/test%20_test", ); - fn run_test(specifier: &str, specifiers: &[&str], expected: &str) { + fn run_test(specifier: &str, expected: &str) { assert_eq!( - dir_name_for_root( - &ModuleSpecifier::parse(specifier).unwrap(), - &specifiers - .iter() - .map(|s| ModuleSpecifier::parse(s).unwrap()) - .collect::>(), - ), + dir_name_for_root(&ModuleSpecifier::parse(specifier).unwrap()), PathBuf::from(expected) ); } diff --git a/cli/tools/vendor/text_changes.rs b/cli/tools/vendor/text_changes.rs deleted file mode 100644 index 75eab44f006a8..0000000000000 --- a/cli/tools/vendor/text_changes.rs +++ /dev/null @@ -1,55 +0,0 @@ -// todo(THIS PR): before merge, delete this file as this functionality is now in deno_ast - -use std::ops::Range; - -#[derive(Clone, Debug)] -pub struct TextChange { - /// Range start to end byte index. - pub range: Range, - /// New text to insert or replace at the provided range. - pub new_text: String, -} - -impl TextChange { - pub fn new(start: usize, end: usize, new_text: String) -> Self { - Self { - range: start..end, - new_text, - } - } -} - -/// Applies the text changes to the given source text. -pub fn apply_text_changes( - source: &str, - mut changes: Vec, -) -> String { - changes.sort_by(|a, b| a.range.start.cmp(&b.range.start)); - - let mut last_index = 0; - let mut final_text = String::new(); - - for change in changes { - if change.range.start > change.range.end { - panic!( - "Text change had start index {} greater than end index {}.", - change.range.start, change.range.end - ) - } - if change.range.start < last_index { - panic!("Text changes were overlapping. Past index was {}, but new change had index {}.", last_index, change.range.start); - } else if change.range.start > last_index && last_index < source.len() { - final_text.push_str( - &source[last_index..std::cmp::min(source.len(), change.range.start)], - ); - } - final_text.push_str(&change.new_text); - last_index = change.range.end; - } - - if last_index < source.len() { - final_text.push_str(&source[last_index..]); - } - - final_text -} From 6a54b3572476430b93338448f4bb656ebd299db6 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 10 Feb 2022 12:42:00 -0500 Subject: [PATCH 10/33] Basic testing infrastructure. --- cli/tools/vendor/build.rs | 110 +++++++++++++ cli/tools/vendor/mod.rs | 55 +------ cli/tools/vendor/specifiers.rs | 17 +- cli/tools/vendor/test.rs | 275 +++++++++++++++++++++++++++++++++ 4 files changed, 395 insertions(+), 62 deletions(-) create mode 100644 cli/tools/vendor/build.rs create mode 100644 cli/tools/vendor/test.rs diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs new file mode 100644 index 0000000000000..82b30a0ed20a9 --- /dev/null +++ b/cli/tools/vendor/build.rs @@ -0,0 +1,110 @@ +use std::path::Path; + +use deno_core::error::AnyError; +use deno_graph::ModuleGraph; +use deno_graph::ModuleKind; + +use super::import_map::build_import_map; +use super::mappings::Mappings; +use super::specifiers::is_remote_specifier; + +pub trait VendorEnvironment { + fn create_dir_all(&self, dir_path: &Path) -> Result<(), AnyError>; + fn write_file(&self, file_path: &Path, text: &str) -> Result<(), AnyError>; +} + +pub struct RealVendorEnvironment; + +impl VendorEnvironment for RealVendorEnvironment { + fn create_dir_all(&self, dir_path: &Path) -> Result<(), AnyError> { + Ok(std::fs::create_dir_all(dir_path)?) + } + + fn write_file(&self, file_path: &Path, text: &str) -> Result<(), AnyError> { + Ok(std::fs::write(file_path, text)?) + } +} + +pub fn build( + graph: &ModuleGraph, + output_dir: &Path, + environment: &impl VendorEnvironment, +) -> Result<(), AnyError> { + let all_modules = graph.modules(); + let remote_modules = all_modules + .iter() + .filter(|m| is_remote_specifier(&m.specifier)) + .copied() + .collect::>(); + let mappings = + Mappings::from_remote_modules(graph, &remote_modules, output_dir)?; + + environment.create_dir_all(output_dir)?; + + // collect and write out all the text changes + for module in &remote_modules { + let source = match &module.maybe_source { + Some(source) => source, + None => continue, + }; + let local_path = mappings.local_path(&module.specifier); + let file_text = match module.kind { + ModuleKind::Esm => { + /*let text_changes = + collect_remote_module_text_changes(&mappings, module); + apply_text_changes(source, text_changes)*/ + source.to_string() + } + ModuleKind::Asserted => source.to_string(), + _ => { + log::warn!( + "Unsupported module kind {:?} for {}", + module.kind, + module.specifier + ); + continue; + } + }; + environment.create_dir_all(local_path.parent().unwrap())?; + environment.write_file(&local_path, &file_text)?; + } + + // create the import map + if !mappings.base_specifiers().is_empty() { + let import_map_text = build_import_map(&all_modules, &mappings); + environment + .write_file(&output_dir.join("import_map.json"), &import_map_text)?; + } + + Ok(()) +} + +#[cfg(test)] +mod test { + use crate::tools::vendor::test::VendorTestBuilder; + use deno_core::serde_json::json; + use pretty_assertions::assert_eq; + + #[tokio::test] + async fn should_handle_remote_files() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .with_loader(|loader| { + loader + .add_local_file("/mod.ts", r#"import "https://localhost/mod.ts";"#) + .add_remote_file("https://localhost/mod.ts", "export class Test {}"); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "https://localhost/": "./localhost" + } + })) + ) + } +} diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index e49657ab7dec9..82764bdd25e31 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -6,7 +6,6 @@ use std::path::PathBuf; use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::resolve_url_or_path; -use deno_graph::ModuleKind; use deno_runtime::permissions::Permissions; use crate::flags::VendorFlags; @@ -15,64 +14,22 @@ use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; -use self::import_map::build_import_map; -use self::mappings::Mappings; -use self::specifiers::is_remote_specifier; - +mod build; mod import_map; mod mappings; mod specifiers; +#[cfg(test)] +mod test; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { // todo: error when someone uses an import map in the vendor folder // todo: need to handle rewriting out the current import map to the new location? Though probably not possible. // I think people will need to manually update + // todo: add integration tests let output_dir = resolve_and_validate_output_dir(&flags)?; let graph = create_graph(&ps, &flags).await?; - let all_modules = graph.modules(); - let remote_modules = all_modules - .iter() - .filter(|m| is_remote_specifier(&m.specifier)) - .copied() - .collect::>(); - let mappings = - Mappings::from_remote_modules(&graph, &remote_modules, &output_dir)?; - - // collect and write out all the text changes - for module in &remote_modules { - let source = match &module.maybe_source { - Some(source) => source, - None => continue, - }; - let local_path = mappings.local_path(&module.specifier); - let file_text = match module.kind { - ModuleKind::Esm => { - /*let text_changes = - collect_remote_module_text_changes(&mappings, module); - apply_text_changes(source, text_changes)*/ - source.to_string() - } - ModuleKind::Asserted => source.to_string(), - _ => { - log::warn!( - "Unsupported module kind {:?} for {}", - module.kind, - module.specifier - ); - continue; - } - }; - std::fs::create_dir_all(local_path.parent().unwrap())?; - std::fs::write(local_path, file_text)?; - } - - // create the import map - if !mappings.base_specifiers().is_empty() { - let import_map_text = build_import_map(&all_modules, &mappings); - std::fs::write(output_dir.join("import_map.json"), import_map_text)?; - } - Ok(()) + build::build(&graph, &output_dir, &build::RealVendorEnvironment) } fn resolve_and_validate_output_dir( @@ -112,7 +69,7 @@ async fn create_graph( .collect::, AnyError>>()?; // todo(dsherret): there is a lot of copy and paste here from - // other parts of the codebase + // other parts of the codebase. We should consolidate this. let mut cache = crate::cache::FetchCacher::new( ps.dir.gen_cache.clone(), ps.file_fetcher.clone(), diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index eb00d211b4d34..f6b3d3f3b9f6a 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -100,7 +100,7 @@ pub fn get_unique_path( ) -> PathBuf { let original_path = path.clone(); let mut count = 2; - // case insensitive comparison for case insensitive file systems + // case insensitive comparison so the output works on case insensitive file systems while !unique_set.insert(path.to_string_lossy().to_lowercase()) { path = path_with_stem_suffix(&original_path, &count.to_string()); count += 1; @@ -239,18 +239,9 @@ mod test { #[test] fn should_get_dir_name_root() { - run_test( - "http://deno.land/x/test", - "deno.land/x/test", - ); - run_test( - "http://localhost", - "localhost", - ); - run_test( - "http://localhost/test%20:test", - "localhost/test%20_test", - ); + run_test("http://deno.land/x/test", "deno.land/x/test"); + run_test("http://localhost", "localhost"); + run_test("http://localhost/test%20:test", "localhost/test%20_test"); fn run_test(specifier: &str, expected: &str) { assert_eq!( diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs new file mode 100644 index 0000000000000..4d918077e854c --- /dev/null +++ b/cli/tools/vendor/test.rs @@ -0,0 +1,275 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + +use std::cell::RefCell; +use std::collections::HashMap; +use std::collections::HashSet; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; + +use deno_ast::ModuleSpecifier; +use deno_core::anyhow::anyhow; +use deno_core::anyhow::bail; +use deno_core::error::AnyError; +use deno_core::futures; +use deno_core::serde_json; +use deno_graph::source::LoadFuture; +use deno_graph::source::LoadResponse; +use deno_graph::source::Loader; +use deno_graph::ModuleGraph; + +use super::build::VendorEnvironment; + +// Utilities that help `deno vendor` edge cases get tested in memory. +// Note that utilities are still required. + +type RemoteFileText = String; +type RemoteFileHeaders = Option>; +type RemoteFileResult = Result<(RemoteFileText, RemoteFileHeaders), String>; + +#[derive(Clone, Default)] +pub struct InMemoryLoader { + local_files: HashMap, + remote_files: HashMap, + redirects: HashMap, +} + +impl InMemoryLoader { + pub fn add_local_file( + &mut self, + path: impl AsRef, + text: impl AsRef, + ) -> &mut Self { + let path = make_path(path.as_ref()); + self + .local_files + .insert(path, text.as_ref().to_string()); + self + } + + pub fn add_remote_file( + &mut self, + specifier: impl AsRef, + text: impl AsRef, + ) -> &mut Self { + self.remote_files.insert( + ModuleSpecifier::parse(specifier.as_ref()).unwrap(), + Ok((text.as_ref().to_string(), None)), + ); + self + } + + pub fn add_remote_file_with_headers( + &mut self, + specifier: impl AsRef, + text: impl AsRef, + headers: &[(&str, &str)], + ) -> &mut Self { + let headers = headers + .iter() + .map(|(key, value)| (key.to_string(), value.to_string())) + .collect(); + self.remote_files.insert( + ModuleSpecifier::parse(specifier.as_ref()).unwrap(), + Ok((text.as_ref().to_string(), Some(headers))), + ); + self + } + + pub fn add_remote_file_with_error( + &mut self, + specifier: impl AsRef, + error_text: impl AsRef, + ) -> &mut Self { + self.remote_files.insert( + ModuleSpecifier::parse(specifier.as_ref()).unwrap(), + Err(error_text.as_ref().to_string()), + ); + self + } + + pub fn add_redirect( + &mut self, + from: impl AsRef, + to: impl AsRef, + ) -> &mut Self { + self.redirects.insert( + ModuleSpecifier::parse(from.as_ref()).unwrap(), + ModuleSpecifier::parse(to.as_ref()).unwrap(), + ); + self + } +} + +impl Loader for InMemoryLoader { + fn load( + &mut self, + specifier: &ModuleSpecifier, + _is_dynamic: bool, + ) -> LoadFuture { + if specifier.scheme() == "file" { + let file_path = specifier.to_file_path().unwrap(); + let result = self.local_files.get(&file_path).map(ToOwned::to_owned); + let specifier = specifier.clone(); + return Box::pin(async move { + Ok(result.map(|result| LoadResponse { + content: Arc::new(result), + maybe_headers: None, + specifier, + })) + }); + } + let specifier = self.redirects.get(specifier).unwrap_or(specifier); + let result = self.remote_files.get(specifier).map(|result| match result { + Ok(result) => Ok(LoadResponse { + specifier: specifier.clone(), + content: Arc::new(result.0.clone()), + maybe_headers: result.1.clone(), + }), + Err(err) => Err(err), + }); + let result = match result { + Some(Ok(result)) => Ok(Some(result)), + Some(Err(err)) => Err(anyhow!("{}", err.to_string())), + None => Ok(None), + }; + Box::pin(futures::future::ready(result)) + } +} + +#[derive(Default)] +struct TestVendorEnvironment { + directories: RefCell>, + files: RefCell>, +} + +impl VendorEnvironment for TestVendorEnvironment { + fn create_dir_all(&self, dir_path: &Path) -> Result<(), AnyError> { + let dir_path = dir_path.to_path_buf(); + let mut directories = self.directories.borrow_mut(); + if !directories.insert(dir_path.clone()) { + for path in dir_path.ancestors() { + if !directories.insert(path.to_path_buf()) { + break; + } + } + } + Ok(()) + } + + fn write_file(&self, file_path: &Path, text: &str) -> Result<(), AnyError> { + if !self + .directories + .borrow() + .contains(file_path.parent().unwrap()) + { + bail!( + "Directory not found: {}", + file_path.parent().unwrap().display() + ); + } + self + .files + .borrow_mut() + .insert(file_path.to_path_buf(), text.to_string()); + Ok(()) + } +} + +pub struct VendorOutput { + pub files: Vec<(String, String)>, + pub import_map: Option, +} + +#[derive(Default)] +pub struct VendorTestBuilder { + entry_points: Vec, + loader: InMemoryLoader, +} + +impl VendorTestBuilder { + pub fn with_default_setup() -> Self { + let mut builder = VendorTestBuilder::default(); + builder.add_entry_point("/mod.ts"); + builder + } + + pub fn add_entry_point(&mut self, entry_point: impl AsRef) -> &mut Self { + let entry_point = make_path(entry_point.as_ref()); + self + .entry_points + .push(ModuleSpecifier::from_file_path(entry_point).unwrap()); + self + } + + pub async fn build(&mut self) -> Result { + let graph = self.build_graph().await; + let output_dir = make_path("/vendor"); + let environment = TestVendorEnvironment::default(); + super::build::build(&graph, &output_dir, &environment)?; + let mut files = environment.files.borrow_mut(); + let import_map = files.remove(&output_dir.join("import_map.json")); + let mut files = files + .iter() + .map(|(path, text)| (path_to_string(path), text.clone())) + .collect::>(); + + files.sort_by(|a, b| a.0.cmp(&b.0)); + + Ok(VendorOutput { + import_map: import_map.map(|text| serde_json::from_str(&text).unwrap()), + files, + }) + } + + pub fn with_loader( + &mut self, + action: impl Fn(&mut InMemoryLoader), + ) -> &mut Self { + action(&mut self.loader); + self + } + + async fn build_graph(&mut self) -> ModuleGraph { + let graph = deno_graph::create_graph( + self + .entry_points + .iter() + .map(|s| (s.to_owned(), deno_graph::ModuleKind::Esm)) + .collect(), + false, + None, + &mut self.loader, + None, + None, + None, + None, + ) + .await; + graph.lock().unwrap(); + graph.valid().unwrap(); + graph + } +} + +fn make_path(text: &str) -> PathBuf { + // This should work all in memory. We're waiting on + // https://github.com/servo/rust-url/issues/730 to provide + // a cross platform path here + assert!(text.starts_with('/')); + if cfg!(windows) { + PathBuf::from(format!("C:{}", text.replace("/", "\\"))) + } else { + PathBuf::from(text) + } +} + +fn path_to_string(path: &Path) -> String { + // inverse of the function above + let path = path.to_string_lossy(); + if cfg!(windows) { + path.replace("C:\\", "\\").replace('\\', "/") + } else { + path.to_string() + } +} From 677f4923193aae4e28e1b2e09db98f3107b16e6d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 10 Feb 2022 17:08:11 -0500 Subject: [PATCH 11/33] Fix more issues. It seems to be working decently now. --- cli/tools/vendor/build.rs | 170 ++++++++++++++++++++++++++++----- cli/tools/vendor/import_map.rs | 77 ++++++++++----- cli/tools/vendor/mod.rs | 3 +- cli/tools/vendor/test.rs | 16 +--- 4 files changed, 202 insertions(+), 64 deletions(-) diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index 82b30a0ed20a9..5deb9dd67ce27 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -48,30 +48,21 @@ pub fn build( None => continue, }; let local_path = mappings.local_path(&module.specifier); - let file_text = match module.kind { - ModuleKind::Esm => { - /*let text_changes = - collect_remote_module_text_changes(&mappings, module); - apply_text_changes(source, text_changes)*/ - source.to_string() - } - ModuleKind::Asserted => source.to_string(), - _ => { - log::warn!( - "Unsupported module kind {:?} for {}", - module.kind, - module.specifier - ); - continue; - } - }; + if !matches!(module.kind, ModuleKind::Esm | ModuleKind::Asserted) { + log::warn!( + "Unsupported module kind {:?} for {}", + module.kind, + module.specifier + ); + continue; + } environment.create_dir_all(local_path.parent().unwrap())?; - environment.write_file(&local_path, &file_text)?; + environment.write_file(&local_path, source)?; } // create the import map if !mappings.base_specifiers().is_empty() { - let import_map_text = build_import_map(&all_modules, &mappings); + let import_map_text = build_import_map(graph, &all_modules, &mappings); environment .write_file(&output_dir.join("import_map.json"), &import_map_text)?; } @@ -86,13 +77,28 @@ mod test { use pretty_assertions::assert_eq; #[tokio::test] - async fn should_handle_remote_files() { + async fn local_specifiers_to_remote() { let mut builder = VendorTestBuilder::with_default_setup(); let output = builder .with_loader(|loader| { loader - .add_local_file("/mod.ts", r#"import "https://localhost/mod.ts";"#) - .add_remote_file("https://localhost/mod.ts", "export class Test {}"); + .add_local_file( + "/mod.ts", + concat!( + r#"import "https://localhost/mod.ts";"#, + r#"import "https://localhost/other.ts?test";"#, + r#"import "https://localhost/redirect.ts";"#, + ), + ) + .add_remote_file("https://localhost/mod.ts", "export class Mod {}") + .add_remote_file( + "https://localhost/other.ts?test", + "export class Other {}", + ) + .add_redirect( + "https://localhost/redirect.ts", + "https://localhost/mod.ts", + ); }) .build() .await @@ -102,9 +108,125 @@ mod test { output.import_map, Some(json!({ "imports": { - "https://localhost/": "./localhost" + "https://localhost/": "./localhost", + "https://localhost/other.ts?test": "./localhost/other.ts", + "https://localhost/redirect.ts": "./localhost/mod.ts", } })) - ) + ); + assert_eq!( + output.files, + to_file_vec(&[ + ("/vendor/localhost/mod.ts", "export class Mod {}"), + ("/vendor/localhost/other.ts", "export class Other {}"), + ]), + ); + } + + #[tokio::test] + async fn remote_specifiers() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .with_loader(|loader| { + loader + .add_local_file( + "/mod.ts", + concat!( + r#"import "https://localhost/mod.ts";"#, + r#"import "https://other/mod.ts";"#, + ), + ) + .add_remote_file( + "https://localhost/mod.ts", + concat!( + "export * from './other.ts';", + "export * from './redirect.ts';", + "export * from '/absolute.ts';", + ), + ) + .add_remote_file( + "https://localhost/other.ts", + "export class Other {}", + ) + .add_redirect( + "https://localhost/redirect.ts", + "https://localhost/other.ts", + ) + .add_remote_file( + "https://localhost/absolute.ts", + "export class Absolute {}", + ) + .add_remote_file( + "https://other/mod.ts", + "export * from './sub/mod.ts';", + ) + .add_remote_file( + "https://other/sub/mod.ts", + concat!( + "export * from '../sub2/mod.ts';", + "export * from '../sub2/other?asdf';", + ), + ) + .add_remote_file("https://other/sub2/mod.ts", "export class Mod {}") + .add_remote_file_with_headers( + "https://other/sub2/other?asdf", + "export class Other {}", + &[("content-type", "application/javascript")], + ); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "https://localhost/": "./localhost", + "https://other/": "./other" + }, + "scopes": { + "./localhost": { + "/absolute.ts": "./localhost/absolute.ts", + "./localhost/redirect.ts": "./localhost/other.ts", + }, + "./other": { + "./other/sub2/other?asdf": "./other/sub2/other.js" + } + } + })) + ); + assert_eq!( + output.files, + to_file_vec(&[ + ("/vendor/localhost/absolute.ts", "export class Absolute {}"), + ( + "/vendor/localhost/mod.ts", + concat!( + "export * from './other.ts';", + "export * from './redirect.ts';", + "export * from '/absolute.ts';", + ) + ), + ("/vendor/localhost/other.ts", "export class Other {}"), + ("/vendor/other/mod.ts", "export * from './sub/mod.ts';"), + ( + "/vendor/other/sub/mod.ts", + concat!( + "export * from '../sub2/mod.ts';", + "export * from '../sub2/other?asdf';", + ) + ), + ("/vendor/other/sub2/mod.ts", "export class Mod {}"), + ("/vendor/other/sub2/other.js", "export class Other {}"), + ]), + ); + } + + fn to_file_vec(items: &[(&str, &str)]) -> Vec<(String, String)> { + items + .iter() + .map(|(f, t)| (f.to_string(), t.to_string())) + .collect() } } diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index 5fa961d53e452..8c9cb20292e39 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -5,6 +5,7 @@ use deno_ast::ModuleSpecifier; use deno_ast::SourceTextInfo; use deno_core::serde_json; use deno_graph::Module; +use deno_graph::ModuleGraph; use deno_graph::Position; use deno_graph::Range; use deno_graph::Resolved; @@ -62,7 +63,9 @@ impl<'a> ImportMapBuilder<'a> { } pub fn into_file_text(self) -> String { - serde_json::to_string_pretty(&self.into_serializable()).unwrap() + let mut text = serde_json::to_string_pretty(&self.into_serializable()).unwrap(); + text.push('\n'); + text } } @@ -89,9 +92,13 @@ impl<'a> ImportsBuilder<'a> { } } -pub fn build_import_map(modules: &[&Module], mappings: &Mappings) -> String { +pub fn build_import_map( + graph: &ModuleGraph, + modules: &[&Module], + mappings: &Mappings, +) -> String { let mut import_map = ImportMapBuilder::new(mappings); - fill_scopes(modules, mappings, &mut import_map); + visit_modules(graph, modules, mappings, &mut import_map); for base_specifier in mappings.base_specifiers() { import_map @@ -102,7 +109,8 @@ pub fn build_import_map(modules: &[&Module], mappings: &Mappings) -> String { import_map.into_file_text() } -fn fill_scopes( +fn visit_modules( + graph: &ModuleGraph, modules: &[&Module], mappings: &Mappings, import_map: &mut ImportMapBuilder, @@ -118,16 +126,18 @@ fn fill_scopes( }; for dep in module.dependencies.values() { - handle_maybe_resolved( + visit_maybe_resolved( &dep.maybe_code, + graph, import_map, &module.specifier, mappings, text_info, source_text, ); - handle_maybe_resolved( + visit_maybe_resolved( &dep.maybe_type, + graph, import_map, &module.specifier, mappings, @@ -137,8 +147,9 @@ fn fill_scopes( } if let Some((_, maybe_resolved)) = &module.maybe_types_dependency { - handle_maybe_resolved( + visit_maybe_resolved( maybe_resolved, + graph, import_map, &module.specifier, mappings, @@ -149,8 +160,9 @@ fn fill_scopes( } } -fn handle_maybe_resolved( +fn visit_maybe_resolved( maybe_resolved: &Resolved, + graph: &ModuleGraph, import_map: &mut ImportMapBuilder, referrer: &ModuleSpecifier, mappings: &Mappings, @@ -164,24 +176,28 @@ fn handle_maybe_resolved( let text = text_from_range(text_info, source_text, range); // if the text is empty then it's probably an x-TypeScript-types if !text.is_empty() { - handle_dep_specifier(import_map, referrer, text, specifier, mappings); + handle_dep_specifier( + text, specifier, graph, import_map, referrer, mappings, + ); } } } fn handle_dep_specifier( + text: &str, + unresolved_specifier: &ModuleSpecifier, + graph: &ModuleGraph, import_map: &mut ImportMapBuilder, referrer: &ModuleSpecifier, - text: &str, - specifier: &ModuleSpecifier, mappings: &Mappings, ) { + let specifier = graph.resolve(unresolved_specifier); // do not handle specifiers pointing at local modules - if !is_remote_specifier(specifier) { + if !is_remote_specifier(&specifier) { return; } - let base_specifier = mappings.base_specifier(specifier); + let base_specifier = mappings.base_specifier(&specifier); if is_absolute_specifier_text(text) { if !text.starts_with(base_specifier.as_str()) { panic!("Expected {} to start with {}", text, base_specifier); @@ -189,32 +205,45 @@ fn handle_dep_specifier( let sub_path = &text[base_specifier.as_str().len()..]; let expected_relative_specifier_text = - mappings.relative_path(base_specifier, specifier); + mappings.relative_path(base_specifier, &specifier); if expected_relative_specifier_text == sub_path { return; } - println!( - "File system text: {} || Sub path: {}", - expected_relative_specifier_text, sub_path - ); if !is_remote_specifier(referrer) { - import_map.imports.add(text.to_string(), specifier); + import_map.imports.add(text.to_string(), &specifier); } else { let imports = import_map.scope(base_specifier); - imports.add(sub_path.to_string(), specifier); + imports.add(sub_path.to_string(), &specifier); } } else { let expected_relative_specifier_text = - mappings.relative_specifier_text(referrer, specifier); + mappings.relative_specifier_text(referrer, &specifier); if expected_relative_specifier_text == text { return; } - // println!("File system text: {} || Actual: {}", file_system_specifier_text, text); + let key = if text.starts_with("./") || text.starts_with("../") { + // resolve relative specifier key + let mut local_base_specifier = mappings.local_uri(base_specifier); + local_base_specifier = local_base_specifier + .join(&unresolved_specifier.path()[1..]) + .unwrap_or_else(|_| { + panic!( + "Error joining {} to {}", + unresolved_specifier.path(), + local_base_specifier + ) + }); + local_base_specifier.set_query(unresolved_specifier.query()); + mappings + .relative_specifier_text(mappings.output_dir(), &local_base_specifier) + } else { + // absolute (`/`) or bare specifier should be left as-is + text.to_string() + }; let imports = import_map.scope(base_specifier); - // todo: wrong - imports.add(text.to_string(), specifier); + imports.add(key, &specifier); } } diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 82764bdd25e31..da27fdf15693b 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -23,9 +23,10 @@ mod test; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { // todo: error when someone uses an import map in the vendor folder - // todo: need to handle rewriting out the current import map to the new location? Though probably not possible. + // todo: need to handle rewriting out the current import map to the new location? Doesn't seem possible. // I think people will need to manually update // todo: add integration tests + // todo: add x-TypeScript-types support via proxy file let output_dir = resolve_and_validate_output_dir(&flags)?; let graph = create_graph(&ps, &flags).await?; diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index 4d918077e854c..5049713a934cc 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -41,9 +41,7 @@ impl InMemoryLoader { text: impl AsRef, ) -> &mut Self { let path = make_path(path.as_ref()); - self - .local_files - .insert(path, text.as_ref().to_string()); + self.local_files.insert(path, text.as_ref().to_string()); self } @@ -76,18 +74,6 @@ impl InMemoryLoader { self } - pub fn add_remote_file_with_error( - &mut self, - specifier: impl AsRef, - error_text: impl AsRef, - ) -> &mut Self { - self.remote_files.insert( - ModuleSpecifier::parse(specifier.as_ref()).unwrap(), - Err(error_text.as_ref().to_string()), - ); - self - } - pub fn add_redirect( &mut self, from: impl AsRef, From 8317c8760914fb484ad8053e045eaf5137e990f9 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 11 Feb 2022 10:25:04 -0500 Subject: [PATCH 12/33] Maintain casing of extensions when able --- cli/tools/vendor/build.rs | 52 ++++++++++++++++++++++++++++++++++ cli/tools/vendor/import_map.rs | 7 +++-- cli/tools/vendor/mappings.rs | 28 ++++++++++++++---- cli/tools/vendor/mod.rs | 1 + cli/tools/vendor/specifiers.rs | 9 +++--- 5 files changed, 84 insertions(+), 13 deletions(-) diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index 5deb9dd67ce27..a7f6dd1fbab5d 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -223,6 +223,58 @@ mod test { ); } + #[tokio::test] + async fn same_target_filename_specifiers() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .with_loader(|loader| { + loader + .add_local_file( + "/mod.ts", + concat!( + r#"import "https://localhost/MOD.TS";"#, + r#"import "https://localhost/mod.TS";"#, + r#"import "https://localhost/mod.ts";"#, + r#"import "https://localhost/mod.ts?test";"#, + r#"import "https://localhost/CAPS.TS";"#, + ), + ) + .add_remote_file("https://localhost/MOD.TS", "export class Mod {}") + .add_remote_file("https://localhost/mod.TS", "export class Mod2 {}") + .add_remote_file("https://localhost/mod.ts", "export class Mod3 {}") + .add_remote_file( + "https://localhost/mod.ts?test", + "export class Mod4 {}", + ) + .add_remote_file("https://localhost/CAPS.TS", "export class Caps {}"); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "https://localhost/": "./localhost", + "https://localhost/mod.TS": "./localhost/mod_2.TS", + "https://localhost/mod.ts": "./localhost/mod_3.ts", + "https://localhost/mod.ts?test": "./localhost/mod_4.ts", + } + })) + ); + assert_eq!( + output.files, + to_file_vec(&[ + ("/vendor/localhost/CAPS.TS", "export class Caps {}"), + ("/vendor/localhost/MOD.TS", "export class Mod {}"), + ("/vendor/localhost/mod_2.TS", "export class Mod2 {}"), + ("/vendor/localhost/mod_3.ts", "export class Mod3 {}"), + ("/vendor/localhost/mod_4.ts", "export class Mod4 {}"), + ]), + ); + } + fn to_file_vec(items: &[(&str, &str)]) -> Vec<(String, String)> { items .iter() diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index 8c9cb20292e39..e2877ce44e286 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -12,8 +12,8 @@ use deno_graph::Resolved; use serde::Serialize; use super::mappings::Mappings; -use super::specifiers::is_absolute_specifier_text; use super::specifiers::is_remote_specifier; +use super::specifiers::is_remote_specifier_text; #[derive(Serialize)] struct SerializableImportMap { @@ -63,7 +63,8 @@ impl<'a> ImportMapBuilder<'a> { } pub fn into_file_text(self) -> String { - let mut text = serde_json::to_string_pretty(&self.into_serializable()).unwrap(); + let mut text = + serde_json::to_string_pretty(&self.into_serializable()).unwrap(); text.push('\n'); text } @@ -198,7 +199,7 @@ fn handle_dep_specifier( } let base_specifier = mappings.base_specifier(&specifier); - if is_absolute_specifier_text(text) { + if is_remote_specifier_text(text) { if !text.starts_with(base_specifier.as_str()) { panic!("Expected {} to start with {}", text, base_specifier); } diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index f604e3116021e..822bf92f8e89e 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -155,31 +155,41 @@ impl Mappings { } } -fn path_with_extension(path: &Path, ext: &str) -> PathBuf { +fn path_with_extension(path: &Path, new_ext: &str) -> PathBuf { if let Some(file_stem) = path.file_stem().map(|f| f.to_string_lossy()) { - if path.extension().is_some() { + if let Some(old_ext) = path.extension().map(|f| f.to_string_lossy()) { if file_stem.to_lowercase().ends_with(".d") { + if new_ext.to_lowercase() == format!("d.{}", old_ext.to_lowercase()) { + // maintain casing + return path.to_path_buf(); + } return path.with_file_name(format!( "{}.{}", &file_stem[..file_stem.len() - ".d".len()], - ext + new_ext )); } + if new_ext.to_lowercase() == old_ext.to_lowercase() { + // maintain casing + return path.to_path_buf(); + } let media_type: MediaType = path.into(); if media_type == MediaType::Unknown { return path.with_file_name(format!( "{}.{}", path.file_name().unwrap().to_string_lossy(), - ext + new_ext )); } } } - path.with_extension(ext) + path.with_extension(new_ext) } #[cfg(test)] mod test { + use pretty_assertions::assert_eq; + use super::*; #[test] @@ -194,7 +204,13 @@ mod test { ); assert_eq!( path_with_extension(&PathBuf::from("/test.D.TS"), "d.ts"), - PathBuf::from("/test.d.ts") + // maintains casing + PathBuf::from("/test.D.TS"), + ); + assert_eq!( + path_with_extension(&PathBuf::from("/test.TS"), "ts"), + // maintains casing + PathBuf::from("/test.TS"), ); assert_eq!( path_with_extension(&PathBuf::from("/test.ts"), "js"), diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index da27fdf15693b..f9ec115f4b03e 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -27,6 +27,7 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { // I think people will need to manually update // todo: add integration tests // todo: add x-TypeScript-types support via proxy file + // todo: test for data url let output_dir = resolve_and_validate_output_dir(&flags)?; let graph = create_graph(&ps, &flags).await?; diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index f6b3d3f3b9f6a..031f021799fcc 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -74,9 +74,11 @@ pub fn path_with_stem_suffix(path: &Path, suffix: &str) -> PathBuf { if let Some(ext) = path.extension().map(|f| f.to_string_lossy()) { return if file_stem.to_lowercase().ends_with(".d") { path.with_file_name(format!( - "{}_{}.d.{}", + "{}_{}.{}.{}", &file_stem[..file_stem.len() - ".d".len()], suffix, + // maintain casing + &file_stem[file_stem.len() - "d".len()..], ext )) } else { @@ -125,7 +127,7 @@ pub fn is_remote_specifier(specifier: &ModuleSpecifier) -> bool { specifier.scheme().to_lowercase().starts_with("http") } -pub fn is_absolute_specifier_text(text: &str) -> bool { +pub fn is_remote_specifier_text(text: &str) -> bool { text.trim_start().to_lowercase().starts_with("http") } @@ -279,8 +281,7 @@ mod test { ); assert_eq!( path_with_stem_suffix(&PathBuf::from("/test.D.TS"), "2"), - // good enough - PathBuf::from("/test_2.d.TS") + PathBuf::from("/test_2.D.TS") ); assert_eq!( path_with_stem_suffix(&PathBuf::from("/test.d.mts"), "2"), From cf746ae5a4bec564457e7f4a788b6226a395f928 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 11 Feb 2022 14:59:52 -0500 Subject: [PATCH 13/33] More tests. --- Cargo.lock | 4 +- cli/Cargo.toml | 2 +- cli/tools/vendor/build.rs | 126 ++++++++++++++++++++++++++++---------- cli/tools/vendor/test.rs | 76 ++++++++++------------- 4 files changed, 129 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 49d478a00be60..5dfc9443f68b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1935,9 +1935,9 @@ checksum = "cb56e1aa765b4b4f3aadfab769793b7087bb03a4ea4920644a6d238e2df5b9ed" [[package]] name = "import_map" -version = "0.8.0" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09ae88504e9128c4c181a0a4726d868d52aa76de270c7fb00c3c40a8f4fbace4" +checksum = "f99e0f89d56c163538ea6bf1f250049669298a26daeee15a9a18f4118cc503f1" dependencies = [ "indexmap", "log", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 1943a635f35f5..6e4f17108f76e 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -68,7 +68,7 @@ encoding_rs = "=0.8.29" env_logger = "=0.8.4" fancy-regex = "=0.7.1" http = "=0.2.4" -import_map = "=0.8.0" +import_map = "=0.9.0" jsonc-parser = { version = "=0.19.0", features = ["serde"] } libc = "=0.2.106" log = { version = "=0.4.14", features = ["serde"] } diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index a7f6dd1fbab5d..ef793f349847e 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -76,13 +76,28 @@ mod test { use deno_core::serde_json::json; use pretty_assertions::assert_eq; + #[tokio::test] + async fn no_remote_modules() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .with_loader(|loader| { + loader.add("/mod.ts", ""); + }) + .build() + .await + .unwrap(); + + assert_eq!(output.import_map, None,); + assert_eq!(output.files, vec![],); + } + #[tokio::test] async fn local_specifiers_to_remote() { let mut builder = VendorTestBuilder::with_default_setup(); let output = builder .with_loader(|loader| { loader - .add_local_file( + .add( "/mod.ts", concat!( r#"import "https://localhost/mod.ts";"#, @@ -90,11 +105,8 @@ mod test { r#"import "https://localhost/redirect.ts";"#, ), ) - .add_remote_file("https://localhost/mod.ts", "export class Mod {}") - .add_remote_file( - "https://localhost/other.ts?test", - "export class Other {}", - ) + .add("https://localhost/mod.ts", "export class Mod {}") + .add("https://localhost/other.ts?test", "export class Other {}") .add_redirect( "https://localhost/redirect.ts", "https://localhost/mod.ts", @@ -129,14 +141,14 @@ mod test { let output = builder .with_loader(|loader| { loader - .add_local_file( + .add( "/mod.ts", concat!( r#"import "https://localhost/mod.ts";"#, r#"import "https://other/mod.ts";"#, ), ) - .add_remote_file( + .add( "https://localhost/mod.ts", concat!( "export * from './other.ts';", @@ -144,31 +156,22 @@ mod test { "export * from '/absolute.ts';", ), ) - .add_remote_file( - "https://localhost/other.ts", - "export class Other {}", - ) + .add("https://localhost/other.ts", "export class Other {}") .add_redirect( "https://localhost/redirect.ts", "https://localhost/other.ts", ) - .add_remote_file( - "https://localhost/absolute.ts", - "export class Absolute {}", - ) - .add_remote_file( - "https://other/mod.ts", - "export * from './sub/mod.ts';", - ) - .add_remote_file( + .add("https://localhost/absolute.ts", "export class Absolute {}") + .add("https://other/mod.ts", "export * from './sub/mod.ts';") + .add( "https://other/sub/mod.ts", concat!( "export * from '../sub2/mod.ts';", "export * from '../sub2/other?asdf';", ), ) - .add_remote_file("https://other/sub2/mod.ts", "export class Mod {}") - .add_remote_file_with_headers( + .add("https://other/sub2/mod.ts", "export class Mod {}") + .add_remote_with_headers( "https://other/sub2/other?asdf", "export class Other {}", &[("content-type", "application/javascript")], @@ -229,7 +232,7 @@ mod test { let output = builder .with_loader(|loader| { loader - .add_local_file( + .add( "/mod.ts", concat!( r#"import "https://localhost/MOD.TS";"#, @@ -239,14 +242,11 @@ mod test { r#"import "https://localhost/CAPS.TS";"#, ), ) - .add_remote_file("https://localhost/MOD.TS", "export class Mod {}") - .add_remote_file("https://localhost/mod.TS", "export class Mod2 {}") - .add_remote_file("https://localhost/mod.ts", "export class Mod3 {}") - .add_remote_file( - "https://localhost/mod.ts?test", - "export class Mod4 {}", - ) - .add_remote_file("https://localhost/CAPS.TS", "export class Caps {}"); + .add("https://localhost/MOD.TS", "export class Mod {}") + .add("https://localhost/mod.TS", "export class Mod2 {}") + .add("https://localhost/mod.ts", "export class Mod3 {}") + .add("https://localhost/mod.ts?test", "export class Mod4 {}") + .add("https://localhost/CAPS.TS", "export class Caps {}"); }) .build() .await @@ -275,6 +275,68 @@ mod test { ); } + #[tokio::test] + async fn json_module() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .with_loader(|loader| { + loader + .add( + "/mod.ts", + r#"import data from "https://localhost/data.json" assert { type: "json" };"#, + ) + .add("https://localhost/data.json", "{}"); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "https://localhost/": "./localhost" + } + })) + ); + assert_eq!( + output.files, + to_file_vec(&[("/vendor/localhost/data.json", "{}"),]), + ); + } + + #[tokio::test] + async fn data_urls() { + let mut builder = VendorTestBuilder::with_default_setup(); + + let mod_file_text = format!( + r#"import * as b from "data:application/typescript,export%20*%20from%20%22https://localhost/mod.ts%22;";"# + ); + + let output = builder + .with_loader(|loader| { + loader + .add("/mod.ts", &mod_file_text) + .add("https://localhost/mod.ts", "export class Example {}"); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "https://localhost/": "./localhost" + } + })) + ); + assert_eq!( + output.files, + to_file_vec(&[("/vendor/localhost/mod.ts", "export class Example {}"),]), + ); + } + fn to_file_vec(items: &[(&str, &str)]) -> Vec<(String, String)> { items .iter() diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index 5049713a934cc..dcfba6c158796 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -20,44 +20,44 @@ use deno_graph::ModuleGraph; use super::build::VendorEnvironment; -// Utilities that help `deno vendor` edge cases get tested in memory. -// Note that utilities are still required. +// Utilities that help `deno vendor` get tested in memory. type RemoteFileText = String; type RemoteFileHeaders = Option>; type RemoteFileResult = Result<(RemoteFileText, RemoteFileHeaders), String>; #[derive(Clone, Default)] -pub struct InMemoryLoader { - local_files: HashMap, - remote_files: HashMap, +pub struct TestLoader { + files: HashMap, redirects: HashMap, } -impl InMemoryLoader { - pub fn add_local_file( +impl TestLoader { + pub fn add( &mut self, - path: impl AsRef, + path_or_specifier: impl AsRef, text: impl AsRef, ) -> &mut Self { - let path = make_path(path.as_ref()); - self.local_files.insert(path, text.as_ref().to_string()); - self - } - - pub fn add_remote_file( - &mut self, - specifier: impl AsRef, - text: impl AsRef, - ) -> &mut Self { - self.remote_files.insert( - ModuleSpecifier::parse(specifier.as_ref()).unwrap(), - Ok((text.as_ref().to_string(), None)), - ); + if path_or_specifier + .as_ref() + .to_lowercase() + .starts_with("http") + { + self.files.insert( + ModuleSpecifier::parse(path_or_specifier.as_ref()).unwrap(), + Ok((text.as_ref().to_string(), None)), + ); + } else { + let path = make_path(path_or_specifier.as_ref()); + let specifier = ModuleSpecifier::from_file_path(path).unwrap(); + self + .files + .insert(specifier, Ok((text.as_ref().to_string(), None))); + } self } - pub fn add_remote_file_with_headers( + pub fn add_remote_with_headers( &mut self, specifier: impl AsRef, text: impl AsRef, @@ -67,7 +67,7 @@ impl InMemoryLoader { .iter() .map(|(key, value)| (key.to_string(), value.to_string())) .collect(); - self.remote_files.insert( + self.files.insert( ModuleSpecifier::parse(specifier.as_ref()).unwrap(), Ok((text.as_ref().to_string(), Some(headers))), ); @@ -87,26 +87,14 @@ impl InMemoryLoader { } } -impl Loader for InMemoryLoader { +impl Loader for TestLoader { fn load( &mut self, specifier: &ModuleSpecifier, _is_dynamic: bool, ) -> LoadFuture { - if specifier.scheme() == "file" { - let file_path = specifier.to_file_path().unwrap(); - let result = self.local_files.get(&file_path).map(ToOwned::to_owned); - let specifier = specifier.clone(); - return Box::pin(async move { - Ok(result.map(|result| LoadResponse { - content: Arc::new(result), - maybe_headers: None, - specifier, - })) - }); - } let specifier = self.redirects.get(specifier).unwrap_or(specifier); - let result = self.remote_files.get(specifier).map(|result| match result { + let result = self.files.get(specifier).map(|result| match result { Ok(result) => Ok(LoadResponse { specifier: specifier.clone(), content: Arc::new(result.0.clone()), @@ -116,7 +104,10 @@ impl Loader for InMemoryLoader { }); let result = match result { Some(Ok(result)) => Ok(Some(result)), - Some(Err(err)) => Err(anyhow!("{}", err.to_string())), + Some(Err(err)) => Err(anyhow!("{}", err)), + None if specifier.scheme() == "data" => { + deno_graph::source::load_data_url(specifier) + } None => Ok(None), }; Box::pin(futures::future::ready(result)) @@ -170,7 +161,7 @@ pub struct VendorOutput { #[derive(Default)] pub struct VendorTestBuilder { entry_points: Vec, - loader: InMemoryLoader, + loader: TestLoader, } impl VendorTestBuilder { @@ -208,10 +199,7 @@ impl VendorTestBuilder { }) } - pub fn with_loader( - &mut self, - action: impl Fn(&mut InMemoryLoader), - ) -> &mut Self { + pub fn with_loader(&mut self, action: impl Fn(&mut TestLoader)) -> &mut Self { action(&mut self.loader); self } From a981b532cd5b932e76d356302e3a587f9ee9ca54 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 11 Feb 2022 17:25:25 -0500 Subject: [PATCH 14/33] - derived import map and functionality test - fix directory paths in import map (bug in url crate) - starting to add integration tests --- cli/flags.rs | 2 + cli/tests/integration/mod.rs | 2 + cli/tests/integration/vendor_tests.rs | 78 +++++++++++++++++++++++++++ cli/tools/vendor/build.rs | 60 ++++++++++++++++----- cli/tools/vendor/import_map.rs | 2 + cli/tools/vendor/mappings.rs | 9 +++- cli/tools/vendor/mod.rs | 24 ++++++--- cli/tools/vendor/test.rs | 21 +++----- 8 files changed, 163 insertions(+), 35 deletions(-) create mode 100644 cli/tests/integration/vendor_tests.rs diff --git a/cli/flags.rs b/cli/flags.rs index 0c26018367a5b..1e1d5f833837d 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1439,6 +1439,7 @@ fn vendor_subcommand<'a>() -> App<'a> { ) .takes_value(false), ) + .arg(import_map_arg()) } fn compile_args(app: App) -> App { @@ -2261,6 +2262,7 @@ fn upgrade_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } fn vendor_parse(flags: &mut Flags, matches: &clap::ArgMatches) { + import_map_arg_parse(flags, matches); flags.subcommand = DenoSubcommand::Vendor(VendorFlags { entry_points: matches .values_of("entry_points") diff --git a/cli/tests/integration/mod.rs b/cli/tests/integration/mod.rs index 3ab79f7df20d5..b77a1179e923d 100644 --- a/cli/tests/integration/mod.rs +++ b/cli/tests/integration/mod.rs @@ -84,6 +84,8 @@ mod run; mod test; #[path = "upgrade_tests.rs"] mod upgrade; +#[path = "vendor_tests.rs"] +mod vendor; #[path = "watcher_tests.rs"] mod watcher; #[path = "worker_tests.rs"] diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs new file mode 100644 index 0000000000000..33ff7e2ada58d --- /dev/null +++ b/cli/tests/integration/vendor_tests.rs @@ -0,0 +1,78 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + +use pretty_assertions::assert_eq; +use std::fs; +use std::process::Stdio; +use tempfile::TempDir; +use test_util as util; + +#[test] +fn output_dir_exists() { + let t = TempDir::new().unwrap(); + let vendor_dir = t.path().join("vendor"); + fs::write(t.path().join("mod.ts"), "").unwrap(); + fs::create_dir_all(&vendor_dir).unwrap(); + fs::write(vendor_dir.join("mod.ts"), "").unwrap(); + + let deno = util::deno_cmd() + .current_dir(t.path()) + .env("NO_COLOR", "1") + .arg("vendor") + .arg("mod.ts") + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + assert_eq!( + String::from_utf8_lossy(&output.stderr).trim(), + format!("error: Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", vendor_dir.display()), + ); + assert!(!output.status.success()); + + // now use `--force` + let status = util::deno_cmd() + .current_dir(t.path()) + .env("NO_COLOR", "1") + .arg("vendor") + .arg("mod.ts") + .arg("--force") + .spawn() + .unwrap() + .wait() + .unwrap(); + assert!(status.success()); +} + +#[test] +fn import_map_output_dir() { + let t = TempDir::new().unwrap(); + let vendor_dir = t.path().join("vendor"); + fs::write(t.path().join("mod.ts"), "").unwrap(); + fs::create_dir_all(&vendor_dir).unwrap(); + let import_map_path = vendor_dir.join("import_map.json"); + fs::write( + &import_map_path, + "{ \"imports\": { \"https://localhost/\": \"./localhost/\" }}", + ) + .unwrap(); + + let deno = util::deno_cmd() + .current_dir(t.path()) + .env("NO_COLOR", "1") + .arg("vendor") + .arg("--force") + .arg("--import-map") + .arg(import_map_path) + .arg("mod.ts") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + println!("{}", String::from_utf8_lossy(&output.stdout).trim()); + assert_eq!( + String::from_utf8_lossy(&output.stderr).trim(), + "error: Using an import map found in the output directory is not supported.", + ); + assert!(!output.status.success()); +} diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index ef793f349847e..56a140719d142 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -1,3 +1,5 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + use std::path::Path; use deno_core::error::AnyError; @@ -39,8 +41,6 @@ pub fn build( let mappings = Mappings::from_remote_modules(graph, &remote_modules, output_dir)?; - environment.create_dir_all(output_dir)?; - // collect and write out all the text changes for module in &remote_modules { let source = match &module.maybe_source { @@ -120,7 +120,7 @@ mod test { output.import_map, Some(json!({ "imports": { - "https://localhost/": "./localhost", + "https://localhost/": "./localhost/", "https://localhost/other.ts?test": "./localhost/other.ts", "https://localhost/redirect.ts": "./localhost/mod.ts", } @@ -185,15 +185,15 @@ mod test { output.import_map, Some(json!({ "imports": { - "https://localhost/": "./localhost", - "https://other/": "./other" + "https://localhost/": "./localhost/", + "https://other/": "./other/" }, "scopes": { - "./localhost": { + "./localhost/": { "/absolute.ts": "./localhost/absolute.ts", "./localhost/redirect.ts": "./localhost/other.ts", }, - "./other": { + "./other/": { "./other/sub2/other?asdf": "./other/sub2/other.js" } } @@ -256,7 +256,7 @@ mod test { output.import_map, Some(json!({ "imports": { - "https://localhost/": "./localhost", + "https://localhost/": "./localhost/", "https://localhost/mod.TS": "./localhost/mod_2.TS", "https://localhost/mod.ts": "./localhost/mod_3.ts", "https://localhost/mod.ts?test": "./localhost/mod_4.ts", @@ -275,6 +275,42 @@ mod test { ); } + #[tokio::test] + async fn multiple_entrypoints() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .add_entry_point("/test.deps.ts") + .with_loader(|loader| { + loader + .add("/mod.ts", r#"import "https://localhost/mod.ts";"#) + .add( + "/test.deps.ts", + r#"export * from "https://localhost/test.ts";"#, + ) + .add("https://localhost/mod.ts", "export class Mod {}") + .add("https://localhost/test.ts", "export class Test {}"); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "https://localhost/": "./localhost/", + } + })) + ); + assert_eq!( + output.files, + to_file_vec(&[ + ("/vendor/localhost/mod.ts", "export class Mod {}"), + ("/vendor/localhost/test.ts", "export class Test {}"), + ]), + ); + } + #[tokio::test] async fn json_module() { let mut builder = VendorTestBuilder::with_default_setup(); @@ -295,7 +331,7 @@ mod test { output.import_map, Some(json!({ "imports": { - "https://localhost/": "./localhost" + "https://localhost/": "./localhost/" } })) ); @@ -309,9 +345,7 @@ mod test { async fn data_urls() { let mut builder = VendorTestBuilder::with_default_setup(); - let mod_file_text = format!( - r#"import * as b from "data:application/typescript,export%20*%20from%20%22https://localhost/mod.ts%22;";"# - ); + let mod_file_text = r#"import * as b from "data:application/typescript,export%20*%20from%20%22https://localhost/mod.ts%22;";"#; let output = builder .with_loader(|loader| { @@ -327,7 +361,7 @@ mod test { output.import_map, Some(json!({ "imports": { - "https://localhost/": "./localhost" + "https://localhost/": "./localhost/" } })) ); diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index e2877ce44e286..7004a2b0bdc9c 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -1,3 +1,5 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + use std::collections::BTreeMap; use deno_ast::LineAndColumnIndex; diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index 822bf92f8e89e..ff956168fc0ba 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -120,7 +120,14 @@ impl Mappings { .unwrap(); } - from.make_relative(&to).unwrap() + // workaround for url crate not adding a trailing slash for a directory + // it seems to be fixed once a version greater than 2.2.2 is released + let is_dir = to.path().ends_with('/'); + let mut text = from.make_relative(&to).unwrap(); + if is_dir && !text.ends_with('/') && to.query().is_none() { + text.push('/'); + } + text } pub fn relative_specifier_text( diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index f9ec115f4b03e..5a026170f9cb0 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -22,13 +22,8 @@ mod specifiers; mod test; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { - // todo: error when someone uses an import map in the vendor folder - // todo: need to handle rewriting out the current import map to the new location? Doesn't seem possible. - // I think people will need to manually update - // todo: add integration tests // todo: add x-TypeScript-types support via proxy file - // todo: test for data url - let output_dir = resolve_and_validate_output_dir(&flags)?; + let output_dir = resolve_and_validate_output_dir(&flags, &ps)?; let graph = create_graph(&ps, &flags).await?; build::build(&graph, &output_dir, &build::RealVendorEnvironment) @@ -36,6 +31,7 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { fn resolve_and_validate_output_dir( flags: &VendorFlags, + ps: &ProcState, ) -> Result { let output_dir = match &flags.output_path { Some(output_path) => output_path.clone(), @@ -44,6 +40,22 @@ fn resolve_and_validate_output_dir( if !flags.force && !is_dir_empty(&output_dir)? { bail!("Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", output_dir.display()); } + + // check the import map + if let Some(import_map_path) = ps + .maybe_import_map + .as_ref() + .and_then(|m| m.base_url().to_file_path().ok()) + { + if import_map_path.starts_with(&output_dir) { + // We don't allow using the output directory to help generate the new state + // of itself because supporting this scenario adds a lot of complexity. + bail!( + "Using an import map found in the output directory is not supported." + ); + } + } + Ok(output_dir) } diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index dcfba6c158796..bdb2827fa9dd4 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -122,28 +122,19 @@ struct TestVendorEnvironment { impl VendorEnvironment for TestVendorEnvironment { fn create_dir_all(&self, dir_path: &Path) -> Result<(), AnyError> { - let dir_path = dir_path.to_path_buf(); let mut directories = self.directories.borrow_mut(); - if !directories.insert(dir_path.clone()) { - for path in dir_path.ancestors() { - if !directories.insert(path.to_path_buf()) { - break; - } + for path in dir_path.ancestors() { + if !directories.insert(path.to_path_buf()) { + break; } } Ok(()) } fn write_file(&self, file_path: &Path, text: &str) -> Result<(), AnyError> { - if !self - .directories - .borrow() - .contains(file_path.parent().unwrap()) - { - bail!( - "Directory not found: {}", - file_path.parent().unwrap().display() - ); + let parent = file_path.parent().unwrap(); + if !self.directories.borrow().contains(parent) { + bail!("Directory not found: {}", parent.display()); } self .files From 2974cd11c737afe234c1ce10e3c19bf84ba3c8cd Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 13:53:00 -0500 Subject: [PATCH 15/33] Support x-typescript-types headers --- cli/fs_util.rs | 66 +++++++++++++++ cli/main.rs | 2 +- cli/tools/vendor/analyze.rs | 31 +++++++ cli/tools/vendor/build.rs | 142 ++++++++++++++++++++++++++++++++- cli/tools/vendor/mappings.rs | 51 ++++++++++++ cli/tools/vendor/mod.rs | 1 + cli/tools/vendor/specifiers.rs | 71 +---------------- cli/tools/vendor/test.rs | 2 +- 8 files changed, 293 insertions(+), 73 deletions(-) create mode 100644 cli/tools/vendor/analyze.rs diff --git a/cli/fs_util.rs b/cli/fs_util.rs index fbdcdc81ae9bb..4f06a0c13058e 100644 --- a/cli/fs_util.rs +++ b/cli/fs_util.rs @@ -362,6 +362,32 @@ pub fn path_has_trailing_slash(path: &Path) -> bool { } } +/// Gets a path with the specified file stem suffix. +pub fn path_with_stem_suffix(path: &Path, suffix: &str) -> PathBuf { + if let Some(file_name) = path.file_name().map(|f| f.to_string_lossy()) { + if let Some(file_stem) = path.file_stem().map(|f| f.to_string_lossy()) { + if let Some(ext) = path.extension().map(|f| f.to_string_lossy()) { + return if file_stem.to_lowercase().ends_with(".d") { + path.with_file_name(format!( + "{}_{}.{}.{}", + &file_stem[..file_stem.len() - ".d".len()], + suffix, + // maintain casing + &file_stem[file_stem.len() - "d".len()..], + ext + )) + } else { + path.with_file_name(format!("{}{}.{}", file_stem, suffix, ext)) + }; + } + } + + path.with_file_name(format!("{}{}", file_name, suffix)) + } else { + path.with_file_name(suffix) + } +} + #[cfg(test)] mod tests { use super::*; @@ -730,4 +756,44 @@ mod tests { assert_eq!(result, expected); } } + + #[test] + fn test_path_with_stem_suffix() { + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/"), "_2"), + PathBuf::from("/_2") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test"), "_2"), + PathBuf::from("/test_2") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.txt"), "_2"), + PathBuf::from("/test_2.txt") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test/subdir"), "_2"), + PathBuf::from("/test/subdir_2") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test/subdir.other.txt"), "_2"), + PathBuf::from("/test/subdir.other_2.txt") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.d.ts"), "_2"), + PathBuf::from("/test_2.d.ts") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.D.TS"), "_2"), + PathBuf::from("/test_2.D.TS") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.d.mts"), "_2"), + PathBuf::from("/test_2.d.mts") + ); + assert_eq!( + path_with_stem_suffix(&PathBuf::from("/test.d.cts"), "_2"), + PathBuf::from("/test_2.d.cts") + ); + } } diff --git a/cli/main.rs b/cli/main.rs index f0ecaa89e2d78..c1fe8317efb1e 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -1303,7 +1303,7 @@ async fn vendor_command( flags: Flags, vendor_flags: VendorFlags, ) -> Result { - let ps = ProcState::build(flags).await?; + let ps = ProcState::build(Arc::new(flags)).await?; tools::vendor::vendor(ps, vendor_flags).await?; Ok(0) } diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs new file mode 100644 index 0000000000000..c53f31875e774 --- /dev/null +++ b/cli/tools/vendor/analyze.rs @@ -0,0 +1,31 @@ +use deno_ast::swc::ast::Program; +use deno_ast::swc::visit::noop_visit_type; +use deno_ast::swc::visit::Visit; +use deno_ast::swc::visit::VisitWith; +use deno_ast::ParsedSource; + +/// Gets if the parsed source has a default export. +pub fn has_default_export(source: &ParsedSource) -> bool { + let mut visitor = DefaultExportFinder { + has_default_export: false, + }; + let program = source.program(); + let program: &Program = &program; + program.visit_with(&mut visitor); + visitor.has_default_export +} + +struct DefaultExportFinder { + has_default_export: bool, +} + +impl<'a> Visit for DefaultExportFinder { + noop_visit_type!(); + + fn visit_export_default_decl( + &mut self, + _: &deno_ast::swc::ast::ExportDefaultDecl, + ) { + self.has_default_export = true; + } +} diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index 56a140719d142..50a215513b146 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -3,11 +3,14 @@ use std::path::Path; use deno_core::error::AnyError; +use deno_graph::Module; use deno_graph::ModuleGraph; use deno_graph::ModuleKind; +use super::analyze::has_default_export; use super::import_map::build_import_map; use super::mappings::Mappings; +use super::mappings::ProxiedModule; use super::specifiers::is_remote_specifier; pub trait VendorEnvironment { @@ -41,13 +44,15 @@ pub fn build( let mappings = Mappings::from_remote_modules(graph, &remote_modules, output_dir)?; - // collect and write out all the text changes + // write out all the files for module in &remote_modules { let source = match &module.maybe_source { Some(source) => source, None => continue, }; - let local_path = mappings.local_path(&module.specifier); + let local_path = mappings + .proxied_path(&module.specifier) + .unwrap_or_else(|| mappings.local_path(&module.specifier)); if !matches!(module.kind, ModuleKind::Esm | ModuleKind::Asserted) { log::warn!( "Unsupported module kind {:?} for {}", @@ -60,6 +65,15 @@ pub fn build( environment.write_file(&local_path, source)?; } + // write out the proxies + for (specifier, proxied_module) in mappings.proxied_modules() { + let proxy_path = mappings.local_path(specifier); + let module = graph.get(specifier).unwrap(); + let text = build_proxy_module_source(module, proxied_module); + + environment.write_file(&proxy_path, &text)?; + } + // create the import map if !mappings.base_specifiers().is_empty() { let import_map_text = build_import_map(graph, &all_modules, &mappings); @@ -70,6 +84,40 @@ pub fn build( Ok(()) } +fn build_proxy_module_source( + module: &Module, + proxied_module: &ProxiedModule, +) -> String { + let mut text = format!( + "// @deno-types=\"{}\"\n", + proxied_module.declaration_specifier + ); + let relative_specifier = format!( + "./{}", + proxied_module + .output_path + .file_name() + .unwrap() + .to_string_lossy() + ); + + // for simplicity, always include the `export *` statement as it won't error + // even when the module does not contain a named export + text.push_str(&format!("export * from \"{}\";\n", relative_specifier)); + + // add a default export if one exists in the module + if let Some(parsed_source) = module.maybe_parsed_source.as_ref() { + if has_default_export(parsed_source) { + text.push_str(&format!( + "export {{ default }} from \"{}\";\n", + relative_specifier + )); + } + } + + text +} + #[cfg(test)] mod test { use crate::tools::vendor::test::VendorTestBuilder; @@ -171,7 +219,7 @@ mod test { ), ) .add("https://other/sub2/mod.ts", "export class Mod {}") - .add_remote_with_headers( + .add_with_headers( "https://other/sub2/other?asdf", "export class Other {}", &[("content-type", "application/javascript")], @@ -371,6 +419,94 @@ mod test { ); } + #[tokio::test] + async fn x_typescript_types_no_default() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .with_loader(|loader| { + loader + .add("/mod.ts", r#"import "https://localhost/mod.js";"#) + .add_with_headers( + "https://localhost/mod.js", + "export class Mod {}", + &[("x-typescript-types", "https://localhost/mod.d.ts")], + ) + .add("https://localhost/mod.d.ts", "export class Mod {}"); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "https://localhost/": "./localhost/" + } + })) + ); + assert_eq!( + output.files, + to_file_vec(&[ + ("/vendor/localhost/mod.d.ts", "export class Mod {}"), + ( + "/vendor/localhost/mod.js", + concat!( + "// @deno-types=\"https://localhost/mod.d.ts\"\n", + "export * from \"./mod.proxied.js\";\n" + ) + ), + ("/vendor/localhost/mod.proxied.js", "export class Mod {}"), + ]), + ); + } + + #[tokio::test] + async fn x_typescript_types_default_export() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .with_loader(|loader| { + loader + .add("/mod.ts", r#"import "https://localhost/mod.js";"#) + .add_with_headers( + "https://localhost/mod.js", + "export default class Mod {}", + &[("x-typescript-types", "https://localhost/mod.d.ts")], + ) + .add("https://localhost/mod.d.ts", "export default class Mod {}"); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "https://localhost/": "./localhost/" + } + })) + ); + assert_eq!( + output.files, + to_file_vec(&[ + ("/vendor/localhost/mod.d.ts", "export default class Mod {}"), + ( + "/vendor/localhost/mod.js", + concat!( + "// @deno-types=\"https://localhost/mod.d.ts\"\n", + "export * from \"./mod.proxied.js\";\n", + "export { default } from \"./mod.proxied.js\";\n", + ) + ), + ( + "/vendor/localhost/mod.proxied.js", + "export default class Mod {}" + ), + ]), + ); + } + fn to_file_vec(items: &[(&str, &str)]) -> Vec<(String, String)> { items .iter() diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index ff956168fc0ba..ddedad7301217 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -10,6 +10,10 @@ use deno_ast::ModuleSpecifier; use deno_core::error::AnyError; use deno_graph::Module; use deno_graph::ModuleGraph; +use deno_graph::Position; +use deno_graph::Resolved; + +use crate::fs_util::path_with_stem_suffix; use super::specifiers::dir_name_for_root; use super::specifiers::get_unique_path; @@ -17,11 +21,17 @@ use super::specifiers::make_url_relative; use super::specifiers::partition_by_root_specifiers; use super::specifiers::sanitize_filepath; +pub struct ProxiedModule { + pub output_path: PathBuf, + pub declaration_specifier: ModuleSpecifier, +} + /// Constructs and holds the remote specifier to local path mappings. pub struct Mappings { output_dir: ModuleSpecifier, mappings: HashMap, base_specifiers: Vec, + proxies: HashMap, } impl Mappings { @@ -34,6 +44,7 @@ impl Mappings { partition_by_root_specifiers(remote_modules.iter().map(|m| &m.specifier)); let mut mapped_paths = HashSet::new(); let mut mappings = HashMap::new(); + let mut proxies = HashMap::new(); let mut base_specifiers = Vec::new(); for (root, specifiers) in partitioned_specifiers.into_iter() { @@ -63,10 +74,40 @@ impl Mappings { mappings.insert(root, base_dir); } + // resolve all the "proxy" paths to use for when an x-typescript-types header is specified + for module in remote_modules { + if let Some(( + _, + Resolved::Ok { + specifier, range, .. + }, + )) = &module.maybe_types_dependency + { + // hack to tell if it's a x-typescript-types header + let is_ts_types_header = + range.start == Position::zeroed() && range.end == Position::zeroed(); + if is_ts_types_header { + let module_path = mappings.get(&module.specifier).unwrap(); + let proxied_path = get_unique_path( + path_with_stem_suffix(module_path, ".proxied"), + &mut mapped_paths, + ); + proxies.insert( + module.specifier.clone(), + ProxiedModule { + output_path: proxied_path, + declaration_specifier: specifier.clone(), + }, + ); + } + } + } + Ok(Self { output_dir: ModuleSpecifier::from_directory_path(output_dir).unwrap(), mappings, base_specifiers, + proxies, }) } @@ -160,6 +201,16 @@ impl Mappings { panic!("Could not find base specifier for {}", child_specifier) }) } + + pub fn proxied_path(&self, specifier: &ModuleSpecifier) -> Option { + self.proxies.get(specifier).map(|s| s.output_path.clone()) + } + + pub fn proxied_modules( + &self, + ) -> std::collections::hash_map::Iter<'_, ModuleSpecifier, ProxiedModule> { + self.proxies.iter() + } } fn path_with_extension(path: &Path, new_ext: &str) -> PathBuf { diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 5a026170f9cb0..df8d22ff7e95b 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -14,6 +14,7 @@ use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; +mod analyze; mod build; mod import_map; mod mappings; diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index 031f021799fcc..a83d1fc324a8d 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -1,13 +1,14 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. use std::collections::HashSet; -use std::path::Path; use std::path::PathBuf; use deno_ast::ModuleSpecifier; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; +use crate::fs_util::path_with_stem_suffix; + /// Partitions the provided specifiers by specifiers that do not have a /// parent specifier. pub fn partition_by_root_specifiers<'a>( @@ -67,32 +68,6 @@ pub fn dir_name_for_root(root: &ModuleSpecifier) -> PathBuf { result } -/// Gets a path with the specified file stem suffix. -pub fn path_with_stem_suffix(path: &Path, suffix: &str) -> PathBuf { - if let Some(file_name) = path.file_name().map(|f| f.to_string_lossy()) { - if let Some(file_stem) = path.file_stem().map(|f| f.to_string_lossy()) { - if let Some(ext) = path.extension().map(|f| f.to_string_lossy()) { - return if file_stem.to_lowercase().ends_with(".d") { - path.with_file_name(format!( - "{}_{}.{}.{}", - &file_stem[..file_stem.len() - ".d".len()], - suffix, - // maintain casing - &file_stem[file_stem.len() - "d".len()..], - ext - )) - } else { - path.with_file_name(format!("{}_{}.{}", file_stem, suffix, ext)) - }; - } - } - - path.with_file_name(format!("{}_{}", file_name, suffix)) - } else { - path.with_file_name(suffix) - } -} - /// Gets a unique file path given the provided file path /// and the set of existing file paths. Inserts to the /// set when finding a unique path. @@ -104,7 +79,7 @@ pub fn get_unique_path( let mut count = 2; // case insensitive comparison so the output works on case insensitive file systems while !unique_set.insert(path.to_string_lossy().to_lowercase()) { - path = path_with_stem_suffix(&original_path, &count.to_string()); + path = path_with_stem_suffix(&original_path, &format!("_{}", count)); count += 1; } path @@ -253,46 +228,6 @@ mod test { } } - #[test] - fn test_path_with_stem_suffix() { - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/"), "2"), - PathBuf::from("/2") - ); - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/test"), "2"), - PathBuf::from("/test_2") - ); - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/test.txt"), "2"), - PathBuf::from("/test_2.txt") - ); - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/test/subdir"), "2"), - PathBuf::from("/test/subdir_2") - ); - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/test/subdir.other.txt"), "2"), - PathBuf::from("/test/subdir.other_2.txt") - ); - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/test.d.ts"), "2"), - PathBuf::from("/test_2.d.ts") - ); - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/test.D.TS"), "2"), - PathBuf::from("/test_2.D.TS") - ); - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/test.d.mts"), "2"), - PathBuf::from("/test_2.d.mts") - ); - assert_eq!( - path_with_stem_suffix(&PathBuf::from("/test.d.cts"), "2"), - PathBuf::from("/test_2.d.cts") - ); - } - #[test] fn test_unique_path() { let mut paths = HashSet::new(); diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index bdb2827fa9dd4..8773127002f55 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -57,7 +57,7 @@ impl TestLoader { self } - pub fn add_remote_with_headers( + pub fn add_with_headers( &mut self, specifier: impl AsRef, text: impl AsRef, From fe1413f7eaa85ffbe329cc817e4265eb1d3ea454 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 14:14:58 -0500 Subject: [PATCH 16/33] Fix and add tests for default export detection. --- cli/tools/vendor/analyze.rs | 88 +++++++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs index c53f31875e774..e6d953a5afab7 100644 --- a/cli/tools/vendor/analyze.rs +++ b/cli/tools/vendor/analyze.rs @@ -1,3 +1,7 @@ +use deno_ast::swc::ast::ExportDefaultDecl; +use deno_ast::swc::ast::ExportSpecifier; +use deno_ast::swc::ast::ModuleExportName; +use deno_ast::swc::ast::NamedExport; use deno_ast::swc::ast::Program; use deno_ast::swc::visit::noop_visit_type; use deno_ast::swc::visit::Visit; @@ -22,10 +26,86 @@ struct DefaultExportFinder { impl<'a> Visit for DefaultExportFinder { noop_visit_type!(); - fn visit_export_default_decl( - &mut self, - _: &deno_ast::swc::ast::ExportDefaultDecl, - ) { + fn visit_export_default_decl(&mut self, _: &ExportDefaultDecl) { self.has_default_export = true; } + + fn visit_named_export(&mut self, named_export: &NamedExport) { + if named_export + .specifiers + .iter() + .any(export_specifier_has_default) + { + self.has_default_export = true; + } + } +} + +fn export_specifier_has_default(s: &ExportSpecifier) -> bool { + match s { + ExportSpecifier::Default(_) => true, + ExportSpecifier::Namespace(_) => false, + ExportSpecifier::Named(named) => { + let export_name = named.exported.as_ref().unwrap_or(&named.orig); + + match export_name { + ModuleExportName::Str(_) => false, + ModuleExportName::Ident(ident) => &*ident.sym == "default", + } + } + } +} + +#[cfg(test)] +mod test { + use deno_ast::MediaType; + use deno_ast::ParseParams; + use deno_ast::ParsedSource; + use deno_ast::SourceTextInfo; + + use super::has_default_export; + + #[test] + fn has_default_when_export_default_decl() { + let parsed_source = parse_module("export default class Class {}"); + assert!(has_default_export(&parsed_source)); + } + + #[test] + fn has_default_when_named_export() { + let parsed_source = parse_module("export {default} from './test.ts';"); + assert!(has_default_export(&parsed_source)); + } + + #[test] + fn has_default_when_named_export_alias() { + let parsed_source = + parse_module("export {test as default} from './test.ts';"); + assert!(has_default_export(&parsed_source)); + } + + #[test] + fn not_has_default_when_named_export_not_exported() { + let parsed_source = + parse_module("export {default as test} from './test.ts';"); + assert!(!has_default_export(&parsed_source)); + } + + #[test] + fn not_has_default_when_not() { + let parsed_source = parse_module("export {test} from './test.ts'; export class Test{} export * from './test';"); + assert!(!has_default_export(&parsed_source)); + } + + fn parse_module(text: &str) -> ParsedSource { + deno_ast::parse_module(ParseParams { + specifier: "file:///mod.ts".to_string(), + capture_tokens: false, + maybe_syntax: None, + media_type: MediaType::TypeScript, + scope_analysis: false, + source: SourceTextInfo::from_string(text.to_string()), + }) + .unwrap() + } } From 36bc9168e60dfbdbe18cd2a96e9341ae7214a264 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 15:17:41 -0500 Subject: [PATCH 17/33] Add flags test. --- cli/flags.rs | 47 +++++++++++++++++++++++++++++++++++++++-- cli/tools/vendor/mod.rs | 1 - 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index 6e1eb97ee328e..a6b3131dc6d4a 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -2549,8 +2549,8 @@ mod tests { /// Creates vector of strings, Vec macro_rules! svec { - ($($x:expr),*) => (vec![$($x.to_string()),*]); -} + ($($x:expr),* $(,)?) => (vec![$($x.to_string()),*]); + } #[test] fn global_flags() { @@ -4907,4 +4907,47 @@ mod tests { .contains("error: The following required arguments were not provided:")); assert!(&error_message.contains("--watch=...")); } + + #[test] + fn vendor_minimal() { + let r = flags_from_vec(svec!["deno", "vendor", "mod.ts",]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Vendor(VendorFlags { + entry_points: svec!["mod.ts"], + force: false, + output_path: None, + }), + ..Flags::default() + } + ); + } + + #[test] + fn vendor_all() { + let r = flags_from_vec(svec![ + "deno", + "vendor", + "--force", + "--output", + "out_dir", + "--import-map", + "import_map.json", + "mod.ts", + "deps.test.ts", + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Vendor(VendorFlags { + entry_points: svec!["mod.ts", "deps.test.ts"], + force: true, + output_path: Some(PathBuf::from("out_dir")), + }), + import_map_path: Some("import_map.json".to_string()), + ..Flags::default() + } + ); + } } diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index df8d22ff7e95b..829fe8f8bb5dd 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -23,7 +23,6 @@ mod specifiers; mod test; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { - // todo: add x-TypeScript-types support via proxy file let output_dir = resolve_and_validate_output_dir(&flags, &ps)?; let graph = create_graph(&ps, &flags).await?; From 9762a40487b8361bad9e8f76a10cdad605983f73 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 16:44:28 -0500 Subject: [PATCH 18/33] More integration tests and fixes. --- cli/tests/integration/vendor_tests.rs | 81 +++++++++++++++++++++- cli/tests/testdata/vendor/logger/logger.ts | 5 ++ cli/tests/testdata/vendor/logger/mod.ts | 1 + cli/tools/vendor/build.rs | 53 ++++++++++++++ cli/tools/vendor/import_map.rs | 1 + cli/tools/vendor/mod.rs | 3 +- cli/tools/vendor/specifiers.rs | 54 ++++++--------- 7 files changed, 163 insertions(+), 35 deletions(-) create mode 100644 cli/tests/testdata/vendor/logger/logger.ts create mode 100644 cli/tests/testdata/vendor/logger/mod.ts diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index 33ff7e2ada58d..045e4862071c7 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -1,10 +1,13 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +use deno_core::serde_json; +use deno_core::serde_json::json; use pretty_assertions::assert_eq; use std::fs; use std::process::Stdio; use tempfile::TempDir; use test_util as util; +use util::http_server; #[test] fn output_dir_exists() { @@ -29,6 +32,24 @@ fn output_dir_exists() { ); assert!(!output.status.success()); + // ensure it errors when using the `--output` arg too + let deno = util::deno_cmd() + .current_dir(t.path()) + .env("NO_COLOR", "1") + .arg("vendor") + .arg("--output") + .arg("vendor") + .arg("mod.ts") + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + assert_eq!( + String::from_utf8_lossy(&output.stderr).trim(), + format!("error: Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", vendor_dir.display()), + ); + assert!(!output.status.success()); + // now use `--force` let status = util::deno_cmd() .current_dir(t.path()) @@ -69,10 +90,68 @@ fn import_map_output_dir() { .spawn() .unwrap(); let output = deno.wait_with_output().unwrap(); - println!("{}", String::from_utf8_lossy(&output.stdout).trim()); assert_eq!( String::from_utf8_lossy(&output.stderr).trim(), "error: Using an import map found in the output directory is not supported.", ); assert!(!output.status.success()); } + +#[test] +fn standard_test() { + let _server = http_server(); + let t = TempDir::new().unwrap(); + let vendor_dir = t.path().join("vendor2"); + fs::write(t.path().join("mod.ts"), "import {Logger} from 'http://localhost:4545/vendor/logger/mod.ts?testing'; new Logger().log('outputted');").unwrap(); + + let status = util::deno_cmd() + .current_dir(t.path()) + .arg("vendor") + .arg("mod.ts") + .arg("--output") + .arg("vendor2") + .spawn() + .unwrap() + .wait() + .unwrap(); + assert!(status.success()); + assert!(vendor_dir.exists()); + assert!(!t.path().join("vendor").exists()); + let import_map: serde_json::Value = serde_json::from_str( + &fs::read_to_string(vendor_dir.join("import_map.json")).unwrap(), + ) + .unwrap(); + assert_eq!( + import_map, + json!({ + "imports": { + "http://localhost:4545/": "./localhost_4545/", + "http://localhost:4545/vendor/logger/mod.ts?testing": "./localhost_4545/vendor/logger/mod.ts", + }, + "scopes": { + "./localhost_4545/": { + "./localhost_4545/vendor/logger/logger.ts?test": "./localhost_4545/vendor/logger/logger.ts" + } + } + }), + ); + + // try running the output with `--no-remote` + let deno = util::deno_cmd() + .current_dir(t.path()) + .env("NO_COLOR", "1") + .arg("run") + .arg("--no-remote") + .arg("--no-check") + .arg("--import-map") + .arg("vendor2/import_map.json") + .arg("mod.ts") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + assert_eq!(String::from_utf8_lossy(&output.stderr).trim(), "",); + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "outputted",); + assert!(output.status.success()); +} diff --git a/cli/tests/testdata/vendor/logger/logger.ts b/cli/tests/testdata/vendor/logger/logger.ts new file mode 100644 index 0000000000000..97f603a48ba30 --- /dev/null +++ b/cli/tests/testdata/vendor/logger/logger.ts @@ -0,0 +1,5 @@ +export class Logger { + log(text: string) { + console.log(text); + } +} diff --git a/cli/tests/testdata/vendor/logger/mod.ts b/cli/tests/testdata/vendor/logger/mod.ts new file mode 100644 index 0000000000000..5dfafb53293f7 --- /dev/null +++ b/cli/tests/testdata/vendor/logger/mod.ts @@ -0,0 +1 @@ +export * from "./logger.ts?test"; diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index 50a215513b146..7ac70870605f9 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -35,6 +35,7 @@ pub fn build( output_dir: &Path, environment: &impl VendorEnvironment, ) -> Result<(), AnyError> { + assert!(output_dir.is_absolute()); let all_modules = graph.modules(); let remote_modules = all_modules .iter() @@ -507,6 +508,58 @@ mod test { ); } + #[tokio::test] + async fn subdir() { + let mut builder = VendorTestBuilder::with_default_setup(); + let output = builder + .with_loader(|loader| { + loader + .add( + "/mod.ts", + r#"import "http://localhost:4545/sub/logger/mod.ts?testing";"#, + ) + .add( + "http://localhost:4545/sub/logger/mod.ts?testing", + "export * from './logger.ts?test';", + ) + .add( + "http://localhost:4545/sub/logger/logger.ts?test", + "export class Logger {}", + ); + }) + .build() + .await + .unwrap(); + + assert_eq!( + output.import_map, + Some(json!({ + "imports": { + "http://localhost:4545/": "./localhost_4545/", + "http://localhost:4545/sub/logger/mod.ts?testing": "./localhost_4545/sub/logger/mod.ts", + }, + "scopes": { + "./localhost_4545/": { + "./localhost_4545/sub/logger/logger.ts?test": "./localhost_4545/sub/logger/logger.ts" + } + } + })) + ); + assert_eq!( + output.files, + to_file_vec(&[ + ( + "/vendor/localhost_4545/sub/logger/logger.ts", + "export class Logger {}", + ), + ( + "/vendor/localhost_4545/sub/logger/mod.ts", + "export * from './logger.ts?test';" + ), + ]), + ); + } + fn to_file_vec(items: &[(&str, &str)]) -> Vec<(String, String)> { items .iter() diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index 7004a2b0bdc9c..5e7a7d1982431 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -229,6 +229,7 @@ fn handle_dep_specifier( let key = if text.starts_with("./") || text.starts_with("../") { // resolve relative specifier key let mut local_base_specifier = mappings.local_uri(base_specifier); + local_base_specifier.set_query(unresolved_specifier.query()); local_base_specifier = local_base_specifier .join(&unresolved_specifier.path()[1..]) .unwrap_or_else(|_| { diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 829fe8f8bb5dd..e294f86e33c54 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -9,6 +9,7 @@ use deno_core::resolve_url_or_path; use deno_runtime::permissions::Permissions; use crate::flags::VendorFlags; +use crate::fs_util::resolve_from_cwd; use crate::lockfile; use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; @@ -34,7 +35,7 @@ fn resolve_and_validate_output_dir( ps: &ProcState, ) -> Result { let output_dir = match &flags.output_path { - Some(output_path) => output_path.clone(), + Some(output_path) => resolve_from_cwd(output_path)?, None => std::env::current_dir()?.join("vendor"), }; if !flags.force && !is_dir_empty(&output_dir)? { diff --git a/cli/tools/vendor/specifiers.rs b/cli/tools/vendor/specifiers.rs index a83d1fc324a8d..b869e989c0b7a 100644 --- a/cli/tools/vendor/specifiers.rs +++ b/cli/tools/vendor/specifiers.rs @@ -1,5 +1,6 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +use std::collections::BTreeMap; use std::collections::HashSet; use std::path::PathBuf; @@ -9,39 +10,19 @@ use deno_core::error::AnyError; use crate::fs_util::path_with_stem_suffix; -/// Partitions the provided specifiers by specifiers that do not have a -/// parent specifier. +/// Partitions the provided specifiers by the non-path and non-query parts of a specifier. pub fn partition_by_root_specifiers<'a>( specifiers: impl Iterator, -) -> Vec<(ModuleSpecifier, Vec)> { - let mut root_specifiers: Vec<(ModuleSpecifier, Vec)> = - Vec::new(); +) -> BTreeMap> { + let mut root_specifiers: BTreeMap> = + Default::default(); for remote_specifier in specifiers { - let mut found = false; - for (root_specifier, specifiers) in root_specifiers.iter_mut() { - if let Some(relative_url) = root_specifier.make_relative(remote_specifier) - { - // found a new root - if relative_url.starts_with("../") { - let end_ancestor_index = - relative_url.len() - relative_url.trim_start_matches("../").len(); - *root_specifier = root_specifier - .join(&relative_url[..end_ancestor_index]) - .unwrap(); - } + let mut root_specifier = remote_specifier.clone(); + root_specifier.set_query(None); + root_specifier.set_path("/"); - specifiers.push(remote_specifier.clone()); - found = true; - break; - } - } - if !found { - // get the specifier without the directory - let root_specifier = remote_specifier - .join("./") - .unwrap_or_else(|_| remote_specifier.clone()); - root_specifiers.push((root_specifier, vec![remote_specifier.clone()])); - } + let specifiers = root_specifiers.entry(root_specifier).or_default(); + specifiers.push(remote_specifier.clone()); } root_specifiers } @@ -141,7 +122,7 @@ mod test { "https://deno.land/x/mod/other/A.ts", ], vec![( - "https://deno.land/x/mod/", + "https://deno.land/", vec![ "https://deno.land/x/mod/A.ts", "https://deno.land/x/mod/other/A.ts", @@ -158,7 +139,7 @@ mod test { "https://deno.land/x/other/A.ts", ], vec![( - "https://deno.land/x/", + "https://deno.land/", vec![ "https://deno.land/x/mod/A.ts", "https://deno.land/x/other/A.ts", @@ -172,12 +153,19 @@ mod test { run_partition_by_root_specifiers_test( vec![ "https://deno.land/mod/A.ts", + "http://deno.land/B.ts", + "https://deno.land:8080/C.ts", "https://localhost/mod/A.ts", "https://other/A.ts", ], vec![ - ("https://deno.land/mod/", vec!["https://deno.land/mod/A.ts"]), - ("https://localhost/mod/", vec!["https://localhost/mod/A.ts"]), + ("http://deno.land/", vec!["http://deno.land/B.ts"]), + ("https://deno.land/", vec!["https://deno.land/mod/A.ts"]), + ( + "https://deno.land:8080/", + vec!["https://deno.land:8080/C.ts"], + ), + ("https://localhost/", vec!["https://localhost/mod/A.ts"]), ("https://other/", vec!["https://other/A.ts"]), ], ); From b77ddb53181b2f84952edc27d7e40e5f60c694e1 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 17:45:29 -0500 Subject: [PATCH 19/33] Add sub command description --- cli/flags.rs | 28 ++++++++++++------ cli/tests/integration/vendor_tests.rs | 41 +++++++++++++++++++++++++++ cli/tools/vendor/mod.rs | 2 +- 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index a6b3131dc6d4a..adc3bdd169b13 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -163,7 +163,7 @@ pub struct UpgradeFlags { #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] pub struct VendorFlags { - pub entry_points: Vec, + pub specifiers: Vec, pub output_path: Option, pub force: bool, } @@ -1411,12 +1411,24 @@ update to a different location, use the --output flag } fn vendor_subcommand<'a>() -> App<'a> { - // todo(THIS PR): write tests for these once finalized App::new("vendor") .about("Vendor remote modules into a local directory") - .long_about("") + .long_about( + "Vendor remote modules into a local directory. + +Analyzes the provided modules and their dependencies, downloads remote +modules to the output directory, and produces an import map that maps +remote specifiers to the downloaded files. + + deno vendor main.ts + deno run --import-map vendor/import_map.json main.ts + +Remote modules and multiple modules may also be specified: + + deno vendor main.ts test.deps.ts https://deno.land/std/path/mod.ts", + ) .arg( - Arg::new("entry_points") + Arg::new("specifiers") .takes_value(true) .multiple_values(true) .multiple_occurrences(true) @@ -2265,8 +2277,8 @@ fn upgrade_parse(flags: &mut Flags, matches: &clap::ArgMatches) { fn vendor_parse(flags: &mut Flags, matches: &clap::ArgMatches) { import_map_arg_parse(flags, matches); flags.subcommand = DenoSubcommand::Vendor(VendorFlags { - entry_points: matches - .values_of("entry_points") + specifiers: matches + .values_of("specifiers") .map(|p| p.map(ToString::to_string).collect()) .unwrap_or_default(), output_path: matches.value_of("output").map(PathBuf::from), @@ -4915,7 +4927,7 @@ mod tests { r.unwrap(), Flags { subcommand: DenoSubcommand::Vendor(VendorFlags { - entry_points: svec!["mod.ts"], + specifiers: svec!["mod.ts"], force: false, output_path: None, }), @@ -4941,7 +4953,7 @@ mod tests { r.unwrap(), Flags { subcommand: DenoSubcommand::Vendor(VendorFlags { - entry_points: svec!["mod.ts", "deps.test.ts"], + specifiers: svec!["mod.ts", "deps.test.ts"], force: true, output_path: Some(PathBuf::from("out_dir")), }), diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index 045e4862071c7..cb709b63736cb 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -155,3 +155,44 @@ fn standard_test() { assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "outputted",); assert!(output.status.success()); } + +#[test] +fn remote_module_test() { + let _server = http_server(); + let t = TempDir::new().unwrap(); + let vendor_dir = t.path().join("vendor"); + + let status = util::deno_cmd() + .current_dir(t.path()) + .arg("vendor") + .arg("http://localhost:4545/vendor/logger/mod.ts") + .spawn() + .unwrap() + .wait() + .unwrap(); + assert!(status.success()); + assert!(vendor_dir.exists()); + assert!(vendor_dir + .join("localhost_4545/vendor/logger/mod.ts") + .exists()); + assert!(vendor_dir + .join("localhost_4545/vendor/logger/logger.ts") + .exists()); + let import_map: serde_json::Value = serde_json::from_str( + &fs::read_to_string(vendor_dir.join("import_map.json")).unwrap(), + ) + .unwrap(); + assert_eq!( + import_map, + json!({ + "imports": { + "http://localhost:4545/": "./localhost_4545/", + }, + "scopes": { + "./localhost_4545/": { + "./localhost_4545/vendor/logger/logger.ts?test": "./localhost_4545/vendor/logger/logger.ts" + } + } + }), + ); +} diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index e294f86e33c54..e46b9938405d3 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -75,7 +75,7 @@ async fn create_graph( flags: &VendorFlags, ) -> Result { let entry_points = flags - .entry_points + .specifiers .iter() .map(|p| { let url = resolve_url_or_path(p)?; From 8c16515000f3cf24567e8e25d2429d40076bfc8f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 17:52:06 -0500 Subject: [PATCH 20/33] Add test for an existing import map. --- cli/tests/integration/vendor_tests.rs | 34 +++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index cb709b63736cb..68d86f5a690a4 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -196,3 +196,37 @@ fn remote_module_test() { }), ); } + +#[test] +fn existing_import_map() { + let _server = http_server(); + let t = TempDir::new().unwrap(); + let vendor_dir = t.path().join("vendor"); + fs::write( + t.path().join("mod.ts"), + "import {Logger} from 'http://localhost:4545/vendor/logger/mod.ts';", + ) + .unwrap(); + fs::write( + t.path().join("imports.json"), + r#"{ "imports": { "http://localhost:4545/vendor/logger/": "./logger/" } }"#, + ) + .unwrap(); + fs::create_dir(t.path().join("logger")).unwrap(); + fs::write(t.path().join("logger/mod.ts"), "export class Logger {}").unwrap(); + + let status = util::deno_cmd() + .current_dir(t.path()) + .arg("vendor") + .arg("mod.ts") + .arg("--import-map") + .arg("imports.json") + .spawn() + .unwrap() + .wait() + .unwrap(); + assert!(status.success()); + // it should not have found any remote dependencies because + // the provided import map mapped it to a local directory + assert!(!vendor_dir.join("import_map.json").exists()); +} From 4286ffd544220154c4d232bf9c6358dc3375eefe Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 18:17:08 -0500 Subject: [PATCH 21/33] Tests for dynamic imports --- cli/tests/integration/vendor_tests.rs | 103 +++++++++++++++--- cli/tests/testdata/vendor/dynamic.ts | 3 + .../testdata/vendor/dynamic_non_analyzable.ts | 4 + .../testdata/vendor/{logger => }/logger.ts | 0 .../{logger/mod.ts => query_reexport.ts} | 0 5 files changed, 96 insertions(+), 14 deletions(-) create mode 100644 cli/tests/testdata/vendor/dynamic.ts create mode 100644 cli/tests/testdata/vendor/dynamic_non_analyzable.ts rename cli/tests/testdata/vendor/{logger => }/logger.ts (100%) rename cli/tests/testdata/vendor/{logger/mod.ts => query_reexport.ts} (100%) diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index 68d86f5a690a4..d854c308f75cb 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -102,7 +102,7 @@ fn standard_test() { let _server = http_server(); let t = TempDir::new().unwrap(); let vendor_dir = t.path().join("vendor2"); - fs::write(t.path().join("mod.ts"), "import {Logger} from 'http://localhost:4545/vendor/logger/mod.ts?testing'; new Logger().log('outputted');").unwrap(); + fs::write(t.path().join("mod.ts"), "import {Logger} from 'http://localhost:4545/vendor/query_reexport.ts?testing'; new Logger().log('outputted');").unwrap(); let status = util::deno_cmd() .current_dir(t.path()) @@ -126,11 +126,11 @@ fn standard_test() { json!({ "imports": { "http://localhost:4545/": "./localhost_4545/", - "http://localhost:4545/vendor/logger/mod.ts?testing": "./localhost_4545/vendor/logger/mod.ts", + "http://localhost:4545/vendor/query_reexport.ts?testing": "./localhost_4545/vendor/query_reexport.ts", }, "scopes": { "./localhost_4545/": { - "./localhost_4545/vendor/logger/logger.ts?test": "./localhost_4545/vendor/logger/logger.ts" + "./localhost_4545/vendor/logger.ts?test": "./localhost_4545/vendor/logger.ts" } } }), @@ -151,8 +151,8 @@ fn standard_test() { .spawn() .unwrap(); let output = deno.wait_with_output().unwrap(); - assert_eq!(String::from_utf8_lossy(&output.stderr).trim(), "",); - assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "outputted",); + assert_eq!(String::from_utf8_lossy(&output.stderr).trim(), ""); + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "outputted"); assert!(output.status.success()); } @@ -165,7 +165,7 @@ fn remote_module_test() { let status = util::deno_cmd() .current_dir(t.path()) .arg("vendor") - .arg("http://localhost:4545/vendor/logger/mod.ts") + .arg("http://localhost:4545/vendor/query_reexport.ts") .spawn() .unwrap() .wait() @@ -173,11 +173,9 @@ fn remote_module_test() { assert!(status.success()); assert!(vendor_dir.exists()); assert!(vendor_dir - .join("localhost_4545/vendor/logger/mod.ts") - .exists()); - assert!(vendor_dir - .join("localhost_4545/vendor/logger/logger.ts") + .join("localhost_4545/vendor/query_reexport.ts") .exists()); + assert!(vendor_dir.join("localhost_4545/vendor/logger.ts").exists()); let import_map: serde_json::Value = serde_json::from_str( &fs::read_to_string(vendor_dir.join("import_map.json")).unwrap(), ) @@ -190,7 +188,7 @@ fn remote_module_test() { }, "scopes": { "./localhost_4545/": { - "./localhost_4545/vendor/logger/logger.ts?test": "./localhost_4545/vendor/logger/logger.ts" + "./localhost_4545/vendor/logger.ts?test": "./localhost_4545/vendor/logger.ts" } } }), @@ -204,16 +202,17 @@ fn existing_import_map() { let vendor_dir = t.path().join("vendor"); fs::write( t.path().join("mod.ts"), - "import {Logger} from 'http://localhost:4545/vendor/logger/mod.ts';", + "import {Logger} from 'http://localhost:4545/vendor/logger.ts';", ) .unwrap(); fs::write( t.path().join("imports.json"), - r#"{ "imports": { "http://localhost:4545/vendor/logger/": "./logger/" } }"#, + r#"{ "imports": { "http://localhost:4545/vendor/": "./logger/" } }"#, ) .unwrap(); fs::create_dir(t.path().join("logger")).unwrap(); - fs::write(t.path().join("logger/mod.ts"), "export class Logger {}").unwrap(); + fs::write(t.path().join("logger/logger.ts"), "export class Logger {}") + .unwrap(); let status = util::deno_cmd() .current_dir(t.path()) @@ -230,3 +229,79 @@ fn existing_import_map() { // the provided import map mapped it to a local directory assert!(!vendor_dir.join("import_map.json").exists()); } + +#[test] +fn dynamic_import() { + let _server = http_server(); + let t = TempDir::new().unwrap(); + let vendor_dir = t.path().join("vendor"); + fs::write(t.path().join("mod.ts"), "import {Logger} from 'http://localhost:4545/vendor/dynamic.ts'; new Logger().log('outputted');").unwrap(); + + let status = util::deno_cmd() + .current_dir(t.path()) + .arg("vendor") + .arg("mod.ts") + .spawn() + .unwrap() + .wait() + .unwrap(); + assert!(status.success()); + let import_map: serde_json::Value = serde_json::from_str( + &fs::read_to_string(vendor_dir.join("import_map.json")).unwrap(), + ) + .unwrap(); + assert_eq!( + import_map, + json!({ + "imports": { + "http://localhost:4545/": "./localhost_4545/", + } + }), + ); + + // try running the output with `--no-remote` + let deno = util::deno_cmd() + .current_dir(t.path()) + .env("NO_COLOR", "1") + .arg("run") + .arg("--allow-read=.") + .arg("--no-remote") + .arg("--no-check") + .arg("--import-map") + .arg("vendor/import_map.json") + .arg("mod.ts") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + assert_eq!(String::from_utf8_lossy(&output.stderr).trim(), ""); + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "outputted"); + assert!(output.status.success()); +} + +#[test] +fn dynamic_non_analyzable_import() { + let _server = http_server(); + let t = TempDir::new().unwrap(); + fs::write(t.path().join("mod.ts"), "import {Logger} from 'http://localhost:4545/vendor/dynamic_non_analyzable.ts'; new Logger().log('outputted');").unwrap(); + + let deno = util::deno_cmd() + .current_dir(t.path()) + .env("NO_COLOR", "1") + .arg("vendor") + .arg("mod.ts") + .arg("--reload") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + // todo(dsherret): it should warn about how it couldn't analyze the dynamic import + assert_eq!( + String::from_utf8_lossy(&output.stderr).trim(), + "Download http://localhost:4545/vendor/dynamic_non_analyzable.ts" + ); + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), ""); + assert!(output.status.success()); +} diff --git a/cli/tests/testdata/vendor/dynamic.ts b/cli/tests/testdata/vendor/dynamic.ts new file mode 100644 index 0000000000000..e2cbb0e59e94a --- /dev/null +++ b/cli/tests/testdata/vendor/dynamic.ts @@ -0,0 +1,3 @@ +const { Logger } = await import("./logger.ts"); + +export { Logger }; diff --git a/cli/tests/testdata/vendor/dynamic_non_analyzable.ts b/cli/tests/testdata/vendor/dynamic_non_analyzable.ts new file mode 100644 index 0000000000000..1847939f647c5 --- /dev/null +++ b/cli/tests/testdata/vendor/dynamic_non_analyzable.ts @@ -0,0 +1,4 @@ +const value = (() => "./logger.ts")(); +const { Logger } = await import(value); + +export { Logger }; diff --git a/cli/tests/testdata/vendor/logger/logger.ts b/cli/tests/testdata/vendor/logger.ts similarity index 100% rename from cli/tests/testdata/vendor/logger/logger.ts rename to cli/tests/testdata/vendor/logger.ts diff --git a/cli/tests/testdata/vendor/logger/mod.ts b/cli/tests/testdata/vendor/query_reexport.ts similarity index 100% rename from cli/tests/testdata/vendor/logger/mod.ts rename to cli/tests/testdata/vendor/query_reexport.ts From 8d10e4d8b862ee1836acdc2440143a196e7b6e17 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 18:38:34 -0500 Subject: [PATCH 22/33] Add support for more flags. --- cli/flags.rs | 31 ++++++++++++++++++---- cli/tests/integration/vendor_tests.rs | 38 ++++++++++++++++++++++----- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index adc3bdd169b13..eeace185a7fd4 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1449,7 +1449,11 @@ Remote modules and multiple modules may also be specified: ) .takes_value(false), ) + .arg(config_arg()) .arg(import_map_arg()) + .arg(lock_arg()) + .arg(reload_arg()) + .arg(ca_file_arg()) } fn compile_args(app: App) -> App { @@ -2275,7 +2279,12 @@ fn upgrade_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } fn vendor_parse(flags: &mut Flags, matches: &clap::ArgMatches) { + ca_file_arg_parse(flags, matches); + config_arg_parse(flags, matches); import_map_arg_parse(flags, matches); + lock_arg_parse(flags, matches); + reload_arg_parse(flags, matches); + flags.subcommand = DenoSubcommand::Vendor(VendorFlags { specifiers: matches .values_of("specifiers") @@ -2492,13 +2501,17 @@ fn no_check_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } fn lock_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) { + lock_arg_parse(flags, matches); + if matches.is_present("lock-write") { + flags.lock_write = true; + } +} + +fn lock_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) { if matches.is_present("lock") { let lockfile = matches.value_of("lock").unwrap(); flags.lock = Some(PathBuf::from(lockfile)); } - if matches.is_present("lock-write") { - flags.lock_write = true; - } } fn config_arg_parse(flags: &mut Flags, matches: &ArgMatches) { @@ -4941,11 +4954,16 @@ mod tests { let r = flags_from_vec(svec![ "deno", "vendor", + "--config", + "deno.json", + "--import-map", + "import_map.json", + "--lock", + "lock.json", "--force", "--output", "out_dir", - "--import-map", - "import_map.json", + "--reload", "mod.ts", "deps.test.ts", ]); @@ -4957,7 +4975,10 @@ mod tests { force: true, output_path: Some(PathBuf::from("out_dir")), }), + config_path: Some("deno.json".to_string()), import_map_path: Some("import_map.json".to_string()), + lock: Some(PathBuf::from("lock.json")), + reload: true, ..Flags::default() } ); diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index d854c308f75cb..889bd30d3bf2c 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -28,7 +28,14 @@ fn output_dir_exists() { let output = deno.wait_with_output().unwrap(); assert_eq!( String::from_utf8_lossy(&output.stderr).trim(), - format!("error: Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", vendor_dir.display()), + format!( + concat!( + "error: Directory {} was not empty. Please provide an empty ", + "directory or use --force to ignore this error and potentially ", + "overwrite its contents.", + ), + vendor_dir.display(), + ), ); assert!(!output.status.success()); @@ -46,7 +53,14 @@ fn output_dir_exists() { let output = deno.wait_with_output().unwrap(); assert_eq!( String::from_utf8_lossy(&output.stderr).trim(), - format!("error: Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", vendor_dir.display()), + format!( + concat!( + "error: Directory {} was not empty. Please provide an empty ", + "directory or use --force to ignore this error and potentially ", + "overwrite its contents.", + ), + vendor_dir.display(), + ), ); assert!(!output.status.success()); @@ -102,7 +116,10 @@ fn standard_test() { let _server = http_server(); let t = TempDir::new().unwrap(); let vendor_dir = t.path().join("vendor2"); - fs::write(t.path().join("mod.ts"), "import {Logger} from 'http://localhost:4545/vendor/query_reexport.ts?testing'; new Logger().log('outputted');").unwrap(); + fs::write( + t.path().join("mod.ts"), + "import {Logger} from 'http://localhost:4545/vendor/query_reexport.ts?testing'; new Logger().log('outputted');", + ).unwrap(); let status = util::deno_cmd() .current_dir(t.path()) @@ -235,7 +252,10 @@ fn dynamic_import() { let _server = http_server(); let t = TempDir::new().unwrap(); let vendor_dir = t.path().join("vendor"); - fs::write(t.path().join("mod.ts"), "import {Logger} from 'http://localhost:4545/vendor/dynamic.ts'; new Logger().log('outputted');").unwrap(); + fs::write( + t.path().join("mod.ts"), + "import {Logger} from 'http://localhost:4545/vendor/dynamic.ts'; new Logger().log('outputted');", + ).unwrap(); let status = util::deno_cmd() .current_dir(t.path()) @@ -284,20 +304,24 @@ fn dynamic_import() { fn dynamic_non_analyzable_import() { let _server = http_server(); let t = TempDir::new().unwrap(); - fs::write(t.path().join("mod.ts"), "import {Logger} from 'http://localhost:4545/vendor/dynamic_non_analyzable.ts'; new Logger().log('outputted');").unwrap(); + fs::write( + t.path().join("mod.ts"), + "import {Logger} from 'http://localhost:4545/vendor/dynamic_non_analyzable.ts'; new Logger().log('outputted');", + ).unwrap(); let deno = util::deno_cmd() .current_dir(t.path()) .env("NO_COLOR", "1") .arg("vendor") - .arg("mod.ts") .arg("--reload") + .arg("mod.ts") .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() .unwrap(); let output = deno.wait_with_output().unwrap(); - // todo(dsherret): it should warn about how it couldn't analyze the dynamic import + // todo(https://github.com/denoland/deno_graph/issues/138): it should warn about + // how it couldn't analyze the dynamic import assert_eq!( String::from_utf8_lossy(&output.stderr).trim(), "Download http://localhost:4545/vendor/dynamic_non_analyzable.ts" From 72fb9b49d48c8f6526b7a58bf6257773a5630892 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 19:14:24 -0500 Subject: [PATCH 23/33] Updates based on self review. --- cli/flags.rs | 6 +++--- cli/fs_util.rs | 2 ++ cli/tools/vendor/analyze.rs | 2 ++ cli/tools/vendor/build.rs | 1 + 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index eeace185a7fd4..1a0a647bcfd0f 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1416,9 +1416,9 @@ fn vendor_subcommand<'a>() -> App<'a> { .long_about( "Vendor remote modules into a local directory. -Analyzes the provided modules and their dependencies, downloads remote -modules to the output directory, and produces an import map that maps -remote specifiers to the downloaded files. +Analyzes the provided modules along with their dependencies, downloads +remote modules to the output directory, and produces an import map that +maps remote specifiers to the downloaded files. deno vendor main.ts deno run --import-map vendor/import_map.json main.ts diff --git a/cli/fs_util.rs b/cli/fs_util.rs index 4f06a0c13058e..fb87e3e10ce11 100644 --- a/cli/fs_util.rs +++ b/cli/fs_util.rs @@ -363,6 +363,8 @@ pub fn path_has_trailing_slash(path: &Path) -> bool { } /// Gets a path with the specified file stem suffix. +/// +/// Ex. `file.ts` with suffix `_2` returns `file_2.ts` pub fn path_with_stem_suffix(path: &Path, suffix: &str) -> PathBuf { if let Some(file_name) = path.file_name().map(|f| f.to_string_lossy()) { if let Some(file_stem) = path.file_stem().map(|f| f.to_string_lossy()) { diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs index e6d953a5afab7..0639c04876930 100644 --- a/cli/tools/vendor/analyze.rs +++ b/cli/tools/vendor/analyze.rs @@ -1,3 +1,5 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + use deno_ast::swc::ast::ExportDefaultDecl; use deno_ast::swc::ast::ExportSpecifier; use deno_ast::swc::ast::ModuleExportName; diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index 7ac70870605f9..9dcf2e23064a5 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -13,6 +13,7 @@ use super::mappings::Mappings; use super::mappings::ProxiedModule; use super::specifiers::is_remote_specifier; +/// Allows substituting the environment for testing purposes. pub trait VendorEnvironment { fn create_dir_all(&self, dir_path: &Path) -> Result<(), AnyError>; fn write_file(&self, file_path: &Path, text: &str) -> Result<(), AnyError>; From c8a8aa5c457aeffc74db7c29a6696a79f53d4245 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 19:25:55 -0500 Subject: [PATCH 24/33] Fix fs_util test Was only running vendor tests mistakenly. --- cli/fs_util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/fs_util.rs b/cli/fs_util.rs index fb87e3e10ce11..2f10a523f4af7 100644 --- a/cli/fs_util.rs +++ b/cli/fs_util.rs @@ -371,7 +371,7 @@ pub fn path_with_stem_suffix(path: &Path, suffix: &str) -> PathBuf { if let Some(ext) = path.extension().map(|f| f.to_string_lossy()) { return if file_stem.to_lowercase().ends_with(".d") { path.with_file_name(format!( - "{}_{}.{}.{}", + "{}{}.{}.{}", &file_stem[..file_stem.len() - ".d".len()], suffix, // maintain casing From 3d097a5bc62d147eaab293f7aeca2c9297a4a64e Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 19:47:14 -0500 Subject: [PATCH 25/33] Maybe fix mac. --- cli/tests/integration/vendor_tests.rs | 9 +++++---- cli/tools/vendor/mod.rs | 16 +++++++++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index 889bd30d3bf2c..80ce455bc666d 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -2,6 +2,7 @@ use deno_core::serde_json; use deno_core::serde_json::json; +use deno_runtime::fs_util; use pretty_assertions::assert_eq; use std::fs; use std::process::Stdio; @@ -30,11 +31,11 @@ fn output_dir_exists() { String::from_utf8_lossy(&output.stderr).trim(), format!( concat!( - "error: Directory {} was not empty. Please provide an empty ", + "error: Output directory {} was not empty. Please provide an empty ", "directory or use --force to ignore this error and potentially ", "overwrite its contents.", ), - vendor_dir.display(), + fs_util::canonicalize_path(&vendor_dir).unwrap().display(), ), ); assert!(!output.status.success()); @@ -55,11 +56,11 @@ fn output_dir_exists() { String::from_utf8_lossy(&output.stderr).trim(), format!( concat!( - "error: Directory {} was not empty. Please provide an empty ", + "error: Output directory {} was not empty. Please provide an empty ", "directory or use --force to ignore this error and potentially ", "overwrite its contents.", ), - vendor_dir.display(), + fs_util::canonicalize_path(&vendor_dir).unwrap().display(), ), ); assert!(!output.status.success()); diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index e46b9938405d3..01187efdb0259 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -34,12 +34,18 @@ fn resolve_and_validate_output_dir( flags: &VendorFlags, ps: &ProcState, ) -> Result { - let output_dir = match &flags.output_path { - Some(output_path) => resolve_from_cwd(output_path)?, - None => std::env::current_dir()?.join("vendor"), - }; + let output_dir = resolve_from_cwd(&match &flags.output_path { + Some(output_path) => output_path.to_owned(), + None => PathBuf::from("vendor"), + })?; if !flags.force && !is_dir_empty(&output_dir)? { - bail!("Directory {} was not empty. Please provide an empty directory or use --force to ignore this error and potentially overwrite its contents.", output_dir.display()); + bail!( + concat!( + "Output directory {} was not empty. Please provide an empty directory or use ", + "--force to ignore this error and potentially overwrite its contents.", + ), + output_dir.display(), + ); } // check the import map From f04366cf37c33101e8935b98a826111bf089768c Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 20:55:46 -0500 Subject: [PATCH 26/33] Fix mac issue on a mac --- cli/tools/vendor/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 01187efdb0259..33de154bf257a 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -9,7 +9,7 @@ use deno_core::resolve_url_or_path; use deno_runtime::permissions::Permissions; use crate::flags::VendorFlags; -use crate::fs_util::resolve_from_cwd; +use crate::fs_util; use crate::lockfile; use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; @@ -34,7 +34,7 @@ fn resolve_and_validate_output_dir( flags: &VendorFlags, ps: &ProcState, ) -> Result { - let output_dir = resolve_from_cwd(&match &flags.output_path { + let output_dir = fs_util::resolve_from_cwd(&match &flags.output_path { Some(output_path) => output_path.to_owned(), None => PathBuf::from("vendor"), })?; @@ -53,6 +53,7 @@ fn resolve_and_validate_output_dir( .maybe_import_map .as_ref() .and_then(|m| m.base_url().to_file_path().ok()) + .and_then(|p| fs_util::canonicalize_path(&p).ok()) { if import_map_path.starts_with(&output_dir) { // We don't allow using the output directory to help generate the new state From 4056996ee8300d267be43d5cf6f2e2b8e632fc1f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 14 Feb 2022 21:33:11 -0500 Subject: [PATCH 27/33] Probably fix on all operating systems....... Removes displaying the output directory in the message because it's a pain to test obviously. --- cli/tests/integration/vendor_tests.rs | 23 ++++++++--------------- cli/tools/vendor/mod.rs | 15 ++++++++------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index 80ce455bc666d..aa193553f55fd 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -2,7 +2,6 @@ use deno_core::serde_json; use deno_core::serde_json::json; -use deno_runtime::fs_util; use pretty_assertions::assert_eq; use std::fs; use std::process::Stdio; @@ -29,13 +28,10 @@ fn output_dir_exists() { let output = deno.wait_with_output().unwrap(); assert_eq!( String::from_utf8_lossy(&output.stderr).trim(), - format!( - concat!( - "error: Output directory {} was not empty. Please provide an empty ", - "directory or use --force to ignore this error and potentially ", - "overwrite its contents.", - ), - fs_util::canonicalize_path(&vendor_dir).unwrap().display(), + concat!( + "error: Output directory was not empty. Please specify an empty ", + "directory or use --force to ignore this error and potentially ", + "overwrite its contents.", ), ); assert!(!output.status.success()); @@ -54,13 +50,10 @@ fn output_dir_exists() { let output = deno.wait_with_output().unwrap(); assert_eq!( String::from_utf8_lossy(&output.stderr).trim(), - format!( - concat!( - "error: Output directory {} was not empty. Please provide an empty ", - "directory or use --force to ignore this error and potentially ", - "overwrite its contents.", - ), - fs_util::canonicalize_path(&vendor_dir).unwrap().display(), + concat!( + "error: Output directory was not empty. Please specify an empty ", + "directory or use --force to ignore this error and potentially ", + "overwrite its contents.", ), ); assert!(!output.status.success()); diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 33de154bf257a..f44967006fd16 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -39,13 +39,10 @@ fn resolve_and_validate_output_dir( None => PathBuf::from("vendor"), })?; if !flags.force && !is_dir_empty(&output_dir)? { - bail!( - concat!( - "Output directory {} was not empty. Please provide an empty directory or use ", - "--force to ignore this error and potentially overwrite its contents.", - ), - output_dir.display(), - ); + bail!(concat!( + "Output directory was not empty. Please specify an empty directory or use ", + "--force to ignore this error and potentially overwrite its contents.", + )); } // check the import map @@ -55,6 +52,10 @@ fn resolve_and_validate_output_dir( .and_then(|m| m.base_url().to_file_path().ok()) .and_then(|p| fs_util::canonicalize_path(&p).ok()) { + // make the output directory in order to canonicalize it for the check below + std::fs::create_dir_all(&output_dir)?; + let output_dir = fs_util::canonicalize_path(&output_dir)?; + if import_map_path.starts_with(&output_dir) { // We don't allow using the output directory to help generate the new state // of itself because supporting this scenario adds a lot of complexity. From d14790077809c135f7d38021add4f51b101e9659 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 16 Feb 2022 10:40:12 -0500 Subject: [PATCH 28/33] Referencing a different origin should add entry to "imports" --- cli/tools/vendor/build.rs | 8 +++++++- cli/tools/vendor/import_map.rs | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index 9dcf2e23064a5..bdd032050a775 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -218,6 +218,9 @@ mod test { concat!( "export * from '../sub2/mod.ts';", "export * from '../sub2/other?asdf';", + // reference a path on a different origin + "export * from 'https://localhost/other.ts';", + "export * from 'https://localhost/redirect.ts';", ), ) .add("https://other/sub2/mod.ts", "export class Mod {}") @@ -236,12 +239,13 @@ mod test { Some(json!({ "imports": { "https://localhost/": "./localhost/", + "https://localhost/redirect.ts": "./localhost/other.ts", "https://other/": "./other/" }, "scopes": { "./localhost/": { - "/absolute.ts": "./localhost/absolute.ts", "./localhost/redirect.ts": "./localhost/other.ts", + "/absolute.ts": "./localhost/absolute.ts", }, "./other/": { "./other/sub2/other?asdf": "./other/sub2/other.js" @@ -268,6 +272,8 @@ mod test { concat!( "export * from '../sub2/mod.ts';", "export * from '../sub2/other?asdf';", + "export * from 'https://localhost/other.ts';", + "export * from 'https://localhost/redirect.ts';", ) ), ("/vendor/other/sub2/mod.ts", "export class Mod {}"), diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index 5e7a7d1982431..7e18d56aa5864 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -213,11 +213,11 @@ fn handle_dep_specifier( return; } - if !is_remote_specifier(referrer) { - import_map.imports.add(text.to_string(), &specifier); - } else { + if referrer.origin() == specifier.origin() { let imports = import_map.scope(base_specifier); imports.add(sub_path.to_string(), &specifier); + } else { + import_map.imports.add(text.to_string(), &specifier); } } else { let expected_relative_specifier_text = From 8b10b6aa7a1bf58ef0d3b6e384d8f63d64bb055d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 16 Feb 2022 11:04:06 -0500 Subject: [PATCH 29/33] Add message on success. --- cli/tests/integration/vendor_tests.rs | 42 +++++++++++++++++++++++---- cli/tools/vendor/build.rs | 5 ++-- cli/tools/vendor/mod.rs | 41 +++++++++++++++++++------- 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index aa193553f55fd..2599135ff672f 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -4,6 +4,7 @@ use deno_core::serde_json; use deno_core::serde_json::json; use pretty_assertions::assert_eq; use std::fs; +use std::path::PathBuf; use std::process::Stdio; use tempfile::TempDir; use test_util as util; @@ -115,17 +116,32 @@ fn standard_test() { "import {Logger} from 'http://localhost:4545/vendor/query_reexport.ts?testing'; new Logger().log('outputted');", ).unwrap(); - let status = util::deno_cmd() + let deno = util::deno_cmd() .current_dir(t.path()) .arg("vendor") .arg("mod.ts") .arg("--output") .arg("vendor2") + .env("NO_COLOR", "1") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) .spawn() - .unwrap() - .wait() .unwrap(); - assert!(status.success()); + let output = deno.wait_with_output().unwrap(); + assert!(output.status.success()); + assert_eq!( + String::from_utf8_lossy(&output.stderr).trim(), + format!( + concat!( + "Download http://localhost:4545/vendor/query_reexport.ts?testing\n", + "Download http://localhost:4545/vendor/logger.ts?test\n", + "{}", + ), + success_text("2 modules", "vendor2"), + ) + ); + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), ""); + assert!(vendor_dir.exists()); assert!(!t.path().join("vendor").exists()); let import_map: serde_json::Value = serde_json::from_str( @@ -318,8 +334,24 @@ fn dynamic_non_analyzable_import() { // how it couldn't analyze the dynamic import assert_eq!( String::from_utf8_lossy(&output.stderr).trim(), - "Download http://localhost:4545/vendor/dynamic_non_analyzable.ts" + format!( + "Download http://localhost:4545/vendor/dynamic_non_analyzable.ts\n{}", + success_text("1 module", "vendor/"), + ) ); assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), ""); assert!(output.status.success()); } + +fn success_text(module_count_text: &str, dir_text: &str) -> String { + format!( + concat!( + "Vendored {} into {} directory.\n\n", + "To use vendored modules, specify the `--import-map` flag when invoking deno subcommands:\n", + " deno run -A --import-map {} main.ts" + ), + module_count_text, + dir_text, + PathBuf::from(dir_text).join("import_map.json").display(), + ) +} diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index bdd032050a775..58f351dd89ed4 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -31,11 +31,12 @@ impl VendorEnvironment for RealVendorEnvironment { } } +/// Vendors remote modules and returns how many were vendored. pub fn build( graph: &ModuleGraph, output_dir: &Path, environment: &impl VendorEnvironment, -) -> Result<(), AnyError> { +) -> Result { assert!(output_dir.is_absolute()); let all_modules = graph.modules(); let remote_modules = all_modules @@ -83,7 +84,7 @@ pub fn build( .write_file(&output_dir.join("import_map.json"), &import_map_text)?; } - Ok(()) + Ok(remote_modules.len()) } fn build_proxy_module_source( diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index f44967006fd16..7544278b0da3c 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -24,21 +24,40 @@ mod specifiers; mod test; pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { - let output_dir = resolve_and_validate_output_dir(&flags, &ps)?; + let raw_output_dir = match &flags.output_path { + Some(output_path) => output_path.to_owned(), + None => PathBuf::from("vendor/"), + }; + let output_dir = fs_util::resolve_from_cwd(&raw_output_dir)?; + validate_output_dir(&output_dir, &flags, &ps)?; let graph = create_graph(&ps, &flags).await?; + let vendored_count = + build::build(&graph, &output_dir, &build::RealVendorEnvironment)?; + + eprintln!( + r#"Vendored {} {} into {} directory. + +To use vendored modules, specify the `--import-map` flag when invoking deno subcommands: + deno run -A --import-map {} main.ts"#, + vendored_count, + if vendored_count == 1 { + "module" + } else { + "modules" + }, + raw_output_dir.display(), + raw_output_dir.join("import_map.json").display(), + ); - build::build(&graph, &output_dir, &build::RealVendorEnvironment) + Ok(()) } -fn resolve_and_validate_output_dir( +fn validate_output_dir( + output_dir: &Path, flags: &VendorFlags, ps: &ProcState, -) -> Result { - let output_dir = fs_util::resolve_from_cwd(&match &flags.output_path { - Some(output_path) => output_path.to_owned(), - None => PathBuf::from("vendor"), - })?; - if !flags.force && !is_dir_empty(&output_dir)? { +) -> Result<(), AnyError> { + if !flags.force && !is_dir_empty(output_dir)? { bail!(concat!( "Output directory was not empty. Please specify an empty directory or use ", "--force to ignore this error and potentially overwrite its contents.", @@ -54,7 +73,7 @@ fn resolve_and_validate_output_dir( { // make the output directory in order to canonicalize it for the check below std::fs::create_dir_all(&output_dir)?; - let output_dir = fs_util::canonicalize_path(&output_dir)?; + let output_dir = fs_util::canonicalize_path(output_dir)?; if import_map_path.starts_with(&output_dir) { // We don't allow using the output directory to help generate the new state @@ -65,7 +84,7 @@ fn resolve_and_validate_output_dir( } } - Ok(output_dir) + Ok(()) } fn is_dir_empty(dir_path: &Path) -> Result { From 326f399dd156b07c02e887480ded976410930424 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 16 Feb 2022 11:14:48 -0500 Subject: [PATCH 30/33] Fix clippy issues on main. --- ext/http/lib.rs | 3 +-- runtime/ops/http.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/ext/http/lib.rs b/ext/http/lib.rs index e11d42da1745c..312942303a4e4 100644 --- a/ext/http/lib.rs +++ b/ext/http/lib.rs @@ -39,7 +39,6 @@ use hyper::service::Service; use hyper::Body; use hyper::Request; use hyper::Response; -use percent_encoding::percent_encode; use serde::Deserialize; use serde::Serialize; use std::borrow::Cow; @@ -428,7 +427,7 @@ fn req_url( // httpie uses http+unix://[percent_encoding_of_path]/ which we follow #[cfg(unix)] HttpSocketAddr::UnixSocket(addr) => Cow::Owned( - percent_encode( + percent_encoding::percent_encode( addr .as_pathname() .and_then(|x| x.to_str()) diff --git a/runtime/ops/http.rs b/runtime/ops/http.rs index 53a99bd47d04d..5b8acb881b5a1 100644 --- a/runtime/ops/http.rs +++ b/runtime/ops/http.rs @@ -8,7 +8,6 @@ use deno_core::OpState; use deno_core::ResourceId; use deno_http::http_create_conn_resource; use deno_net::io::TcpStreamResource; -use deno_net::io::UnixStreamResource; use deno_net::ops_tls::TlsStreamResource; pub fn init() -> Extension { @@ -49,7 +48,7 @@ fn op_http_start( #[cfg(unix)] if let Ok(resource_rc) = state .resource_table - .take::(tcp_stream_rid) + .take::(tcp_stream_rid) { super::check_unstable(state, "Deno.serveHttp"); From dd8d5617809fc041c5876dc91e002a971afddf32 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 16 Feb 2022 11:17:51 -0500 Subject: [PATCH 31/33] Update for new deno_graph changes. --- cli/tools/vendor/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index 8773127002f55..b37e2b3b0b940 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -95,7 +95,7 @@ impl Loader for TestLoader { ) -> LoadFuture { let specifier = self.redirects.get(specifier).unwrap_or(specifier); let result = self.files.get(specifier).map(|result| match result { - Ok(result) => Ok(LoadResponse { + Ok(result) => Ok(LoadResponse::Module { specifier: specifier.clone(), content: Arc::new(result.0.clone()), maybe_headers: result.1.clone(), From bfa2ee990b1bcef61776a2e1289746d54201b3c2 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 16 Feb 2022 12:04:48 -0500 Subject: [PATCH 32/33] Add context to canonicalize. --- cli/tools/vendor/mappings.rs | 2 +- cli/tools/vendor/mod.rs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index ddedad7301217..2e85445dcb939 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -83,7 +83,7 @@ impl Mappings { }, )) = &module.maybe_types_dependency { - // hack to tell if it's a x-typescript-types header + // hack to tell if it's an x-typescript-types header let is_ts_types_header = range.start == Position::zeroed() && range.end == Position::zeroed(); if is_ts_types_header { diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 7544278b0da3c..20e73ab4027c0 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -4,6 +4,7 @@ use std::path::Path; use std::path::PathBuf; use deno_core::anyhow::bail; +use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::resolve_url_or_path; use deno_runtime::permissions::Permissions; @@ -73,7 +74,10 @@ fn validate_output_dir( { // make the output directory in order to canonicalize it for the check below std::fs::create_dir_all(&output_dir)?; - let output_dir = fs_util::canonicalize_path(output_dir)?; + let output_dir = + fs_util::canonicalize_path(output_dir).with_context(|| { + format!("Failed to canonicalize: {}", output_dir.display()) + })?; if import_map_path.starts_with(&output_dir) { // We don't allow using the output directory to help generate the new state From 90bce375fe70cd3537f88e91411bb97c6a31a8ab Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 16 Feb 2022 12:32:34 -0500 Subject: [PATCH 33/33] Use first non-remote entry point in output message. --- cli/tests/integration/vendor_tests.rs | 45 ++++++++++++++++++--------- cli/tools/vendor/mod.rs | 9 +++++- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/cli/tests/integration/vendor_tests.rs b/cli/tests/integration/vendor_tests.rs index 2599135ff672f..4aa883a7ebde2 100644 --- a/cli/tests/integration/vendor_tests.rs +++ b/cli/tests/integration/vendor_tests.rs @@ -112,14 +112,14 @@ fn standard_test() { let t = TempDir::new().unwrap(); let vendor_dir = t.path().join("vendor2"); fs::write( - t.path().join("mod.ts"), + t.path().join("my_app.ts"), "import {Logger} from 'http://localhost:4545/vendor/query_reexport.ts?testing'; new Logger().log('outputted');", ).unwrap(); let deno = util::deno_cmd() .current_dir(t.path()) .arg("vendor") - .arg("mod.ts") + .arg("my_app.ts") .arg("--output") .arg("vendor2") .env("NO_COLOR", "1") @@ -128,7 +128,6 @@ fn standard_test() { .spawn() .unwrap(); let output = deno.wait_with_output().unwrap(); - assert!(output.status.success()); assert_eq!( String::from_utf8_lossy(&output.stderr).trim(), format!( @@ -137,10 +136,11 @@ fn standard_test() { "Download http://localhost:4545/vendor/logger.ts?test\n", "{}", ), - success_text("2 modules", "vendor2"), + success_text("2 modules", "vendor2", "my_app.ts"), ) ); assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), ""); + assert!(output.status.success()); assert!(vendor_dir.exists()); assert!(!t.path().join("vendor").exists()); @@ -172,7 +172,7 @@ fn standard_test() { .arg("--no-check") .arg("--import-map") .arg("vendor2/import_map.json") - .arg("mod.ts") + .arg("my_app.ts") .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() @@ -189,15 +189,29 @@ fn remote_module_test() { let t = TempDir::new().unwrap(); let vendor_dir = t.path().join("vendor"); - let status = util::deno_cmd() + let deno = util::deno_cmd() .current_dir(t.path()) + .env("NO_COLOR", "1") .arg("vendor") .arg("http://localhost:4545/vendor/query_reexport.ts") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) .spawn() - .unwrap() - .wait() .unwrap(); - assert!(status.success()); + let output = deno.wait_with_output().unwrap(); + assert_eq!( + String::from_utf8_lossy(&output.stderr).trim(), + format!( + concat!( + "Download http://localhost:4545/vendor/query_reexport.ts\n", + "Download http://localhost:4545/vendor/logger.ts?test\n", + "{}", + ), + success_text("2 modules", "vendor/", "main.ts"), + ) + ); + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), ""); + assert!(output.status.success()); assert!(vendor_dir.exists()); assert!(vendor_dir .join("localhost_4545/vendor/query_reexport.ts") @@ -336,22 +350,23 @@ fn dynamic_non_analyzable_import() { String::from_utf8_lossy(&output.stderr).trim(), format!( "Download http://localhost:4545/vendor/dynamic_non_analyzable.ts\n{}", - success_text("1 module", "vendor/"), + success_text("1 module", "vendor/", "mod.ts"), ) ); assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), ""); assert!(output.status.success()); } -fn success_text(module_count_text: &str, dir_text: &str) -> String { +fn success_text(module_count: &str, dir: &str, entry_point: &str) -> String { format!( concat!( "Vendored {} into {} directory.\n\n", "To use vendored modules, specify the `--import-map` flag when invoking deno subcommands:\n", - " deno run -A --import-map {} main.ts" + " deno run -A --import-map {} {}" ), - module_count_text, - dir_text, - PathBuf::from(dir_text).join("import_map.json").display(), + module_count, + dir, + PathBuf::from(dir).join("import_map.json").display(), + entry_point, ) } diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 20e73ab4027c0..eb9c91071bc01 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -15,6 +15,7 @@ use crate::lockfile; use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; +use crate::tools::vendor::specifiers::is_remote_specifier_text; mod analyze; mod build; @@ -39,7 +40,7 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> { r#"Vendored {} {} into {} directory. To use vendored modules, specify the `--import-map` flag when invoking deno subcommands: - deno run -A --import-map {} main.ts"#, + deno run -A --import-map {} {}"#, vendored_count, if vendored_count == 1 { "module" @@ -48,6 +49,12 @@ To use vendored modules, specify the `--import-map` flag when invoking deno subc }, raw_output_dir.display(), raw_output_dir.join("import_map.json").display(), + flags + .specifiers + .iter() + .map(|s| s.as_str()) + .find(|s| !is_remote_specifier_text(s)) + .unwrap_or("main.ts"), ); Ok(())