1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00

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);
```
This commit is contained in:
David Sherret 2023-12-01 15:12:10 -05:00 committed by GitHub
parent d8e8497eb3
commit a1d823e27d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 327 additions and 42 deletions

21
Cargo.lock generated
View file

@ -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",

View file

@ -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

View file

@ -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(),

View file

@ -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<CliOptions>,
fs: Arc<dyn FileSystem>,
resolver: Arc<CliGraphResolver>,
npm_resolver: Arc<dyn CliNpmResolver>,
module_info_cache: Arc<ModuleInfoCache>,
@ -210,6 +208,7 @@ impl ModuleGraphBuilder {
#[allow(clippy::too_many_arguments)]
pub fn new(
options: Arc<CliOptions>,
fs: Arc<dyn FileSystem>,
resolver: Arc<CliGraphResolver>,
npm_resolver: Arc<dyn CliNpmResolver>,
module_info_cache: Arc<ModuleInfoCache>,
@ -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<deno_graph::source::DirEntry> {
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;

View file

@ -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,
deno_graph::ParseModuleFromAstOptions {
graph_kind: deno_graph::GraphKind::All,
specifier,
maybe_headers,
parsed_source,
Some(resolver),
Some(npm_resolver),
// 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()),

View file

@ -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<CliOptions>,
fs: Arc<dyn deno_fs::FileSystem>,
graph_container: Arc<ModuleGraphContainer>,
lockfile: Option<Arc<Mutex<Lockfile>>>,
maybe_file_watcher_reporter: Option<FileWatcherReporter>,
@ -79,6 +81,7 @@ impl ModuleLoadPreparer {
#[allow(clippy::too_many_arguments)]
pub fn new(
options: Arc<CliOptions>,
fs: Arc<dyn deno_fs::FileSystem>,
graph_container: Arc<ModuleGraphContainer>,
lockfile: Option<Arc<Mutex<Lockfile>>>,
maybe_file_watcher_reporter: Option<FileWatcherReporter>,
@ -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),

View file

@ -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");
}

View file

@ -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());
}

View file

@ -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,
});

View file

@ -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])

View file

@ -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);
}

View file

@ -0,0 +1,3 @@
{
"data": 5
}

View file

@ -0,0 +1,3 @@
{
"data": 1
}

View file

@ -0,0 +1 @@
console.log("a");

View file

@ -0,0 +1 @@
console.log("b");

View file

@ -1 +1 @@
Written 7 files to "./docs/"
Written 9 files to "./docs/"

View file

@ -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",

View file

@ -7,6 +7,7 @@
"/mod.ts": {
"dependencies": [{
"kind": "import",
"type": "static",
"range": [[0, 0], [0, 35]],
"specifier": "./other.ts",
"specifierRange": [[0, 22], [0, 34]]

View file

@ -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)

View file

@ -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<deno_ast::TextChange>| {
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<deno_ast::TextChange>,
) -> 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);
}