From a1d823e27d1b605b5658fddc1c9273667f0e9e84 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 1 Dec 2023 15:12:10 -0500 Subject: [PATCH] feat(compile): support discovering modules for more dynamic arguments (#21381) This PR causes Deno to include more files in the graph based on how a template literal looks that's provided to a dynamic import: ```ts const file = await import(`./dir/${expr}`); ``` In this case, it will search the `dir` directory and descendant directories for any .js/jsx/etc modules and include them in the graph. To opt out of this behaviour, move the template literal to a separate line: ```ts const specifier = `./dir/${expr}` const file = await import(specifier); ``` --- Cargo.lock | 21 +-- cli/Cargo.toml | 10 +- cli/factory.rs | 2 + cli/graph_util.rs | 88 ++++++++++- cli/lsp/documents.rs | 17 ++- cli/module_loader.rs | 5 + cli/tests/integration/compile_tests.rs | 24 +++ cli/tests/integration/doc_tests.rs | 10 +- cli/tests/integration/info_tests.rs | 6 + .../dynamic_imports_tmp_lit/main.info.out | 10 ++ .../compile/dynamic_imports_tmp_lit/main.js | 14 ++ .../dynamic_imports_tmp_lit/other/data.json | 3 + .../other/sub/data2.json | 3 + .../compile/dynamic_imports_tmp_lit/sub/a.js | 1 + .../compile/dynamic_imports_tmp_lit/sub/b.ts | 1 + cli/tests/testdata/doc/lint_success_html.out | 2 +- .../registry/@denotest/deps/1.0.0_meta.json | 2 + .../@denotest/module_graph/1.4.0_meta.json | 1 + cli/tools/doc.rs | 7 +- cli/util/import_map.rs | 142 ++++++++++++++++-- 20 files changed, 327 insertions(+), 42 deletions(-) create mode 100644 cli/tests/testdata/compile/dynamic_imports_tmp_lit/main.info.out create mode 100644 cli/tests/testdata/compile/dynamic_imports_tmp_lit/main.js create mode 100644 cli/tests/testdata/compile/dynamic_imports_tmp_lit/other/data.json create mode 100644 cli/tests/testdata/compile/dynamic_imports_tmp_lit/other/sub/data2.json create mode 100644 cli/tests/testdata/compile/dynamic_imports_tmp_lit/sub/a.js create mode 100644 cli/tests/testdata/compile/dynamic_imports_tmp_lit/sub/b.ts diff --git a/Cargo.lock b/Cargo.lock index 1dffcb877a..269bf912da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1139,9 +1139,9 @@ dependencies = [ [[package]] name = "deno_doc" -version = "0.73.3" +version = "0.73.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "729bacea0b5a1b2406f70c105ba82354e0f4bd99c51ede4fd29b15ce750dcdd4" +checksum = "76d4d2960ce5c7af64e57ec4e7fd99023bd1ab664cedc4ec267db1d294b6002a" dependencies = [ "anyhow", "cfg-if", @@ -1162,9 +1162,9 @@ dependencies = [ [[package]] name = "deno_emit" -version = "0.31.4" +version = "0.31.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8910a6da498d0eb2a28d9ea613c47291a86377a85b3771dd90d624004814aeb" +checksum = "5b98917905e9be9740722f89c3b2d3a963beaed8a05dce58e642947d0f6aa17d" dependencies = [ "anyhow", "base64 0.13.1", @@ -1230,9 +1230,9 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.61.1" +version = "0.61.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "076c0b611c10901456b78c837408b9c40fe0c3602e767307d986f46f0cc56b51" +checksum = "332e6a1930b3266dc848edeaf1426bbbd3ddca25c5e107e70823efb1b3ce68be" dependencies = [ "anyhow", "async-trait", @@ -1242,6 +1242,7 @@ dependencies = [ "futures", "import_map", "indexmap 2.0.2", + "log", "monch", "once_cell", "parking_lot 0.12.1", @@ -2203,9 +2204,9 @@ dependencies = [ [[package]] name = "eszip" -version = "0.55.4" +version = "0.55.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0535cf1ea8c46379c39d791dc87e7104e90b3730829ee6d8888285eab22fa69" +checksum = "455c055f28756fc7ba0d64506e6167b91878f3a39854271d7d63b577deb8b45d" dependencies = [ "anyhow", "base64 0.21.4", @@ -2854,9 +2855,9 @@ checksum = "cb56e1aa765b4b4f3aadfab769793b7087bb03a4ea4920644a6d238e2df5b9ed" [[package]] name = "import_map" -version = "0.17.0" +version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e5bf51a0adfdc08afcb9e5a1c8f8c804227ec50d493c65e57e6d117d594bd1b" +checksum = "0ecd467768fe83c2860e70e5de5297a7366a230ff53e1da2158bdac2384cd39d" dependencies = [ "indexmap 1.9.3", "log", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index d7837ecf38..d32181ebc9 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -57,16 +57,16 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_gra deno_cache_dir = "=0.6.1" deno_config = "=0.6.5" deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } -deno_doc = { version = "=0.73.3", features = ["html"] } -deno_emit = "=0.31.4" -deno_graph = "=0.61.1" +deno_doc = { version = "=0.73.5", features = ["html"] } +deno_emit = "=0.31.5" +deno_graph = "=0.61.5" deno_lint = { version = "=0.52.2", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "0.15.2" deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "exclude_runtime_main_js", "include_js_files_for_snapshotting"] } deno_semver = "0.5.1" deno_task_shell = "=0.14.0" -eszip = "=0.55.4" +eszip = "=0.55.5" napi_sym.workspace = true async-trait.workspace = true @@ -100,7 +100,7 @@ glob = "0.3.1" hex.workspace = true http.workspace = true hyper.workspace = true -import_map = { version = "=0.17.0", features = ["ext"] } +import_map = { version = "=0.18.0", features = ["ext"] } indexmap.workspace = true jsonc-parser = { version = "=0.23.0", features = ["serde"] } lazy-regex.workspace = true diff --git a/cli/factory.rs b/cli/factory.rs index fc4819d648..027cc8fe4e 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -513,6 +513,7 @@ impl CliFactory { .get_or_try_init_async(async { Ok(Arc::new(ModuleGraphBuilder::new( self.options.clone(), + self.fs().clone(), self.resolver().await?.clone(), self.npm_resolver().await?.clone(), self.module_info_cache()?.clone(), @@ -556,6 +557,7 @@ impl CliFactory { .get_or_try_init_async(async { Ok(Arc::new(ModuleLoadPreparer::new( self.options.clone(), + self.fs().clone(), self.graph_container().clone(), self.maybe_lockfile().clone(), self.maybe_file_watcher_reporter().clone(), diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 727037748e..445f269e31 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -35,6 +35,7 @@ use deno_graph::ModuleGraph; use deno_graph::ModuleGraphError; use deno_graph::ResolutionError; use deno_graph::SpecifierError; +use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node; use deno_runtime::permissions::PermissionsContainer; use deno_semver::package::PackageNv; @@ -116,12 +117,8 @@ pub fn graph_valid( if let Some(range) = error.maybe_range() { if !is_root && !range.specifier.as_str().contains("/$deno$eval") { - message.push_str(&format!( - "\n at {}:{}:{}", - colors::cyan(range.specifier.as_str()), - colors::yellow(&(range.start.line + 1).to_string()), - colors::yellow(&(range.start.character + 1).to_string()) - )); + message.push_str("\n at "); + message.push_str(&format_range_with_colors(range)); } } @@ -194,6 +191,7 @@ pub struct CreateGraphOptions<'a> { pub struct ModuleGraphBuilder { options: Arc, + fs: Arc, resolver: Arc, npm_resolver: Arc, module_info_cache: Arc, @@ -210,6 +208,7 @@ impl ModuleGraphBuilder { #[allow(clippy::too_many_arguments)] pub fn new( options: Arc, + fs: Arc, resolver: Arc, npm_resolver: Arc, module_info_cache: Arc, @@ -223,6 +222,7 @@ impl ModuleGraphBuilder { ) -> Self { Self { options, + fs, resolver, npm_resolver, module_info_cache, @@ -295,6 +295,7 @@ impl ModuleGraphBuilder { is_dynamic: false, imports: maybe_imports, resolver: Some(graph_resolver), + file_system: Some(&DenoGraphFsAdapter(self.fs.as_ref())), npm_resolver: Some(graph_npm_resolver), module_analyzer: Some(options.analyzer), reporter: maybe_file_watcher_reporter, @@ -344,6 +345,7 @@ impl ModuleGraphBuilder { deno_graph::BuildOptions { is_dynamic: false, imports: maybe_imports, + file_system: Some(&DenoGraphFsAdapter(self.fs.as_ref())), resolver: Some(graph_resolver), npm_resolver: Some(graph_npm_resolver), module_analyzer: Some(&analyzer), @@ -770,6 +772,80 @@ fn workspace_member_config_try_into_workspace_member( }) } +pub struct DenoGraphFsAdapter<'a>( + pub &'a dyn deno_runtime::deno_fs::FileSystem, +); + +impl<'a> deno_graph::source::FileSystem for DenoGraphFsAdapter<'a> { + fn read_dir( + &self, + dir_url: &deno_graph::ModuleSpecifier, + ) -> Vec { + use deno_core::anyhow; + use deno_graph::source::DirEntry; + use deno_graph::source::DirEntryKind; + + let dir_path = match dir_url.to_file_path() { + Ok(path) => path, + // ignore, treat as non-analyzable + Err(()) => return vec![], + }; + let entries = match self.0.read_dir_sync(&dir_path) { + Ok(dir) => dir, + Err(err) + if matches!( + err.kind(), + std::io::ErrorKind::PermissionDenied | std::io::ErrorKind::NotFound + ) => + { + return vec![]; + } + Err(err) => { + return vec![DirEntry { + kind: DirEntryKind::Error( + anyhow::Error::from(err) + .context("Failed to read directory.".to_string()), + ), + url: dir_url.clone(), + }]; + } + }; + let mut dir_entries = Vec::with_capacity(entries.len()); + for entry in entries { + let entry_path = dir_path.join(&entry.name); + dir_entries.push(if entry.is_directory { + DirEntry { + kind: DirEntryKind::Dir, + url: ModuleSpecifier::from_directory_path(&entry_path).unwrap(), + } + } else if entry.is_file { + DirEntry { + kind: DirEntryKind::File, + url: ModuleSpecifier::from_file_path(&entry_path).unwrap(), + } + } else if entry.is_symlink { + DirEntry { + kind: DirEntryKind::Symlink, + url: ModuleSpecifier::from_file_path(&entry_path).unwrap(), + } + } else { + continue; + }); + } + + dir_entries + } +} + +pub fn format_range_with_colors(range: &deno_graph::Range) -> String { + format!( + "{}:{}:{}", + colors::cyan(range.specifier.as_str()), + colors::yellow(&(range.start.line + 1).to_string()), + colors::yellow(&(range.start.character + 1).to_string()) + ) +} + #[cfg(test)] mod test { use std::sync::Arc; diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index ede063c5fb..95e8df917b 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1743,12 +1743,17 @@ fn analyze_module( ) -> ModuleResult { match parsed_source_result { Ok(parsed_source) => Ok(deno_graph::parse_module_from_ast( - deno_graph::GraphKind::All, - specifier, - maybe_headers, - parsed_source, - Some(resolver), - Some(npm_resolver), + deno_graph::ParseModuleFromAstOptions { + graph_kind: deno_graph::GraphKind::All, + specifier, + maybe_headers, + parsed_source, + // use a null file system because there's no need to bother resolving + // dynamic imports like import(`./dir/${something}`) in the LSP + file_system: &deno_graph::source::NullFileSystem, + maybe_resolver: Some(resolver), + maybe_npm_resolver: Some(npm_resolver), + }, )), Err(err) => Err(deno_graph::ModuleGraphError::ModuleError( deno_graph::ModuleError::ParseErr(specifier.clone(), err.clone()), diff --git a/cli/module_loader.rs b/cli/module_loader.rs index d8ab2d8c9c..8332ea4f99 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -9,6 +9,7 @@ use crate::emit::Emitter; use crate::graph_util::graph_lock_or_exit; use crate::graph_util::graph_valid_with_cli_options; use crate::graph_util::workspace_config_to_workspace_members; +use crate::graph_util::DenoGraphFsAdapter; use crate::graph_util::FileWatcherReporter; use crate::graph_util::ModuleGraphBuilder; use crate::graph_util::ModuleGraphContainer; @@ -64,6 +65,7 @@ use std::sync::Arc; pub struct ModuleLoadPreparer { options: Arc, + fs: Arc, graph_container: Arc, lockfile: Option>>, maybe_file_watcher_reporter: Option, @@ -79,6 +81,7 @@ impl ModuleLoadPreparer { #[allow(clippy::too_many_arguments)] pub fn new( options: Arc, + fs: Arc, graph_container: Arc, lockfile: Option>>, maybe_file_watcher_reporter: Option, @@ -91,6 +94,7 @@ impl ModuleLoadPreparer { ) -> Self { Self { options, + fs, graph_container, lockfile, maybe_file_watcher_reporter, @@ -155,6 +159,7 @@ impl ModuleLoadPreparer { deno_graph::BuildOptions { is_dynamic, imports: maybe_imports, + file_system: Some(&DenoGraphFsAdapter(self.fs.as_ref())), resolver: Some(graph_resolver), npm_resolver: Some(graph_npm_resolver), module_analyzer: Some(&analyzer), diff --git a/cli/tests/integration/compile_tests.rs b/cli/tests/integration/compile_tests.rs index df339c3690..653b2bc8da 100644 --- a/cli/tests/integration/compile_tests.rs +++ b/cli/tests/integration/compile_tests.rs @@ -1061,3 +1061,27 @@ fn compile_node_modules_symlink_outside() { let output = context.new_command().name(binary_path).run(); output.assert_matches_file("compile/node_modules_symlink_outside/main.out"); } + +#[test] +fn dynamic_imports_tmp_lit() { + let context = TestContextBuilder::new().build(); + let dir = context.temp_dir(); + let exe = if cfg!(windows) { + dir.path().join("app.exe") + } else { + dir.path().join("app") + }; + let output = context + .new_command() + .args_vec([ + "compile", + "--output", + &exe.to_string_lossy(), + "./compile/dynamic_imports_tmp_lit/main.js", + ]) + .run(); + output.assert_exit_code(0); + output.skip_output_check(); + let output = context.new_command().name(&exe).run(); + output.assert_matches_text("a\nb\n{ data: 5 }\n{ data: 1 }\n"); +} diff --git a/cli/tests/integration/doc_tests.rs b/cli/tests/integration/doc_tests.rs index 7a3f08f487..9accd83040 100644 --- a/cli/tests/integration/doc_tests.rs +++ b/cli/tests/integration/doc_tests.rs @@ -138,13 +138,15 @@ fn deno_doc_html() { .run(); output.assert_exit_code(0); - assert_contains!(output.stderr(), "Written 8 files to"); + assert_contains!(output.stderr(), "Written 10 files to"); + assert!(temp_dir.path().join("all_symbols.html").exists()); assert!(temp_dir.path().join("index.html").exists()); - assert!(temp_dir.path().join("compound_index.html").exists()); assert!(temp_dir.path().join("fuse.js").exists()); + assert!(temp_dir.path().join("page.css").exists()); assert!(temp_dir.path().join("search.js").exists()); assert!(temp_dir.path().join("search_index.js").exists()); assert!(temp_dir.path().join("styles.css").exists()); - assert!(temp_dir.path().join("MyInterface.html").exists()); - assert!(temp_dir.path().join("MyClass.html").exists()); + assert!(temp_dir.path().join("~/MyInterface.html").exists()); + assert!(temp_dir.path().join("~/MyClass.html").exists()); + assert!(temp_dir.path().join("~/index.html").exists()); } diff --git a/cli/tests/integration/info_tests.rs b/cli/tests/integration/info_tests.rs index 47c5cc1d07..f7889f3c31 100644 --- a/cli/tests/integration/info_tests.rs +++ b/cli/tests/integration/info_tests.rs @@ -154,3 +154,9 @@ itest!(info_import_map { cwd: Some("info/with_import_map"), exit_code: 0, }); + +itest!(info_dynamic_imports_tmpl_lit { + args: "info compile/dynamic_imports_tmp_lit/main.js", + output: "compile/dynamic_imports_tmp_lit/main.info.out", + exit_code: 0, +}); diff --git a/cli/tests/testdata/compile/dynamic_imports_tmp_lit/main.info.out b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/main.info.out new file mode 100644 index 0000000000..57d730a643 --- /dev/null +++ b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/main.info.out @@ -0,0 +1,10 @@ +local: [WILDCARD]main.js +type: JavaScript +dependencies: 4 unique +size: [WILDCARD] + +file:///[WILDCARD]/dynamic_imports_tmp_lit/main.js ([WILDCARD]) +├── file:///[WILDCARD]/dynamic_imports_tmp_lit/sub/a.js ([WILDCARD]) +├── file:///[WILDCARD]/dynamic_imports_tmp_lit/sub/b.ts ([WILDCARD]) +├── file:///[WILDCARD]/dynamic_imports_tmp_lit/other/data.json ([WILDCARD]) +└── file:///[WILDCARD]/dynamic_imports_tmp_lit/other/sub/data2.json ([WILDCARD]) diff --git a/cli/tests/testdata/compile/dynamic_imports_tmp_lit/main.js b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/main.js new file mode 100644 index 0000000000..3bda597724 --- /dev/null +++ b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/main.js @@ -0,0 +1,14 @@ +const fileNames = [ + "a.js", + "b.ts", +]; + +for (const fileName of fileNames) { + await import(`./sub/${fileName}`); +} + +const jsonFileNames = ["data.json", "sub/data2.json"]; +for (const fileName of jsonFileNames) { + const mod = await import(`./other/${fileName}`, { with: { type: "json" } }); + console.log(mod.default); +} diff --git a/cli/tests/testdata/compile/dynamic_imports_tmp_lit/other/data.json b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/other/data.json new file mode 100644 index 0000000000..0131e01e4b --- /dev/null +++ b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/other/data.json @@ -0,0 +1,3 @@ +{ + "data": 5 +} diff --git a/cli/tests/testdata/compile/dynamic_imports_tmp_lit/other/sub/data2.json b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/other/sub/data2.json new file mode 100644 index 0000000000..858a13cdd1 --- /dev/null +++ b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/other/sub/data2.json @@ -0,0 +1,3 @@ +{ + "data": 1 +} diff --git a/cli/tests/testdata/compile/dynamic_imports_tmp_lit/sub/a.js b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/sub/a.js new file mode 100644 index 0000000000..7b2a346011 --- /dev/null +++ b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/sub/a.js @@ -0,0 +1 @@ +console.log("a"); diff --git a/cli/tests/testdata/compile/dynamic_imports_tmp_lit/sub/b.ts b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/sub/b.ts new file mode 100644 index 0000000000..6d012e7f1f --- /dev/null +++ b/cli/tests/testdata/compile/dynamic_imports_tmp_lit/sub/b.ts @@ -0,0 +1 @@ +console.log("b"); diff --git a/cli/tests/testdata/doc/lint_success_html.out b/cli/tests/testdata/doc/lint_success_html.out index 8c4c2d187e..9503a335fe 100644 --- a/cli/tests/testdata/doc/lint_success_html.out +++ b/cli/tests/testdata/doc/lint_success_html.out @@ -1 +1 @@ -Written 7 files to "./docs/" +Written 9 files to "./docs/" diff --git a/cli/tests/testdata/jsr/registry/@denotest/deps/1.0.0_meta.json b/cli/tests/testdata/jsr/registry/@denotest/deps/1.0.0_meta.json index f60c650268..914e4bd73c 100644 --- a/cli/tests/testdata/jsr/registry/@denotest/deps/1.0.0_meta.json +++ b/cli/tests/testdata/jsr/registry/@denotest/deps/1.0.0_meta.json @@ -5,11 +5,13 @@ "moduleGraph1": { "/mod.ts": { "dependencies": [{ + "type": "static", "kind": "import", "range": [[0, 0], [0, 59]], "specifier": "jsr:@denotest/module_graph@1/other", "specifierRange": [[0, 22], [0, 58]] }, { + "type": "static", "kind": "import", "range": [[1, 0], [1, 57]], "specifier": "jsr:@denotest/no_module_graph@^0.1", diff --git a/cli/tests/testdata/jsr/registry/@denotest/module_graph/1.4.0_meta.json b/cli/tests/testdata/jsr/registry/@denotest/module_graph/1.4.0_meta.json index 8745d72b90..ff105b58ab 100644 --- a/cli/tests/testdata/jsr/registry/@denotest/module_graph/1.4.0_meta.json +++ b/cli/tests/testdata/jsr/registry/@denotest/module_graph/1.4.0_meta.json @@ -7,6 +7,7 @@ "/mod.ts": { "dependencies": [{ "kind": "import", + "type": "static", "range": [[0, 0], [0, 35]], "specifier": "./other.ts", "specifierRange": [[0, 22], [0, 34]] diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index ff8b8d62ea..be3f029ec7 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -28,6 +28,7 @@ use doc::DocDiagnostic; use indexmap::IndexMap; use std::collections::BTreeMap; use std::path::PathBuf; +use std::rc::Rc; use std::sync::Arc; async fn generate_doc_nodes_for_builtin_types( @@ -185,7 +186,11 @@ async fn generate_docs_directory( let output_dir_resolved = cwd.join(&html_options.output); let options = deno_doc::html::GenerateOptions { - package_name: html_options.name, + package_name: Some(html_options.name), + main_entrypoint: None, + global_symbols: Default::default(), + global_symbol_href_resolver: Rc::new(|_, _| String::new()), + url_resolver: Rc::new(deno_doc::html::default_url_resolver), }; let files = deno_doc::html::generate(options, doc_nodes_by_url) diff --git a/cli/util/import_map.rs b/cli/util/import_map.rs index ac5ff24a4a..b4689064db 100644 --- a/cli/util/import_map.rs +++ b/cli/util/import_map.rs @@ -4,10 +4,14 @@ use deno_ast::ParsedSource; use deno_core::error::AnyError; use deno_core::ModuleSpecifier; use deno_graph::DefaultModuleAnalyzer; +use deno_graph::DependencyDescriptor; +use deno_graph::DynamicTemplatePart; use deno_graph::MediaType; use deno_graph::TypeScriptReference; use import_map::ImportMap; +use crate::graph_util::format_range_with_colors; + pub struct ImportMapUnfurler<'a> { import_map: &'a ImportMap, } @@ -59,23 +63,49 @@ impl<'a> ImportMapUnfurler<'a> { })?; let mut text_changes = Vec::new(); let module_info = DefaultModuleAnalyzer::module_info(&parsed_source); - let mut analyze_specifier = - |specifier: &str, range: &deno_graph::PositionRange| { + let analyze_specifier = + |specifier: &str, + range: &deno_graph::PositionRange, + text_changes: &mut Vec| { let resolved = self.import_map.resolve(specifier, url); if let Ok(resolved) = resolved { - let new_text = if resolved.scheme() == "file" { - format!("./{}", url.make_relative(&resolved).unwrap()) - } else { - resolved.to_string() - }; text_changes.push(deno_ast::TextChange { range: to_range(&parsed_source, range), - new_text, + new_text: make_relative_to(url, &resolved), }); } }; for dep in &module_info.dependencies { - analyze_specifier(&dep.specifier, &dep.specifier_range); + match dep { + DependencyDescriptor::Static(dep) => { + analyze_specifier( + &dep.specifier, + &dep.specifier_range, + &mut text_changes, + ); + } + DependencyDescriptor::Dynamic(dep) => { + let success = try_unfurl_dynamic_dep( + self.import_map, + url, + &parsed_source, + dep, + &mut text_changes, + ); + + if !success { + log::warn!( + "{} Dynamic import was not analyzable and won't use the import map once published.\n at {}", + crate::colors::yellow("Warning"), + format_range_with_colors(&deno_graph::Range { + specifier: url.clone(), + start: dep.range.start.clone(), + end: dep.range.end.clone(), + }) + ); + } + } + } } for ts_ref in &module_info.ts_references { let specifier_with_range = match ts_ref { @@ -85,18 +115,21 @@ impl<'a> ImportMapUnfurler<'a> { analyze_specifier( &specifier_with_range.text, &specifier_with_range.range, + &mut text_changes, ); } for specifier_with_range in &module_info.jsdoc_imports { analyze_specifier( &specifier_with_range.text, &specifier_with_range.range, + &mut text_changes, ); } if let Some(specifier_with_range) = &module_info.jsx_import_source { analyze_specifier( &specifier_with_range.text, &specifier_with_range.range, + &mut text_changes, ); } Ok( @@ -120,6 +153,81 @@ impl<'a> ImportMapUnfurler<'a> { } } +fn make_relative_to(from: &ModuleSpecifier, to: &ModuleSpecifier) -> String { + if to.scheme() == "file" { + format!("./{}", from.make_relative(to).unwrap()) + } else { + to.to_string() + } +} + +/// Attempts to unfurl the dynamic dependency returning `true` on success +/// or `false` when the import was not analyzable. +fn try_unfurl_dynamic_dep( + import_map: &ImportMap, + module_url: &lsp_types::Url, + parsed_source: &ParsedSource, + dep: &deno_graph::DynamicDependencyDescriptor, + text_changes: &mut Vec, +) -> bool { + match &dep.argument { + deno_graph::DynamicArgument::String(value) => { + let range = to_range(parsed_source, &dep.argument_range); + let maybe_relative_index = + parsed_source.text_info().text_str()[range.start..].find(value); + let Some(relative_index) = maybe_relative_index else { + return false; + }; + let resolved = import_map.resolve(value, module_url); + let Ok(resolved) = resolved else { + return false; + }; + let start = range.start + relative_index; + text_changes.push(deno_ast::TextChange { + range: start..start + value.len(), + new_text: make_relative_to(module_url, &resolved), + }); + true + } + deno_graph::DynamicArgument::Template(parts) => match parts.first() { + Some(DynamicTemplatePart::String { value }) => { + // relative doesn't need to be modified + let is_relative = value.starts_with("./") || value.starts_with("../"); + if is_relative { + return true; + } + if !value.ends_with('/') { + return false; + } + let Ok(resolved) = import_map.resolve(value, module_url) else { + return false; + }; + let range = to_range(parsed_source, &dep.argument_range); + let maybe_relative_index = + parsed_source.text_info().text_str()[range.start..].find(value); + let Some(relative_index) = maybe_relative_index else { + return false; + }; + let start = range.start + relative_index; + text_changes.push(deno_ast::TextChange { + range: start..start + value.len(), + new_text: make_relative_to(module_url, &resolved), + }); + true + } + Some(DynamicTemplatePart::Expr) => { + false // failed analyzing + } + None => { + true // ignore + } + }, + deno_graph::DynamicArgument::Expr => { + false // failed analyzing + } + } +} + fn to_range( parsed_source: &ParsedSource, range: &deno_graph::PositionRange, @@ -166,6 +274,14 @@ mod tests { import foo from "lib/foo.ts"; import bar from "lib/bar.ts"; import fizz from "fizz"; + +const test1 = await import("lib/foo.ts"); +const test2 = await import(`lib/foo.ts`); +const test3 = await import(`lib/${expr}`); +const test4 = await import(`./lib/${expr}`); +// will warn +const test5 = await import(`lib${expr}`); +const test6 = await import(`${expr}`); "#; let specifier = ModuleSpecifier::parse("file:///dev/mod.ts").unwrap(); let unfurled_source = unfurler @@ -175,6 +291,14 @@ import fizz from "fizz"; import foo from "./lib/foo.ts"; import bar from "./lib/bar.ts"; import fizz from "./fizz/mod.ts"; + +const test1 = await import("./lib/foo.ts"); +const test2 = await import(`./lib/foo.ts`); +const test3 = await import(`./lib/${expr}`); +const test4 = await import(`./lib/${expr}`); +// will warn +const test5 = await import(`lib${expr}`); +const test6 = await import(`${expr}`); "#; assert_eq!(unfurled_source, expected_source); }