From 29883b3d8e1f919e713ed69d2ea5465417e9cf1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sun, 10 Oct 2021 14:09:25 +0200 Subject: [PATCH 01/40] temp --- cli/main.rs | 1 + cli/node_module_loader.rs | 127 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 cli/node_module_loader.rs diff --git a/cli/main.rs b/cli/main.rs index 4ffa8340c70abc..6f6c14e7230e02 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -24,6 +24,7 @@ mod logger; mod lsp; mod module_graph; mod module_loader; +mod node_module_loader; mod ops; mod proc_state; mod source_maps; diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs new file mode 100644 index 00000000000000..e00dbfa5936499 --- /dev/null +++ b/cli/node_module_loader.rs @@ -0,0 +1,127 @@ +// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. + +use crate::module_graph::TypeLib; +use crate::proc_state::ProcState; +use deno_core::error::AnyError; +use deno_core::futures::future::FutureExt; +use deno_core::futures::Future; +use deno_core::ModuleLoadId; +use deno_core::ModuleLoader; +use deno_core::ModuleSpecifier; +use deno_core::OpState; +use deno_runtime::permissions::Permissions; +use import_map::ImportMap; +use std::cell::RefCell; +use std::pin::Pin; +use std::rc::Rc; +use std::str; + +/// This function is an implementation of `defaultResolve` in +/// `lib/internal/modules/esm/resolve.js` from Node. +fn node_resolve( + &self, + specifier: &str, + referrer: &str, + is_main: bool, +) -> Result { + // TODO(bartlomieju): shipped "policy" part + + if let Ok(url) = Url::parse(specifier) { + if url.scheme() == "data:" { + return Ok(url); + } + + let protocol = url.protocol(); + + if protocol == "node:" { + return Ok(url); + } + + if protocol != "file:" && protocol != "data:" { + return Err(generic_error(format!("Only file and data URLs are supported by the default ESM loader. Received protocol '{}'", protocol))); + } + + // In Deno there's no way to expose internal Node modules anyway, + // so calls to NativeModule.canBeRequiredByUsers would only work for built-in modules. + + if referrer.starts_with("data:") { + return Url::parse(specifier, referrer).map_err(AnyError::from); + } + + let referrer = if is_main { + // path_to_file_url() + referrer + } else { + referrer + }; + + let url = module_resolve(specifier, referrer)?; + + // TODO: check codes + + Ok(url) + } + + Ok(module_specifier) +} + +fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { + if specifier == "" { + return false; + } + + if specifier[0] == "/" { + return true; + } + + is_relative_specifier(specifier) +} + +fn is_relative_specifier(specifier: &str) -> bool { + if specifier[0] == "." { + if specifier.len() == 1 || specifier[1] == "/" { + return true; + } + if specifier[1] == "." { + if specifier.len() == 2 || specifier[2] == "/" { + return true; + } + } + } + false +} + +fn module_resolve(specifier: &str, base: &str) { + let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { + Url::parse(specifier, base) + // TODO(bartlomieju): check len, can panic + } else if specifier[0] == "#" { + package_imports_resolve(specifier, base) + } else { + if let Ok(resolved) = Url::parse(specifier) { + resolved + } else { + package_resolve(specifier, base) + } + }; + finalize_resolution(resolved, base) +} + +fn finalize_resolution(resolved: &str, base: &str) { + todo!() +} + +fn package_imports_resolve(specifier: &str, base: &str) { + todo!() +} + +fn package_resolve(specifier: &str, base: &str) -> Result { + let (package_name, package_subpath, is_scoped) = parse_package_name(specifier, base); + + todo!() +} + +fn parse_package_name(specifier: &str, base: &str) -> (&str, &str, &str) { + todo!() + } + From 9200bb2a744c56711bb46e20eebffb6233cbd789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 12 Oct 2021 00:53:54 +0200 Subject: [PATCH 02/40] parse_package_name --- cli/node_module_loader.rs | 207 +++++++++++++++++++++++--------------- 1 file changed, 127 insertions(+), 80 deletions(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index e00dbfa5936499..9f7ca342d3b626 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -1,76 +1,66 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. -use crate::module_graph::TypeLib; -use crate::proc_state::ProcState; +use deno_core::error::generic_error; use deno_core::error::AnyError; -use deno_core::futures::future::FutureExt; -use deno_core::futures::Future; -use deno_core::ModuleLoadId; -use deno_core::ModuleLoader; +use deno_core::url::Url; use deno_core::ModuleSpecifier; -use deno_core::OpState; -use deno_runtime::permissions::Permissions; -use import_map::ImportMap; -use std::cell::RefCell; -use std::pin::Pin; -use std::rc::Rc; -use std::str; /// This function is an implementation of `defaultResolve` in /// `lib/internal/modules/esm/resolve.js` from Node. -fn node_resolve( - &self, - specifier: &str, - referrer: &str, - is_main: bool, -) -> Result { - // TODO(bartlomieju): shipped "policy" part - - if let Ok(url) = Url::parse(specifier) { - if url.scheme() == "data:" { - return Ok(url); - } +// fn node_resolve( +// specifier: &str, +// referrer: &str, +// is_main: bool, +// ) -> Result { +// // TODO(bartlomieju): shipped "policy" part - let protocol = url.protocol(); +// if let Ok(url) = Url::parse(specifier) { +// if url.scheme() == "data:" { +// return Ok(url); +// } - if protocol == "node:" { - return Ok(url); - } +// let protocol = url.scheme(); - if protocol != "file:" && protocol != "data:" { - return Err(generic_error(format!("Only file and data URLs are supported by the default ESM loader. Received protocol '{}'", protocol))); - } +// if protocol == "node" { +// return Ok(url); +// } - // In Deno there's no way to expose internal Node modules anyway, - // so calls to NativeModule.canBeRequiredByUsers would only work for built-in modules. +// if protocol != "file" && protocol != "data" { +// return Err(generic_error(format!("Only file and data URLs are supported by the default ESM loader. Received protocol '{}'", protocol))); +// } - if referrer.starts_with("data:") { - return Url::parse(specifier, referrer).map_err(AnyError::from); - } +// // In Deno there's no way to expose internal Node modules anyway, +// // so calls to NativeModule.canBeRequiredByUsers would only work for built-in modules. - let referrer = if is_main { - // path_to_file_url() - referrer - } else { - referrer - }; +// if referrer.starts_with("data:") { +// let referrer_url = Url::parse(referrer)?; +// return referrer_url.join(specifier).map_err(AnyError::from); +// } - let url = module_resolve(specifier, referrer)?; +// let referrer = if is_main { +// // path_to_file_url() +// referrer +// } else { +// referrer +// }; - // TODO: check codes +// let url = module_resolve(specifier, referrer)?; - Ok(url) - } +// // TODO: check codes - Ok(module_specifier) -} +// Ok(url) +// } + +// // Ok(module_specifier) +// todo!() +// } fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { if specifier == "" { return false; } - if specifier[0] == "/" { + if specifier.chars().nth(0) == Some('/') { return true; } @@ -78,12 +68,15 @@ fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { } fn is_relative_specifier(specifier: &str) -> bool { - if specifier[0] == "." { - if specifier.len() == 1 || specifier[1] == "/" { + let specifier_len = specifier.len(); + let mut specifier_chars = specifier.chars(); + + if specifier_chars.nth(0) == Some('.') { + if specifier_len == 1 || specifier_chars.nth(1) == Some('/') { return true; } - if specifier[1] == "." { - if specifier.len() == 2 || specifier[2] == "/" { + if specifier_chars.nth(1) == Some('.') { + if specifier_len == 2 || specifier_chars.nth(2) == Some('/') { return true; } } @@ -91,37 +84,91 @@ fn is_relative_specifier(specifier: &str) -> bool { false } -fn module_resolve(specifier: &str, base: &str) { - let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { - Url::parse(specifier, base) - // TODO(bartlomieju): check len, can panic - } else if specifier[0] == "#" { - package_imports_resolve(specifier, base) +// fn module_resolve( +// specifier: &str, +// base: &str, +// ) -> Result { +// let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { +// Url::parse(specifier, base) +// // TODO(bartlomieju): check len, can panic +// } else if specifier[0] == "#" { +// package_imports_resolve(specifier, base) +// } else { +// if let Ok(resolved) = Url::parse(specifier) { +// resolved +// } else { +// package_resolve(specifier, base) +// } +// }; +// finalize_resolution(resolved, base) +// } + +// fn finalize_resolution(resolved: &str, base: &str) { +// todo!() +// } + +// fn package_imports_resolve(specifier: &str, base: &str) { +// todo!() +// } + +// fn package_resolve( +// specifier: &str, +// base: &str, +// ) -> Result { +// let (package_name, package_subpath, is_scoped) = +// parse_package_name(specifier, base); + +// todo!() +// } + +fn parse_package_name( + specifier: &str, + base: &str, +) -> Result<(String, String, bool), AnyError> { + let mut separator_index = specifier.find('/'); + let mut valid_package_name = false; + let mut is_scoped = false; + if specifier.is_empty() { + valid_package_name = false; } else { - if let Ok(resolved) = Url::parse(specifier) { - resolved - } else { - package_resolve(specifier, base) + if specifier.chars().nth(0) == Some('@') { + is_scoped = true; + if let Some(index) = separator_index { + separator_index = specifier[index + 1..].find('/'); + } else { + valid_package_name = false; + } } + } + + let package_name = if let Some(index) = separator_index { + specifier[0..index].to_string() + } else { + specifier.to_string() }; - finalize_resolution(resolved, base) -} -fn finalize_resolution(resolved: &str, base: &str) { - todo!() -} + // Package name cannot have leading . and cannot have percent-encoding or separators. + for ch in package_name.chars() { + if ch == '%' || ch == '\\' { + valid_package_name = false; + break; + } + } -fn package_imports_resolve(specifier: &str, base: &str) { - todo!() -} + if !valid_package_name { + // TODO(bartlomieju): apply fileURLToPath + return Err(generic_error(format!( + "{} is not a valid package name {}", + specifier, base + ))); + } -fn package_resolve(specifier: &str, base: &str) -> Result { - let (package_name, package_subpath, is_scoped) = parse_package_name(specifier, base); + let package_subpath = if let Some(index) = separator_index { + format!(".{}", specifier.chars().skip(index).collect::()) + .to_string() + } else { + ".".to_string() + }; - todo!() + Ok((package_name, package_subpath, is_scoped)) } - -fn parse_package_name(specifier: &str, base: &str) -> (&str, &str, &str) { - todo!() - } - From becf7499d2438bdca52fb60d95997724237139e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 12 Oct 2021 03:06:30 +0200 Subject: [PATCH 03/40] module_resolve --- cli/node_module_loader.rs | 140 ++++++++++++++++++++++++++++---------- 1 file changed, 105 insertions(+), 35 deletions(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index 9f7ca342d3b626..92130fe4700b36 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -4,6 +4,7 @@ use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::url::Url; use deno_core::ModuleSpecifier; +use regex::Regex; /// This function is an implementation of `defaultResolve` in /// `lib/internal/modules/esm/resolve.js` from Node. @@ -84,46 +85,94 @@ fn is_relative_specifier(specifier: &str) -> bool { false } -// fn module_resolve( -// specifier: &str, -// base: &str, -// ) -> Result { -// let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { -// Url::parse(specifier, base) -// // TODO(bartlomieju): check len, can panic -// } else if specifier[0] == "#" { -// package_imports_resolve(specifier, base) -// } else { -// if let Ok(resolved) = Url::parse(specifier) { -// resolved -// } else { -// package_resolve(specifier, base) -// } -// }; -// finalize_resolution(resolved, base) -// } +fn module_resolve( + specifier: &str, + base: &ModuleSpecifier, +) -> Result { + let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { + base.join(specifier)? + } else if specifier.chars().nth(0) == Some('#') { + package_imports_resolve(specifier, base)? + } else { + if let Ok(resolved) = Url::parse(specifier) { + resolved + } else { + package_resolve(specifier, base)? + } + }; + finalize_resolution(resolved, base) +} -// fn finalize_resolution(resolved: &str, base: &str) { -// todo!() -// } +fn finalize_resolution( + resolved: ModuleSpecifier, + base: &ModuleSpecifier, +) -> Result { + let encoded_sep_re = Regex::new(r"%2F|%2C").expect("bad regex"); -// fn package_imports_resolve(specifier: &str, base: &str) { -// todo!() -// } + if encoded_sep_re.is_match(resolved.path()) { + return Err(generic_error(format!( + "{} must not include encoded \"/\" or \"\\\\\" characters {}", + resolved.path(), + base.to_file_path().unwrap().display() + ))); + } -// fn package_resolve( -// specifier: &str, -// base: &str, -// ) -> Result { -// let (package_name, package_subpath, is_scoped) = -// parse_package_name(specifier, base); + let path = resolved.to_file_path().unwrap(); -// todo!() -// } + // TODO(bartlomieju): currently not supported + // if (getOptionValue('--experimental-specifier-resolution') === 'node') { + // ... + // } + + let p_str = path.to_str().unwrap(); + let p = if p_str.ends_with('/') { + p_str[p_str.len() - 1..].to_string() + } else { + p_str.to_string() + }; + + let stats = std::fs::metadata(&p)?; + if stats.is_dir() { + return Err( + generic_error( + format!("Directory import {} is not supported resolving ES modules imported from {}", + path.display(), base.to_file_path().unwrap().display() + ) + )); + } else if !stats.is_file() { + return Err(generic_error(format!( + "Cannot find module {} imported from {}", + path.display(), + base.to_file_path().unwrap().display() + ))); + } + + Ok(resolved) +} + +fn package_imports_resolve( + specifier: &str, + base: &ModuleSpecifier, +) -> Result { + todo!() +} + +fn package_resolve( + specifier: &str, + base: &ModuleSpecifier, +) -> Result { + let (package_name, package_subpath, is_scoped) = + parse_package_name(specifier, base)?; + + // ResolveSelf + // let package_config = get_package_scope_config(base); + + todo!() +} fn parse_package_name( specifier: &str, - base: &str, + base: &ModuleSpecifier, ) -> Result<(String, String, bool), AnyError> { let mut separator_index = specifier.find('/'); let mut valid_package_name = false; @@ -156,10 +205,10 @@ fn parse_package_name( } if !valid_package_name { - // TODO(bartlomieju): apply fileURLToPath return Err(generic_error(format!( "{} is not a valid package name {}", - specifier, base + specifier, + base.to_file_path().unwrap().display() ))); } @@ -172,3 +221,24 @@ fn parse_package_name( Ok((package_name, package_subpath, is_scoped)) } + +// enum ExportConfig { +// Str(String), +// StrArray(Vec), +// } + +// enum PackageType { +// Module, +// CommonJs, +// } + +// struct PackageConfig { +// exports: Option, +// name: Option, +// main: Option, +// typ: Option +// } + +// fn get_package_scope_config(resolved: &str) { +// todo!() +// } From 96a84d3a7ad2ed3f30a5721b56f1d1fc7619d1df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 12 Oct 2021 04:06:24 +0200 Subject: [PATCH 04/40] package_exports_resolve --- cli/node_module_loader.rs | 273 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 263 insertions(+), 10 deletions(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index 92130fe4700b36..657814bf7dbfa4 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -2,9 +2,14 @@ use deno_core::error::generic_error; use deno_core::error::AnyError; +use deno_core::serde_json; +use deno_core::serde_json::Map; +use deno_core::serde_json::Value; use deno_core::url::Url; use deno_core::ModuleSpecifier; use regex::Regex; +use std::path::Path; +use std::path::PathBuf; /// This function is an implementation of `defaultResolve` in /// `lib/internal/modules/esm/resolve.js` from Node. @@ -157,6 +162,83 @@ fn package_imports_resolve( todo!() } +fn is_conditional_exports_main_sugar( + exports: &Value, + package_json_url: &Url, + base: &ModuleSpecifier, +) -> Result { + if exports.is_string() || exports.is_array() { + return Ok(true); + } + + if exports.is_null() || !exports.is_object() { + return Ok(false); + } + + let exports_obj = exports.as_object().unwrap(); + let mut is_conditional_sugar = false; + let mut i = 0; + for key in exports_obj.keys() { + let cur_is_conditional_sugar = key == "" || !key.starts_with('.'); + if i == 0 { + is_conditional_sugar = cur_is_conditional_sugar; + i += 1; + } else if is_conditional_sugar != cur_is_conditional_sugar { + let msg = format!( + "Invalid package config {} while importing {}. + \"exports\" cannot contains some keys starting with \'.\' and some not. + The exports object must either be an object of package subpath keys + or an object of main entry condition name keys only.", + package_json_url.to_file_path().unwrap().display(), + base.as_str() + ); + return Err(generic_error(msg)); + } + } + + Ok(is_conditional_sugar) +} + +// TODO(bartlomieju): last argument "conditions" was skipped +fn package_exports_resolve( + package_json_url: Url, + package_subpath: String, + package_config: PackageConfig, + base: &ModuleSpecifier, +) -> Result { + let exports = &package_config.exports; + + let exports_map = + if is_conditional_exports_main_sugar(exports, &package_json_url, base) { + let mut map = Map::new(); + map.insert(".".to_string(), exports.to_owned()); + map + } else { + exports.as_object().unwrap().to_owned() + }; + + if exports_map.contains_key(&package_subpath) + && package_subpath.find('*').is_none() + && !package_subpath.ends_with('/') + { + let target = exports_map.get(&package_subpath).unwrap(); + // TODO(bartlomieju): last argument "conditions" was skipped + let resolved = resolve_package_target( + package_json_url, + target, + "", + package_subpath, + base, + false, + false, + )?; + // TODO() + return Ok(resolved); + } + + todo!() +} + fn package_resolve( specifier: &str, base: &ModuleSpecifier, @@ -165,7 +247,24 @@ fn package_resolve( parse_package_name(specifier, base)?; // ResolveSelf - // let package_config = get_package_scope_config(base); + let package_config = get_package_scope_config(base)?; + if package_config.exists { + let package_json_url = + Url::from_file_path(&package_config.pjsonpath).unwrap(); + if package_config.name == Some(package_name) { + if let Some(exports) = &package_config.exports { + if !exports.is_null() { + // TODO(bartlomieju): last argument "conditions" was skipped + return package_exports_resolve( + package_json_url, + package_subpath, + package_config, + base, + ); + } + } + } + } todo!() } @@ -232,13 +331,167 @@ fn parse_package_name( // CommonJs, // } -// struct PackageConfig { -// exports: Option, -// name: Option, -// main: Option, -// typ: Option -// } +#[derive(Clone, Debug)] +struct PackageConfig { + exists: bool, + exports: Option, + imports: Option>, + main: Option, + name: Option, + pjsonpath: PathBuf, + typ: String, +} -// fn get_package_scope_config(resolved: &str) { -// todo!() -// } +fn get_package_config( + path: PathBuf, + specifier: &ModuleSpecifier, + maybe_base: Option<&ModuleSpecifier>, +) -> Result { + // TODO(bartlomieju): + // if let Some(existing) = package_json_cache.get(path) { + // return Ok(existing.clone()); + // } + + // TODO: maybe shouldn't error be return empty package + let source = std::fs::read_to_string(&path)?; + if source.is_empty() { + let package_config = PackageConfig { + pjsonpath: path, + exists: false, + main: None, + name: None, + typ: "none".to_string(), + exports: None, + imports: None, + }; + // TODO(bartlomieju): + // package_json_cache.set(package_json_path, package_config.clone()); + return Ok(package_config); + } + + let package_json: Value = serde_json::from_str(&source).map_err(|_err| { + let mut msg = format!("Invalid package config {}", path.display()); + + if let Some(base) = maybe_base { + msg = format!( + "{} \"{}\" from {}", + msg, + specifier.as_str(), + base.to_file_path().unwrap().display() + ); + } + + generic_error(msg) + })?; + + let imports_val = package_json.get("imports"); + let main_val = package_json.get("main"); + let name_val = package_json.get("name"); + let typ_val = package_json.get("type"); + let exports = package_json.get("exports").map(|e| e.to_owned()); + + // TODO(bartlomieju): refactor + let imports = if let Some(imp) = imports_val { + if let Some(imp) = imp.as_object() { + Some(imp.to_owned()) + } else { + None + } + } else { + None + }; + let main = if let Some(m) = main_val { + if let Some(m) = m.as_str() { + Some(m.to_string()) + } else { + None + } + } else { + None + }; + let name = if let Some(n) = name_val { + if let Some(n) = n.as_str() { + Some(n.to_string()) + } else { + None + } + } else { + None + }; + + // Ignore unknown types for forwards compatibility + let typ = if let Some(t) = typ_val { + if let Some(t) = t.as_str() { + if t != "module" && t != "commonjs" { + "none".to_string() + } else { + t.to_string() + } + } else { + "none".to_string() + } + } else { + "none".to_string() + }; + + let package_config = PackageConfig { + pjsonpath: path, + exists: false, + main, + name, + typ, + exports, + imports, + }; + // TODO(bartlomieju): + // package_json_cache.set(package_json_path, package_config.clone()); + Ok(package_config) +} + +fn get_package_scope_config( + resolved: &ModuleSpecifier, +) -> Result { + let mut package_json_url = resolved.join("./package.json")?; + + loop { + let package_json_path = package_json_url.path(); + + if package_json_path.ends_with("node_modules/package.json") { + break; + } + + let package_config = get_package_config( + package_json_url.to_file_path().unwrap(), + resolved, + None, + )?; + if package_config.exists { + return Ok(package_config); + } + + let last_package_json_url = package_json_url.clone(); + package_json_url = package_json_url.join("../package.json")?; + + // Terminates at root where ../package.json equals ../../package.json + // (can't just check "/package.json" for Windows support) + if package_json_url.path() == last_package_json_url.path() { + break; + } + } + + let package_json_path = package_json_url.to_file_path().unwrap(); + let package_config = PackageConfig { + pjsonpath: package_json_path, + exists: false, + main: None, + name: None, + typ: "none".to_string(), + exports: None, + imports: None, + }; + + // TODO(bartlomieju): + // package_json_cache.set(package_json_path, package_config.clone()); + + Ok(package_config) +} From 371ffccecba048409c1ba709b8d1011e9ce8bacd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 12 Oct 2021 04:27:28 +0200 Subject: [PATCH 05/40] resolve_package_target_string --- cli/node_module_loader.rs | 100 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 96 insertions(+), 4 deletions(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index 657814bf7dbfa4..98c2dfe86cb745 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -199,6 +199,98 @@ fn is_conditional_exports_main_sugar( Ok(is_conditional_sugar) } +// TODO(bartlomieju): last argument "conditions" was skipped +fn resolve_package_target_string( + target: String, + subpath: String, + match_: String, + package_json_url: Url, + base: &ModuleSpecifier, + pattern: bool, + internal: bool, +) -> Result { + if subpath == "" && !pattern && !target.ends_with('/') { + todo!() + } + + if !target.starts_with("./") { + if internal && !target.starts_with("../") && !target.starts_with('/') { + todo!() + } + todo!() + } + + let invalid_segment_re = + Regex::new(r"(^|\|/)(..?|node_modules)(\|/|$)").expect("bad regex"); + let pattern_re = Regex::new(r"*").expect("bad regex"); + + if invalid_segment_re.is_match(&target[2..]) { + todo!() + } + + let resolved = package_json_url.join(&target)?; + let resolved_path = resolved.path(); + let package_url = package_json_url.join(".").unwrap(); + let package_path = package_url.path(); + + if !resolved_path.starts_with(package_path) { + todo!() + } + + if subpath == "" { + return Ok(resolved); + } + + if invalid_segment_re.is_match(&subpath) { + todo!() + } + + if pattern { + let replaced = pattern_re + .replace(resolved.as_str(), |_caps: ®ex::Captures| subpath.clone()); + let url = Url::parse(&replaced)?; + return Ok(url); + } + + Ok(resolved.join(&subpath)?) +} + +// TODO(bartlomieju): last argument "conditions" was skipped +fn resolve_package_target( + package_json_url: Url, + target: Value, + subpath: String, + package_subpath: String, + base: &ModuleSpecifier, + pattern: bool, + internal: bool, +) -> Result { + if let Some(target) = target.as_str() { + return resolve_package_target_string( + target.to_string(), + subpath, + package_subpath, + package_json_url, + base, + pattern, + internal, + // TODO(bartlomieju): last argument "conditions" was skipped + ); + } else if let Some(target_arr) = target.as_array() { + if target_arr.is_empty() { + todo!() + } + + todo!() + } else if let Some(target_obj) = target.as_object() { + todo!() + } else if target.is_null() { + todo!() + } + + todo!() +} + // TODO(bartlomieju): last argument "conditions" was skipped fn package_exports_resolve( package_json_url: Url, @@ -206,10 +298,10 @@ fn package_exports_resolve( package_config: PackageConfig, base: &ModuleSpecifier, ) -> Result { - let exports = &package_config.exports; + let exports = &package_config.exports.unwrap(); let exports_map = - if is_conditional_exports_main_sugar(exports, &package_json_url, base) { + if is_conditional_exports_main_sugar(exports, &package_json_url, base)? { let mut map = Map::new(); map.insert(".".to_string(), exports.to_owned()); map @@ -221,12 +313,12 @@ fn package_exports_resolve( && package_subpath.find('*').is_none() && !package_subpath.ends_with('/') { - let target = exports_map.get(&package_subpath).unwrap(); + let target = exports_map.get(&package_subpath).unwrap().to_owned(); // TODO(bartlomieju): last argument "conditions" was skipped let resolved = resolve_package_target( package_json_url, target, - "", + "".to_string(), package_subpath, base, false, From dfd1190099d5ff47cc8ed9440257171c068c6222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 12 Oct 2021 21:44:14 +0200 Subject: [PATCH 06/40] node_resolve --- cli/node_module_loader.rs | 74 ++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index 98c2dfe86cb745..612ad94a869e51 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -8,58 +8,54 @@ use deno_core::serde_json::Value; use deno_core::url::Url; use deno_core::ModuleSpecifier; use regex::Regex; -use std::path::Path; use std::path::PathBuf; /// This function is an implementation of `defaultResolve` in /// `lib/internal/modules/esm/resolve.js` from Node. -// fn node_resolve( -// specifier: &str, -// referrer: &str, -// is_main: bool, -// ) -> Result { -// // TODO(bartlomieju): shipped "policy" part - -// if let Ok(url) = Url::parse(specifier) { -// if url.scheme() == "data:" { -// return Ok(url); -// } - -// let protocol = url.scheme(); +fn node_resolve( + specifier: &str, + referrer: &str, +) -> Result { + // TODO(bartlomieju): skipped "policy" part -// if protocol == "node" { -// return Ok(url); -// } + if let Ok(url) = Url::parse(specifier) { + if url.scheme() == "data:" { + return Ok(url); + } -// if protocol != "file" && protocol != "data" { -// return Err(generic_error(format!("Only file and data URLs are supported by the default ESM loader. Received protocol '{}'", protocol))); -// } + let protocol = url.scheme(); -// // In Deno there's no way to expose internal Node modules anyway, -// // so calls to NativeModule.canBeRequiredByUsers would only work for built-in modules. + if protocol == "node" { + return Ok(url); + } -// if referrer.starts_with("data:") { -// let referrer_url = Url::parse(referrer)?; -// return referrer_url.join(specifier).map_err(AnyError::from); -// } + if protocol != "file" && protocol != "data" { + return Err(generic_error(format!("Only file and data URLs are supported by the default ESM loader. Received protocol '{}'", protocol))); + } -// let referrer = if is_main { -// // path_to_file_url() -// referrer -// } else { -// referrer -// }; + // In Deno there's no way to expose internal Node modules anyway, + // so calls to NativeModule.canBeRequiredByUsers would only work for built-in modules. -// let url = module_resolve(specifier, referrer)?; + if referrer.starts_with("data:") { + let referrer_url = Url::parse(referrer)?; + return referrer_url.join(specifier).map_err(AnyError::from); + } + } -// // TODO: check codes + let is_main = referrer.is_empty(); + let parent_url = if is_main { + let cwd = std::env::current_dir()?; + Url::from_directory_path(cwd).unwrap() + } else { + Url::parse(referrer).expect("referrer was not proper url") + }; -// Ok(url) -// } + let url = module_resolve(specifier, &parent_url)?; -// // Ok(module_specifier) -// todo!() -// } + // TODO(bartlomieju): skipped checking errors for commonJS resolution and + // "preserveSymlinksMain"/"preserveSymlinks" options. + Ok(url) +} fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { if specifier == "" { From 6954cf6b514ca3eb71c68857d872c70785c3f6f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 00:04:45 +0200 Subject: [PATCH 07/40] chalk example working --- cli/compat.rs | 4 +- cli/node_module_loader.rs | 196 +++++++++++++++++++++++++++++++------- cli/proc_state.rs | 11 ++- 3 files changed, 175 insertions(+), 36 deletions(-) diff --git a/cli/compat.rs b/cli/compat.rs index f4cc2a08d49943..99e3c127a06ac0 100644 --- a/cli/compat.rs +++ b/cli/compat.rs @@ -7,10 +7,10 @@ use std::collections::HashMap; // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded -static STD_URL: &str = "https://deno.land/std@0.111.0/"; +pub(crate) static STD_URL: &str = "https://deno.land/std@0.111.0/"; static GLOBAL_MODULE: &str = "global.ts"; -static SUPPORTED_MODULES: &[&str] = &[ +pub(crate) static SUPPORTED_MODULES: &[&str] = &[ "assert", "assert/strict", "async_hooks", diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index 612ad94a869e51..c0fefdd18e64e4 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -7,17 +7,46 @@ use deno_core::serde_json::Map; use deno_core::serde_json::Value; use deno_core::url::Url; use deno_core::ModuleSpecifier; +use deno_graph::source::Resolver; use regex::Regex; use std::path::PathBuf; +#[derive(Debug, Default)] +pub(crate) struct NodeResolver; + +impl NodeResolver { + pub fn as_resolver(&self) -> &dyn Resolver { + self + } +} + +impl Resolver for NodeResolver { + fn resolve( + &self, + specifier: &str, + referrer: &ModuleSpecifier, + ) -> Result { + if referrer.as_str().starts_with("https://deno.land/std") { + return referrer.join(specifier).map_err(AnyError::from); + } + node_resolve(specifier, referrer.as_str()) + } +} + /// This function is an implementation of `defaultResolve` in /// `lib/internal/modules/esm/resolve.js` from Node. fn node_resolve( specifier: &str, referrer: &str, ) -> Result { + // eprintln!("NODE_RESOLVE {} {}", specifier, referrer); // TODO(bartlomieju): skipped "policy" part + if crate::compat::SUPPORTED_MODULES.contains(&specifier) { + let module_url = format!("{}node/{}.ts", crate::compat::STD_URL, specifier); + return Ok(Url::parse(&module_url).unwrap()); + } + if let Ok(url) = Url::parse(specifier) { if url.scheme() == "data:" { return Ok(url); @@ -33,9 +62,6 @@ fn node_resolve( return Err(generic_error(format!("Only file and data URLs are supported by the default ESM loader. Received protocol '{}'", protocol))); } - // In Deno there's no way to expose internal Node modules anyway, - // so calls to NativeModule.canBeRequiredByUsers would only work for built-in modules. - if referrer.starts_with("data:") { let referrer_url = Url::parse(referrer)?; return referrer_url.join(specifier).map_err(AnyError::from); @@ -71,14 +97,14 @@ fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { fn is_relative_specifier(specifier: &str) -> bool { let specifier_len = specifier.len(); - let mut specifier_chars = specifier.chars(); + let specifier_chars: Vec<_> = specifier.chars().collect(); - if specifier_chars.nth(0) == Some('.') { - if specifier_len == 1 || specifier_chars.nth(1) == Some('/') { + if !specifier_chars.is_empty() && specifier_chars[0] == '.' { + if specifier_len == 1 || specifier_chars[1] == '/' { return true; } - if specifier_chars.nth(1) == Some('.') { - if specifier_len == 2 || specifier_chars.nth(2) == Some('/') { + if specifier_chars[1] == '.' { + if specifier_len == 2 || specifier_chars[2] == '/' { return true; } } @@ -90,6 +116,7 @@ fn module_resolve( specifier: &str, base: &ModuleSpecifier, ) -> Result { + // eprintln!("module resolve {} {}", specifier, base.as_str()); let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { base.join(specifier)? } else if specifier.chars().nth(0) == Some('#') { @@ -108,6 +135,11 @@ fn finalize_resolution( resolved: ModuleSpecifier, base: &ModuleSpecifier, ) -> Result { + // TODO(bartlomieju): short circuit for remote modules + if resolved.scheme().starts_with("http") { + return Ok(resolved); + } + let encoded_sep_re = Regex::new(r"%2F|%2C").expect("bad regex"); if encoded_sep_re.is_match(resolved.path()) { @@ -118,6 +150,7 @@ fn finalize_resolution( ))); } + // eprintln!("resolved {}, base: {}", resolved.as_str(), base.as_str()); let path = resolved.to_file_path().unwrap(); // TODO(bartlomieju): currently not supported @@ -132,15 +165,20 @@ fn finalize_resolution( p_str.to_string() }; - let stats = std::fs::metadata(&p)?; - if stats.is_dir() { + // eprintln!("meta data path: {}", p); + let (is_dir, is_file) = if let Ok(stats) = std::fs::metadata(&p) { + (stats.is_dir(), stats.is_file()) + } else { + (false, false) + }; + if is_dir { return Err( generic_error( format!("Directory import {} is not supported resolving ES modules imported from {}", path.display(), base.to_file_path().unwrap().display() ) )); - } else if !stats.is_file() { + } else if !is_file { return Err(generic_error(format!( "Cannot find module {} imported from {}", path.display(), @@ -152,8 +190,8 @@ fn finalize_resolution( } fn package_imports_resolve( - specifier: &str, - base: &ModuleSpecifier, + _specifier: &str, + _base: &ModuleSpecifier, ) -> Result { todo!() } @@ -199,13 +237,13 @@ fn is_conditional_exports_main_sugar( fn resolve_package_target_string( target: String, subpath: String, - match_: String, + _match_: String, package_json_url: Url, - base: &ModuleSpecifier, + _base: &ModuleSpecifier, pattern: bool, internal: bool, ) -> Result { - if subpath == "" && !pattern && !target.ends_with('/') { + if subpath != "" && !pattern && !target.ends_with('/') { todo!() } @@ -218,7 +256,7 @@ fn resolve_package_target_string( let invalid_segment_re = Regex::new(r"(^|\|/)(..?|node_modules)(\|/|$)").expect("bad regex"); - let pattern_re = Regex::new(r"*").expect("bad regex"); + let pattern_re = Regex::new(r"\*").expect("bad regex"); if invalid_segment_re.is_match(&target[2..]) { todo!() @@ -260,9 +298,9 @@ fn resolve_package_target( base: &ModuleSpecifier, pattern: bool, internal: bool, -) -> Result { +) -> Result, AnyError> { if let Some(target) = target.as_str() { - return resolve_package_target_string( + return Ok(Some(resolve_package_target_string( target.to_string(), subpath, package_subpath, @@ -271,7 +309,7 @@ fn resolve_package_target( pattern, internal, // TODO(bartlomieju): last argument "conditions" was skipped - ); + )?)); } else if let Some(target_arr) = target.as_array() { if target_arr.is_empty() { todo!() @@ -279,7 +317,29 @@ fn resolve_package_target( todo!() } else if let Some(target_obj) = target.as_object() { - todo!() + for key in target_obj.keys() { + // TODO(bartlomieju): verify that keys are not numeric + + // TODO(bartlomieju): this should be injected as an + // argument to the function + let conditions = vec!["node".to_string(), "import".to_string()]; + if key == "default" || conditions.contains(&key) { + let condition_target = target_obj.get(key).unwrap().to_owned(); + let resolved = resolve_package_target( + package_json_url.clone(), + condition_target, + subpath.clone(), + package_subpath.clone(), + base, + pattern, + internal, + )?; + if resolved.is_none() { + continue; + } + return Ok(resolved); + } + } } else if target.is_null() { todo!() } @@ -320,8 +380,11 @@ fn package_exports_resolve( false, false, )?; - // TODO() - return Ok(resolved); + // TODO(): + if resolved.is_none() { + todo!() + } + return Ok(resolved.unwrap()); } todo!() @@ -331,6 +394,7 @@ fn package_resolve( specifier: &str, base: &ModuleSpecifier, ) -> Result { + // eprintln!("package_resolve {} {}", specifier, base.as_str()); let (package_name, package_subpath, is_scoped) = parse_package_name(specifier, base)?; @@ -339,7 +403,11 @@ fn package_resolve( if package_config.exists { let package_json_url = Url::from_file_path(&package_config.pjsonpath).unwrap(); - if package_config.name == Some(package_name) { + // eprintln!( + // "package_config.name {:?} package_name {:?} exports {:#?}", + // package_config.name, package_name, package_config.exports + // ); + if package_config.name.as_ref() == Some(&package_name) { if let Some(exports) = &package_config.exports { if !exports.is_null() { // TODO(bartlomieju): last argument "conditions" was skipped @@ -354,7 +422,61 @@ fn package_resolve( } } - todo!() + let mut package_json_url = + base.join(&format!("./node_modules/{}/package.json", package_name))?; + let mut package_json_path = package_json_url.to_file_path().unwrap(); + let mut last_path; + loop { + let p_str = package_json_path.to_str().unwrap(); + let p = p_str[0..=p_str.len() - 13].to_string(); + let is_dir = if let Ok(stats) = std::fs::metadata(&p) { + stats.is_dir() + } else { + false + }; + if !is_dir { + last_path = package_json_path; + + let prefix = if is_scoped { + "../../../../node_modules/" + } else { + "../../../node_modules/" + }; + package_json_url = package_json_url + .join(&format!("{}{}/package.json", prefix, package_name))?; + package_json_path = package_json_url.to_file_path().unwrap(); + if package_json_path.to_str().unwrap().len() + == last_path.to_str().unwrap().len() + { + break; + } else { + continue; + } + } + + // Package match. + // eprintln!("got package match!"); + let package_config = + get_package_config(package_json_path.clone(), specifier, Some(base))?; + // eprintln!("got package match {} {} {:#?}", package_json_path.display(), specifier, package_config.exports); + if package_config.exports.is_some() { + return package_exports_resolve( + package_json_url, + package_subpath, + package_config, + base, + ); + } + if package_subpath == "." { + todo!(); + } + + return package_json_url + .join(&package_subpath) + .map_err(AnyError::from); + } + + Err(generic_error(format!("module not found {}", specifier))) } fn parse_package_name( @@ -362,7 +484,7 @@ fn parse_package_name( base: &ModuleSpecifier, ) -> Result<(String, String, bool), AnyError> { let mut separator_index = specifier.find('/'); - let mut valid_package_name = false; + let mut valid_package_name = true; let mut is_scoped = false; if specifier.is_empty() { valid_package_name = false; @@ -378,7 +500,7 @@ fn parse_package_name( } let package_name = if let Some(index) = separator_index { - specifier[0..index].to_string() + specifier[0..=index].to_string() } else { specifier.to_string() }; @@ -391,6 +513,7 @@ fn parse_package_name( } } + // // eprintln!("specifier: {:?}, base: {:?}", specifier, base); if !valid_package_name { return Err(generic_error(format!( "{} is not a valid package name {}", @@ -432,7 +555,7 @@ struct PackageConfig { fn get_package_config( path: PathBuf, - specifier: &ModuleSpecifier, + specifier: &str, maybe_base: Option<&ModuleSpecifier>, ) -> Result { // TODO(bartlomieju): @@ -441,7 +564,10 @@ fn get_package_config( // } // TODO: maybe shouldn't error be return empty package - let source = std::fs::read_to_string(&path)?; + // eprintln!("get package config: {} {:?}", specifier, path); + let result = std::fs::read_to_string(&path); + + let source = result.unwrap_or_else(|_| "".to_string()); if source.is_empty() { let package_config = PackageConfig { pjsonpath: path, @@ -464,7 +590,7 @@ fn get_package_config( msg = format!( "{} \"{}\" from {}", msg, - specifier.as_str(), + specifier, base.to_file_path().unwrap().display() ); } @@ -524,7 +650,7 @@ fn get_package_config( let package_config = PackageConfig { pjsonpath: path, - exists: false, + exists: true, main, name, typ, @@ -548,11 +674,17 @@ fn get_package_scope_config( break; } + // eprintln!("get package scope config"); let package_config = get_package_config( package_json_url.to_file_path().unwrap(), - resolved, + resolved.as_str(), None, )?; + // eprintln!( + // "found package config {}, exists: {}", + // package_json_url.as_str(), + // package_config.exists + // ); if package_config.exists { return Ok(package_config); } diff --git a/cli/proc_state.rs b/cli/proc_state.rs index fe707754f7062c..346a26a6ff245b 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -13,6 +13,7 @@ use crate::flags; use crate::http_cache; use crate::lockfile::as_maybe_locker; use crate::lockfile::Lockfile; +use crate::node_module_loader::NodeResolver; use crate::resolver::ImportMapResolver; use crate::source_maps::SourceMapGetter; use crate::version; @@ -316,14 +317,20 @@ impl ProcState { ); let maybe_locker = as_maybe_locker(self.lockfile.clone()); let maybe_imports = self.get_maybe_imports(); - let maybe_resolver = + let node_resolver = NodeResolver; + let import_map_resolver = self.maybe_import_map.as_ref().map(ImportMapResolver::new); + let maybe_resolver = if self.flags.compat { + Some(node_resolver.as_resolver()) + } else { + import_map_resolver.as_ref().map(|im| im.as_resolver()) + }; let graph = deno_graph::create_graph( roots, is_dynamic, maybe_imports, &mut cache, - maybe_resolver.as_ref().map(|im| im.as_resolver()), + maybe_resolver, maybe_locker, None, ) From a0b1ba677bd34d0e6b73fcdbf146a26153ef3877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 02:03:25 +0200 Subject: [PATCH 08/40] legacy_main_resolve --- cli/node_module_loader.rs | 56 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index c0fefdd18e64e4..0b9fc198509d8d 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -468,7 +468,7 @@ fn package_resolve( ); } if package_subpath == "." { - todo!(); + return legacy_main_resolve(&package_json_url, &package_config, base); } return package_json_url @@ -715,3 +715,57 @@ fn get_package_scope_config( Ok(package_config) } + +fn file_exists(path_url: &Url) -> bool { + if let Ok(stats) = std::fs::metadata(path_url.to_file_path().unwrap()) { + stats.is_file() + } else { + false + } +} + +fn legacy_main_resolve( + package_json_url: &Url, + package_config: &PackageConfig, + _base: &ModuleSpecifier, +) -> Result { + let mut guess; + + if let Some(main) = &package_config.main { + guess = package_json_url.join(&format!("./{}", main))?; + if file_exists(&guess) { + return Ok(guess); + } + + let mut found = false; + for ext in [ + ".js", + ".json", + ".node", + "/index.js", + "/index.json", + "/index.node", + ] { + guess = package_json_url.join(&format!("./{}{}", main, ext))?; + if file_exists(&guess) { + found = true; + break; + } + } + + if found { + // TODO(bartlomieju): emitLegacyIndexDeprecation() + return Ok(guess); + } + } + + for p in ["./index.js", "./index.json", "./index.node"] { + guess = package_json_url.join(p)?; + if file_exists(&guess) { + // TODO(bartlomieju): emitLegacyIndexDeprecation() + return Ok(guess); + } + } + + Err(generic_error("not found")) +} From 5d413936ee264c8af2cf920002da113e286205e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 07:25:11 +0200 Subject: [PATCH 09/40] pass conditions --- cli/node_module_loader.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index 0b9fc198509d8d..c23be3abde9661 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -33,6 +33,8 @@ impl Resolver for NodeResolver { } } +static DEFAULT_CONDITIONS: &[&str] = &["node", "import"]; + /// This function is an implementation of `defaultResolve` in /// `lib/internal/modules/esm/resolve.js` from Node. fn node_resolve( @@ -76,7 +78,8 @@ fn node_resolve( Url::parse(referrer).expect("referrer was not proper url") }; - let url = module_resolve(specifier, &parent_url)?; + let conditions = DEFAULT_CONDITIONS; + let url = module_resolve(specifier, &parent_url, conditions)?; // TODO(bartlomieju): skipped checking errors for commonJS resolution and // "preserveSymlinksMain"/"preserveSymlinks" options. @@ -115,6 +118,7 @@ fn is_relative_specifier(specifier: &str) -> bool { fn module_resolve( specifier: &str, base: &ModuleSpecifier, + conditions: &[&str], ) -> Result { // eprintln!("module resolve {} {}", specifier, base.as_str()); let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { @@ -125,7 +129,7 @@ fn module_resolve( if let Ok(resolved) = Url::parse(specifier) { resolved } else { - package_resolve(specifier, base)? + package_resolve(specifier, base, conditions)? } }; finalize_resolution(resolved, base) @@ -233,7 +237,6 @@ fn is_conditional_exports_main_sugar( Ok(is_conditional_sugar) } -// TODO(bartlomieju): last argument "conditions" was skipped fn resolve_package_target_string( target: String, subpath: String, @@ -242,6 +245,7 @@ fn resolve_package_target_string( _base: &ModuleSpecifier, pattern: bool, internal: bool, + _conditions: &[&str], ) -> Result { if subpath != "" && !pattern && !target.ends_with('/') { todo!() @@ -289,7 +293,6 @@ fn resolve_package_target_string( Ok(resolved.join(&subpath)?) } -// TODO(bartlomieju): last argument "conditions" was skipped fn resolve_package_target( package_json_url: Url, target: Value, @@ -298,6 +301,7 @@ fn resolve_package_target( base: &ModuleSpecifier, pattern: bool, internal: bool, + conditions: &[&str], ) -> Result, AnyError> { if let Some(target) = target.as_str() { return Ok(Some(resolve_package_target_string( @@ -308,7 +312,7 @@ fn resolve_package_target( base, pattern, internal, - // TODO(bartlomieju): last argument "conditions" was skipped + conditions, )?)); } else if let Some(target_arr) = target.as_array() { if target_arr.is_empty() { @@ -320,10 +324,7 @@ fn resolve_package_target( for key in target_obj.keys() { // TODO(bartlomieju): verify that keys are not numeric - // TODO(bartlomieju): this should be injected as an - // argument to the function - let conditions = vec!["node".to_string(), "import".to_string()]; - if key == "default" || conditions.contains(&key) { + if key == "default" || conditions.contains(&key.as_str()) { let condition_target = target_obj.get(key).unwrap().to_owned(); let resolved = resolve_package_target( package_json_url.clone(), @@ -333,6 +334,7 @@ fn resolve_package_target( base, pattern, internal, + conditions, )?; if resolved.is_none() { continue; @@ -347,12 +349,12 @@ fn resolve_package_target( todo!() } -// TODO(bartlomieju): last argument "conditions" was skipped fn package_exports_resolve( package_json_url: Url, package_subpath: String, package_config: PackageConfig, base: &ModuleSpecifier, + conditions: &[&str], ) -> Result { let exports = &package_config.exports.unwrap(); @@ -370,7 +372,6 @@ fn package_exports_resolve( && !package_subpath.ends_with('/') { let target = exports_map.get(&package_subpath).unwrap().to_owned(); - // TODO(bartlomieju): last argument "conditions" was skipped let resolved = resolve_package_target( package_json_url, target, @@ -379,6 +380,7 @@ fn package_exports_resolve( base, false, false, + conditions, )?; // TODO(): if resolved.is_none() { @@ -393,6 +395,7 @@ fn package_exports_resolve( fn package_resolve( specifier: &str, base: &ModuleSpecifier, + conditions: &[&str], ) -> Result { // eprintln!("package_resolve {} {}", specifier, base.as_str()); let (package_name, package_subpath, is_scoped) = @@ -410,12 +413,12 @@ fn package_resolve( if package_config.name.as_ref() == Some(&package_name) { if let Some(exports) = &package_config.exports { if !exports.is_null() { - // TODO(bartlomieju): last argument "conditions" was skipped return package_exports_resolve( package_json_url, package_subpath, package_config, base, + conditions, ); } } @@ -465,6 +468,7 @@ fn package_resolve( package_subpath, package_config, base, + conditions, ); } if package_subpath == "." { From 03ef6f7aa52515e2c80d99802d6525a016268417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 07:31:12 +0200 Subject: [PATCH 10/40] rename, remove debug prints --- cli/node_module_loader.rs | 51 ++++++++++++--------------------------- cli/proc_state.rs | 4 +-- 2 files changed, 17 insertions(+), 38 deletions(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index c23be3abde9661..61309e55fb5530 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -12,20 +12,24 @@ use regex::Regex; use std::path::PathBuf; #[derive(Debug, Default)] -pub(crate) struct NodeResolver; +pub(crate) struct NodeEsmResolver; -impl NodeResolver { +impl NodeEsmResolver { pub fn as_resolver(&self) -> &dyn Resolver { self } } -impl Resolver for NodeResolver { +impl Resolver for NodeEsmResolver { fn resolve( &self, specifier: &str, referrer: &ModuleSpecifier, ) -> Result { + // TODO(bartlomieju): this is hacky, remove + // needed to add it here because `deno_std/node` has + // triple-slash references and they should still resolve + // the regular way (I think) if referrer.as_str().starts_with("https://deno.land/std") { return referrer.join(specifier).map_err(AnyError::from); } @@ -41,9 +45,9 @@ fn node_resolve( specifier: &str, referrer: &str, ) -> Result { - // eprintln!("NODE_RESOLVE {} {}", specifier, referrer); - // TODO(bartlomieju): skipped "policy" part + // TODO(bartlomieju): skipped "policy" part as we don't plan to support it + // TODO(bartlomieju): this remapping should be done in `compat` module if crate::compat::SUPPORTED_MODULES.contains(&specifier) { let module_url = format!("{}node/{}.ts", crate::compat::STD_URL, specifier); return Ok(Url::parse(&module_url).unwrap()); @@ -120,7 +124,6 @@ fn module_resolve( base: &ModuleSpecifier, conditions: &[&str], ) -> Result { - // eprintln!("module resolve {} {}", specifier, base.as_str()); let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { base.join(specifier)? } else if specifier.chars().nth(0) == Some('#') { @@ -139,7 +142,9 @@ fn finalize_resolution( resolved: ModuleSpecifier, base: &ModuleSpecifier, ) -> Result { - // TODO(bartlomieju): short circuit for remote modules + // TODO(bartlomieju): this is not part of Node resolution + // (as it doesn't support http/https); + // but I had to short circuit for remote modules to avoid errors if resolved.scheme().starts_with("http") { return Ok(resolved); } @@ -154,7 +159,6 @@ fn finalize_resolution( ))); } - // eprintln!("resolved {}, base: {}", resolved.as_str(), base.as_str()); let path = resolved.to_file_path().unwrap(); // TODO(bartlomieju): currently not supported @@ -169,7 +173,6 @@ fn finalize_resolution( p_str.to_string() }; - // eprintln!("meta data path: {}", p); let (is_dir, is_file) = if let Ok(stats) = std::fs::metadata(&p) { (stats.is_dir(), stats.is_file()) } else { @@ -382,7 +385,7 @@ fn package_exports_resolve( false, conditions, )?; - // TODO(): + // TODO(bartlomieju): return error here if resolved.is_none() { todo!() } @@ -397,7 +400,6 @@ fn package_resolve( base: &ModuleSpecifier, conditions: &[&str], ) -> Result { - // eprintln!("package_resolve {} {}", specifier, base.as_str()); let (package_name, package_subpath, is_scoped) = parse_package_name(specifier, base)?; @@ -406,10 +408,6 @@ fn package_resolve( if package_config.exists { let package_json_url = Url::from_file_path(&package_config.pjsonpath).unwrap(); - // eprintln!( - // "package_config.name {:?} package_name {:?} exports {:#?}", - // package_config.name, package_name, package_config.exports - // ); if package_config.name.as_ref() == Some(&package_name) { if let Some(exports) = &package_config.exports { if !exports.is_null() { @@ -458,10 +456,8 @@ fn package_resolve( } // Package match. - // eprintln!("got package match!"); let package_config = get_package_config(package_json_path.clone(), specifier, Some(base))?; - // eprintln!("got package match {} {} {:#?}", package_json_path.display(), specifier, package_config.exports); if package_config.exports.is_some() { return package_exports_resolve( package_json_url, @@ -517,7 +513,6 @@ fn parse_package_name( } } - // // eprintln!("specifier: {:?}, base: {:?}", specifier, base); if !valid_package_name { return Err(generic_error(format!( "{} is not a valid package name {}", @@ -536,16 +531,6 @@ fn parse_package_name( Ok((package_name, package_subpath, is_scoped)) } -// enum ExportConfig { -// Str(String), -// StrArray(Vec), -// } - -// enum PackageType { -// Module, -// CommonJs, -// } - #[derive(Clone, Debug)] struct PackageConfig { exists: bool, @@ -567,8 +552,6 @@ fn get_package_config( // return Ok(existing.clone()); // } - // TODO: maybe shouldn't error be return empty package - // eprintln!("get package config: {} {:?}", specifier, path); let result = std::fs::read_to_string(&path); let source = result.unwrap_or_else(|_| "".to_string()); @@ -678,17 +661,12 @@ fn get_package_scope_config( break; } - // eprintln!("get package scope config"); let package_config = get_package_config( package_json_url.to_file_path().unwrap(), resolved.as_str(), None, )?; - // eprintln!( - // "found package config {}, exists: {}", - // package_json_url.as_str(), - // package_config.exists - // ); + if package_config.exists { return Ok(package_config); } @@ -696,6 +674,7 @@ fn get_package_scope_config( let last_package_json_url = package_json_url.clone(); package_json_url = package_json_url.join("../package.json")?; + // TODO(bartlomieju): I'm not sure this will work properly // Terminates at root where ../package.json equals ../../package.json // (can't just check "/package.json" for Windows support) if package_json_url.path() == last_package_json_url.path() { diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 346a26a6ff245b..500555301c5788 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -13,7 +13,7 @@ use crate::flags; use crate::http_cache; use crate::lockfile::as_maybe_locker; use crate::lockfile::Lockfile; -use crate::node_module_loader::NodeResolver; +use crate::node_module_loader::NodeEsmResolver; use crate::resolver::ImportMapResolver; use crate::source_maps::SourceMapGetter; use crate::version; @@ -317,7 +317,7 @@ impl ProcState { ); let maybe_locker = as_maybe_locker(self.lockfile.clone()); let maybe_imports = self.get_maybe_imports(); - let node_resolver = NodeResolver; + let node_resolver = NodeEsmResolver; let import_map_resolver = self.maybe_import_map.as_ref().map(ImportMapResolver::new); let maybe_resolver = if self.flags.compat { From ccf73ec9a08b244d6306d2a596ebe366ff0094f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 07:47:47 +0200 Subject: [PATCH 11/40] clippy --- cli/node_module_loader.rs | 63 +++++++++++++++------------------------ 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/cli/node_module_loader.rs b/cli/node_module_loader.rs index 61309e55fb5530..db1f4e33cf8a1e 100644 --- a/cli/node_module_loader.rs +++ b/cli/node_module_loader.rs @@ -91,11 +91,11 @@ fn node_resolve( } fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { - if specifier == "" { + if specifier.is_empty() { return false; } - if specifier.chars().nth(0) == Some('/') { + if specifier.starts_with('/') { return true; } @@ -110,10 +110,10 @@ fn is_relative_specifier(specifier: &str) -> bool { if specifier_len == 1 || specifier_chars[1] == '/' { return true; } - if specifier_chars[1] == '.' { - if specifier_len == 2 || specifier_chars[2] == '/' { - return true; - } + if specifier_chars[1] == '.' + && (specifier_len == 2 || specifier_chars[2] == '/') + { + return true; } } false @@ -126,14 +126,12 @@ fn module_resolve( ) -> Result { let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { base.join(specifier)? - } else if specifier.chars().nth(0) == Some('#') { + } else if specifier.starts_with('#') { package_imports_resolve(specifier, base)? + } else if let Ok(resolved) = Url::parse(specifier) { + resolved } else { - if let Ok(resolved) = Url::parse(specifier) { - resolved - } else { - package_resolve(specifier, base, conditions)? - } + package_resolve(specifier, base, conditions)? }; finalize_resolution(resolved, base) } @@ -220,7 +218,7 @@ fn is_conditional_exports_main_sugar( let mut is_conditional_sugar = false; let mut i = 0; for key in exports_obj.keys() { - let cur_is_conditional_sugar = key == "" || !key.starts_with('.'); + let cur_is_conditional_sugar = key.is_empty() || !key.starts_with('.'); if i == 0 { is_conditional_sugar = cur_is_conditional_sugar; i += 1; @@ -240,6 +238,7 @@ fn is_conditional_exports_main_sugar( Ok(is_conditional_sugar) } +#[allow(clippy::too_many_arguments)] fn resolve_package_target_string( target: String, subpath: String, @@ -250,7 +249,7 @@ fn resolve_package_target_string( internal: bool, _conditions: &[&str], ) -> Result { - if subpath != "" && !pattern && !target.ends_with('/') { + if !subpath.is_empty() && !pattern && !target.ends_with('/') { todo!() } @@ -278,7 +277,7 @@ fn resolve_package_target_string( todo!() } - if subpath == "" { + if subpath.is_empty() { return Ok(resolved); } @@ -296,6 +295,7 @@ fn resolve_package_target_string( Ok(resolved.join(&subpath)?) } +#[allow(clippy::too_many_arguments)] fn resolve_package_target( package_json_url: Url, target: Value, @@ -488,14 +488,12 @@ fn parse_package_name( let mut is_scoped = false; if specifier.is_empty() { valid_package_name = false; - } else { - if specifier.chars().nth(0) == Some('@') { - is_scoped = true; - if let Some(index) = separator_index { - separator_index = specifier[index + 1..].find('/'); - } else { - valid_package_name = false; - } + } else if specifier.starts_with('@') { + is_scoped = true; + if let Some(index) = separator_index { + separator_index = specifier[index + 1..].find('/'); + } else { + valid_package_name = false; } } @@ -523,7 +521,6 @@ fn parse_package_name( let package_subpath = if let Some(index) = separator_index { format!(".{}", specifier.chars().skip(index).collect::()) - .to_string() } else { ".".to_string() }; @@ -593,29 +590,17 @@ fn get_package_config( // TODO(bartlomieju): refactor let imports = if let Some(imp) = imports_val { - if let Some(imp) = imp.as_object() { - Some(imp.to_owned()) - } else { - None - } + imp.as_object().map(|imp| imp.to_owned()) } else { None }; let main = if let Some(m) = main_val { - if let Some(m) = m.as_str() { - Some(m.to_string()) - } else { - None - } + m.as_str().map(|m| m.to_string()) } else { None }; let name = if let Some(n) = name_val { - if let Some(n) = n.as_str() { - Some(n.to_string()) - } else { - None - } + n.as_str().map(|n| n.to_string()) } else { None }; From 34bb54125462e61e60f239e3da408de99c9e52cf Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 13 Oct 2021 08:03:30 -0400 Subject: [PATCH 12/40] add tests --- cli/{compat.rs => compat/mod.rs} | 4 ++ cli/{ => compat}/node_module_loader.rs | 59 ++++++++++++++++++- cli/compat/testdata/basic/main.js | 1 + .../testdata/basic/node_modules/foo/index.js | 0 .../basic/node_modules/foo/package.json | 5 ++ cli/compat/testdata/basic/package.json | 7 +++ cli/compat/testdata/basic_deps/main.js | 1 + .../basic_deps/node_modules/bar/bar.js | 1 + .../basic_deps/node_modules/bar/package.json | 6 ++ .../basic_deps/node_modules/foo/foo.js | 1 + .../basic_deps/node_modules/foo/package.json | 8 +++ cli/compat/testdata/basic_deps/package.json | 7 +++ cli/main.rs | 1 - cli/proc_state.rs | 2 +- 14 files changed, 98 insertions(+), 5 deletions(-) rename cli/{compat.rs => compat/mod.rs} (97%) rename cli/{ => compat}/node_module_loader.rs (91%) create mode 100644 cli/compat/testdata/basic/main.js create mode 100644 cli/compat/testdata/basic/node_modules/foo/index.js create mode 100644 cli/compat/testdata/basic/node_modules/foo/package.json create mode 100644 cli/compat/testdata/basic/package.json create mode 100644 cli/compat/testdata/basic_deps/main.js create mode 100644 cli/compat/testdata/basic_deps/node_modules/bar/bar.js create mode 100644 cli/compat/testdata/basic_deps/node_modules/bar/package.json create mode 100644 cli/compat/testdata/basic_deps/node_modules/foo/foo.js create mode 100644 cli/compat/testdata/basic_deps/node_modules/foo/package.json create mode 100644 cli/compat/testdata/basic_deps/package.json diff --git a/cli/compat.rs b/cli/compat/mod.rs similarity index 97% rename from cli/compat.rs rename to cli/compat/mod.rs index 99e3c127a06ac0..7a2f35c489e515 100644 --- a/cli/compat.rs +++ b/cli/compat/mod.rs @@ -1,8 +1,12 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. +mod node_module_loader; + use deno_core::url::Url; use std::collections::HashMap; +pub use node_module_loader::NodeEsmResolver; + // TODO(bartlomieju): this needs to be bumped manually for // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is diff --git a/cli/node_module_loader.rs b/cli/compat/node_module_loader.rs similarity index 91% rename from cli/node_module_loader.rs rename to cli/compat/node_module_loader.rs index db1f4e33cf8a1e..65c944303679fa 100644 --- a/cli/node_module_loader.rs +++ b/cli/compat/node_module_loader.rs @@ -12,7 +12,7 @@ use regex::Regex; use std::path::PathBuf; #[derive(Debug, Default)] -pub(crate) struct NodeEsmResolver; +pub struct NodeEsmResolver; impl NodeEsmResolver { pub fn as_resolver(&self) -> &dyn Resolver { @@ -33,7 +33,7 @@ impl Resolver for NodeEsmResolver { if referrer.as_str().starts_with("https://deno.land/std") { return referrer.join(specifier).map_err(AnyError::from); } - node_resolve(specifier, referrer.as_str()) + node_resolve(specifier, referrer.as_str(), &std::env::current_dir()?) } } @@ -44,6 +44,7 @@ static DEFAULT_CONDITIONS: &[&str] = &["node", "import"]; fn node_resolve( specifier: &str, referrer: &str, + cwd: &std::path::Path, ) -> Result { // TODO(bartlomieju): skipped "policy" part as we don't plan to support it @@ -76,7 +77,6 @@ fn node_resolve( let is_main = referrer.is_empty(); let parent_url = if is_main { - let cwd = std::env::current_dir()?; Url::from_directory_path(cwd).unwrap() } else { Url::parse(referrer).expect("referrer was not proper url") @@ -737,3 +737,56 @@ fn legacy_main_resolve( Err(generic_error("not found")) } + +#[cfg(test)] +mod tests { + use super::*; + + fn testdir(name: &str) -> PathBuf { + let c = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + c.join("compat/testdata/").join(name) + } + + #[test] + fn basic() { + let cwd = testdir("basic"); + let main = Url::from_file_path(cwd.join("main.js")).unwrap(); + let actual = node_resolve("foo", main.as_str(), &cwd).unwrap(); + let expected = + Url::from_file_path(cwd.join("node_modules/foo/index.js")).unwrap(); + assert_eq!(actual, expected); + } + + #[test] + fn basic_deps() { + let cwd = testdir("basic_deps"); + let main = Url::from_file_path(cwd.join("main.js")).unwrap(); + let actual = node_resolve("foo", main.as_str(), &cwd).unwrap(); + let foo_js = + Url::from_file_path(cwd.join("node_modules/foo/foo.js")).unwrap(); + assert_eq!(actual, foo_js); + + let actual = node_resolve("bar", foo_js.as_str(), &cwd).unwrap(); + + let bar_js = + Url::from_file_path(cwd.join("node_modules/bar/bar.js")).unwrap(); + assert_eq!(actual, bar_js); + } + + #[test] + fn builtin_http() { + let cwd = testdir("basic"); + let main = Url::from_file_path(cwd.join("main.js")).unwrap(); + let expected = + Url::parse("https://deno.land/std@0.111.0/node/http.ts").unwrap(); + + let actual = node_resolve("http", main.as_str(), &cwd).unwrap(); + println!("actual {}", actual); + assert_eq!(actual, expected); + + // TODO(ry) + // let actual = node_resolve("node:http", main.as_str(), &cwd).unwrap(); + // println!("actual {}", actual); + // assert_eq!(actual, expected); + } +} diff --git a/cli/compat/testdata/basic/main.js b/cli/compat/testdata/basic/main.js new file mode 100644 index 00000000000000..c0748305d53575 --- /dev/null +++ b/cli/compat/testdata/basic/main.js @@ -0,0 +1 @@ +import "foo"; diff --git a/cli/compat/testdata/basic/node_modules/foo/index.js b/cli/compat/testdata/basic/node_modules/foo/index.js new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/cli/compat/testdata/basic/node_modules/foo/package.json b/cli/compat/testdata/basic/node_modules/foo/package.json new file mode 100644 index 00000000000000..a74d52fd351934 --- /dev/null +++ b/cli/compat/testdata/basic/node_modules/foo/package.json @@ -0,0 +1,5 @@ +{ + "name": "foo", + "type": "module", + "exports": "./index.js" +} diff --git a/cli/compat/testdata/basic/package.json b/cli/compat/testdata/basic/package.json new file mode 100644 index 00000000000000..f86a065ad7f33e --- /dev/null +++ b/cli/compat/testdata/basic/package.json @@ -0,0 +1,7 @@ +{ + "name": "bar", + "type": "module", + "dependencies": { + "foo": "1.0.0" + } +} diff --git a/cli/compat/testdata/basic_deps/main.js b/cli/compat/testdata/basic_deps/main.js new file mode 100644 index 00000000000000..c0748305d53575 --- /dev/null +++ b/cli/compat/testdata/basic_deps/main.js @@ -0,0 +1 @@ +import "foo"; diff --git a/cli/compat/testdata/basic_deps/node_modules/bar/bar.js b/cli/compat/testdata/basic_deps/node_modules/bar/bar.js new file mode 100644 index 00000000000000..98e51675eab6f5 --- /dev/null +++ b/cli/compat/testdata/basic_deps/node_modules/bar/bar.js @@ -0,0 +1 @@ +export const BAR = 123; diff --git a/cli/compat/testdata/basic_deps/node_modules/bar/package.json b/cli/compat/testdata/basic_deps/node_modules/bar/package.json new file mode 100644 index 00000000000000..c2043f6100c027 --- /dev/null +++ b/cli/compat/testdata/basic_deps/node_modules/bar/package.json @@ -0,0 +1,6 @@ +{ + "name": "bar", + "version": "0.1.2", + "type": "module", + "exports": "./bar.js" +} diff --git a/cli/compat/testdata/basic_deps/node_modules/foo/foo.js b/cli/compat/testdata/basic_deps/node_modules/foo/foo.js new file mode 100644 index 00000000000000..0026acc8ebc14c --- /dev/null +++ b/cli/compat/testdata/basic_deps/node_modules/foo/foo.js @@ -0,0 +1 @@ +import "bar"; diff --git a/cli/compat/testdata/basic_deps/node_modules/foo/package.json b/cli/compat/testdata/basic_deps/node_modules/foo/package.json new file mode 100644 index 00000000000000..376dae81e635a1 --- /dev/null +++ b/cli/compat/testdata/basic_deps/node_modules/foo/package.json @@ -0,0 +1,8 @@ +{ + "name": "foo", + "type": "module", + "exports": "./foo.js", + "dependencies": { + "bar": "0.1.2" + } +} diff --git a/cli/compat/testdata/basic_deps/package.json b/cli/compat/testdata/basic_deps/package.json new file mode 100644 index 00000000000000..38ff2e1bcf197e --- /dev/null +++ b/cli/compat/testdata/basic_deps/package.json @@ -0,0 +1,7 @@ +{ + "name": "main_program", + "type": "module", + "dependencies": { + "foo": "1.0.0" + } +} diff --git a/cli/main.rs b/cli/main.rs index d73417a1a0c769..8a85cacfbc44c1 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -24,7 +24,6 @@ mod lockfile; mod logger; mod lsp; mod module_loader; -mod node_module_loader; mod ops; mod proc_state; mod resolver; diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 500555301c5788..ca1d22280a4d43 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -3,6 +3,7 @@ use crate::cache; use crate::colors; use crate::compat; +use crate::compat::NodeEsmResolver; use crate::config_file::ConfigFile; use crate::deno_dir; use crate::emit; @@ -13,7 +14,6 @@ use crate::flags; use crate::http_cache; use crate::lockfile::as_maybe_locker; use crate::lockfile::Lockfile; -use crate::node_module_loader::NodeEsmResolver; use crate::resolver::ImportMapResolver; use crate::source_maps::SourceMapGetter; use crate::version; From bea885aead1bee5fd677fdba8f6e0172f4957315 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 13 Oct 2021 08:14:11 -0400 Subject: [PATCH 13/40] add deep test --- cli/compat/node_module_loader.rs | 10 ++++++++++ cli/compat/testdata/deep/a/b/c/d/main.js | 1 + cli/compat/testdata/deep/node_modules/foo/index.js | 0 cli/compat/testdata/deep/node_modules/foo/package.json | 5 +++++ 4 files changed, 16 insertions(+) create mode 100644 cli/compat/testdata/deep/a/b/c/d/main.js create mode 100644 cli/compat/testdata/deep/node_modules/foo/index.js create mode 100644 cli/compat/testdata/deep/node_modules/foo/package.json diff --git a/cli/compat/node_module_loader.rs b/cli/compat/node_module_loader.rs index 65c944303679fa..65f6e0fb52f027 100644 --- a/cli/compat/node_module_loader.rs +++ b/cli/compat/node_module_loader.rs @@ -757,6 +757,16 @@ mod tests { assert_eq!(actual, expected); } + #[test] + fn deep() { + let cwd = testdir("deep"); + let main = Url::from_file_path(cwd.join("a/b/c/d/main.js")).unwrap(); + let actual = node_resolve("foo", main.as_str(), &cwd).unwrap(); + let expected = + Url::from_file_path(cwd.join("node_modules/foo/index.js")).unwrap(); + assert_eq!(actual, expected); + } + #[test] fn basic_deps() { let cwd = testdir("basic_deps"); diff --git a/cli/compat/testdata/deep/a/b/c/d/main.js b/cli/compat/testdata/deep/a/b/c/d/main.js new file mode 100644 index 00000000000000..c0748305d53575 --- /dev/null +++ b/cli/compat/testdata/deep/a/b/c/d/main.js @@ -0,0 +1 @@ +import "foo"; diff --git a/cli/compat/testdata/deep/node_modules/foo/index.js b/cli/compat/testdata/deep/node_modules/foo/index.js new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/cli/compat/testdata/deep/node_modules/foo/package.json b/cli/compat/testdata/deep/node_modules/foo/package.json new file mode 100644 index 00000000000000..a74d52fd351934 --- /dev/null +++ b/cli/compat/testdata/deep/node_modules/foo/package.json @@ -0,0 +1,5 @@ +{ + "name": "foo", + "type": "module", + "exports": "./index.js" +} From bba059a52e6e61f65f8ce14991db2d2e49ac8e93 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 13 Oct 2021 12:12:04 -0400 Subject: [PATCH 14/40] fmt --- cli/compat/testdata/basic/package.json | 10 +++++----- cli/compat/testdata/basic_deps/package.json | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/compat/testdata/basic/package.json b/cli/compat/testdata/basic/package.json index f86a065ad7f33e..cc4ac5493293f5 100644 --- a/cli/compat/testdata/basic/package.json +++ b/cli/compat/testdata/basic/package.json @@ -1,7 +1,7 @@ { - "name": "bar", - "type": "module", - "dependencies": { - "foo": "1.0.0" - } + "name": "bar", + "type": "module", + "dependencies": { + "foo": "1.0.0" + } } diff --git a/cli/compat/testdata/basic_deps/package.json b/cli/compat/testdata/basic_deps/package.json index 38ff2e1bcf197e..138d401ed7a97c 100644 --- a/cli/compat/testdata/basic_deps/package.json +++ b/cli/compat/testdata/basic_deps/package.json @@ -1,7 +1,7 @@ { - "name": "main_program", - "type": "module", - "dependencies": { - "foo": "1.0.0" - } + "name": "main_program", + "type": "module", + "dependencies": { + "foo": "1.0.0" + } } From 89e11b32d0c4ee6c1c100e393b5fd4f5dd4c8f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 18:41:15 +0200 Subject: [PATCH 15/40] remove import map --- cli/compat/mod.rs | 25 ++++------------- cli/compat/node_module_loader.rs | 24 ++++++++++------ cli/proc_state.rs | 28 +------------------ cli/tests/integration/compat_tests.rs | 6 ---- .../testdata/compat/existing_import_map.json | 5 ---- .../testdata/compat/existing_import_map.out | 7 ----- 6 files changed, 22 insertions(+), 73 deletions(-) delete mode 100644 cli/tests/testdata/compat/existing_import_map.json delete mode 100644 cli/tests/testdata/compat/existing_import_map.out diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 7a2f35c489e515..7b58ffc054174c 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -3,7 +3,6 @@ mod node_module_loader; use deno_core::url::Url; -use std::collections::HashMap; pub use node_module_loader::NodeEsmResolver; @@ -70,23 +69,11 @@ pub(crate) fn get_node_imports() -> Vec<(Url, Vec)> { vec![(COMPAT_IMPORT_URL.clone(), vec![GLOBAL_URL_STR.clone()])] } -/// Create a map that can be used to update import map. -/// -/// Keys are built-in Node modules (and built-ins prefixed with "node:"), while -/// values are URLs pointing to relevant files in deno.land/std/node/ directory. -pub fn get_mapped_node_builtins() -> HashMap { - let mut mappings = HashMap::new(); - - for module in SUPPORTED_MODULES { - // TODO(bartlomieju): this is unversioned, and should be fixed to use latest stable? - let module_url = format!("{}node/{}.ts", STD_URL, module); - mappings.insert(module.to_string(), module_url.clone()); - - // Support for `node:` - // https://nodejs.org/api/esm.html#esm_node_imports - let node_prefixed = format!("node:{}", module); - mappings.insert(node_prefixed, module_url); +pub(crate) fn try_resolve_builtin_module(specifier: &str) -> Option { + if SUPPORTED_MODULES.contains(&specifier) { + let module_url = format!("{}node/{}.ts", crate::compat::STD_URL, specifier); + Some(Url::parse(&module_url).unwrap()) + } else { + None } - - mappings } diff --git a/cli/compat/node_module_loader.rs b/cli/compat/node_module_loader.rs index 65f6e0fb52f027..2b766f8e083476 100644 --- a/cli/compat/node_module_loader.rs +++ b/cli/compat/node_module_loader.rs @@ -48,10 +48,8 @@ fn node_resolve( ) -> Result { // TODO(bartlomieju): skipped "policy" part as we don't plan to support it - // TODO(bartlomieju): this remapping should be done in `compat` module - if crate::compat::SUPPORTED_MODULES.contains(&specifier) { - let module_url = format!("{}node/{}.ts", crate::compat::STD_URL, specifier); - return Ok(Url::parse(&module_url).unwrap()); + if let Some(resolved) = crate::compat::try_resolve_builtin_module(specifier) { + return Ok(resolved); } if let Ok(url) = Url::parse(specifier) { @@ -62,7 +60,16 @@ fn node_resolve( let protocol = url.scheme(); if protocol == "node" { - return Ok(url); + let mut split_specifier = url.as_str().split(':'); + split_specifier.next(); + let specifier = split_specifier.collect::>().join(""); + if let Some(resolved) = + crate::compat::try_resolve_builtin_module(&specifier) + { + return Ok(resolved); + } else { + return Err(generic_error(format!("Unknown module {}", specifier))); + } } if protocol != "file" && protocol != "data" { @@ -794,9 +801,8 @@ mod tests { println!("actual {}", actual); assert_eq!(actual, expected); - // TODO(ry) - // let actual = node_resolve("node:http", main.as_str(), &cwd).unwrap(); - // println!("actual {}", actual); - // assert_eq!(actual, expected); + let actual = node_resolve("node:http", main.as_str(), &cwd).unwrap(); + println!("actual {}", actual); + assert_eq!(actual, expected); } } diff --git a/cli/proc_state.rs b/cli/proc_state.rs index ca1d22280a4d43..0b4938d50bd710 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -196,7 +196,7 @@ impl ProcState { None }; - let mut maybe_import_map: Option = + let maybe_import_map: Option = match flags.import_map_path.as_ref() { None => None, Some(import_map_url) => { @@ -218,32 +218,6 @@ impl ProcState { } }; - if flags.compat { - let mut import_map = match maybe_import_map { - Some(import_map) => import_map, - None => { - // INFO: we're creating an empty import map, with its specifier pointing - // to `CWD/node_import_map.json` to make sure the map still works as expected. - let import_map_specifier = - std::env::current_dir()?.join("node_import_map.json"); - ImportMap::from_json(import_map_specifier.to_str().unwrap(), "{}") - .unwrap() - } - }; - let node_builtins = compat::get_mapped_node_builtins(); - let diagnostics = import_map.update_imports(node_builtins)?; - - if !diagnostics.is_empty() { - log::info!("Some Node built-ins were not added to the import map:"); - for diagnostic in diagnostics { - log::info!(" - {}", diagnostic); - } - log::info!("If you want to use Node built-ins provided by Deno remove listed specifiers from \"imports\" mapping in the import map file."); - } - - maybe_import_map = Some(import_map); - } - let maybe_inspect_host = flags.inspect.or(flags.inspect_brk); let maybe_inspector_server = maybe_inspect_host.map(|host| { Arc::new(InspectorServer::new(host, version::get_user_agent())) diff --git a/cli/tests/integration/compat_tests.rs b/cli/tests/integration/compat_tests.rs index 6b6ab81b54ffcf..f0feb14e6ecbbe 100644 --- a/cli/tests/integration/compat_tests.rs +++ b/cli/tests/integration/compat_tests.rs @@ -18,12 +18,6 @@ itest!(node_prefix_fs_promises { output: "compat/fs_promises.out", }); -itest!(existing_import_map { - args: "run --compat --unstable --import-map compat/existing_import_map.json compat/fs_promises.js", - output: "compat/existing_import_map.out", - exit_code: 1, -}); - #[test] fn globals_in_repl() { let (out, _err) = util::run_and_collect_output_with_args( diff --git a/cli/tests/testdata/compat/existing_import_map.json b/cli/tests/testdata/compat/existing_import_map.json deleted file mode 100644 index db59c0cc2400d7..00000000000000 --- a/cli/tests/testdata/compat/existing_import_map.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "imports": { - "fs/promises": "./non_existent_file.js" - } -} diff --git a/cli/tests/testdata/compat/existing_import_map.out b/cli/tests/testdata/compat/existing_import_map.out deleted file mode 100644 index 46125d411640ab..00000000000000 --- a/cli/tests/testdata/compat/existing_import_map.out +++ /dev/null @@ -1,7 +0,0 @@ -[WILDCARD] -Some Node built-ins were not added to the import map: - - "fs/promises" already exists and is mapped to "file://[WILDCARD]/non_existent_file.js" -If you want to use Node built-ins provided by Deno remove listed specifiers from "imports" mapping in the import map file. -[WILDCARD] -error: Cannot load module "file://[WILDCARD]/non_existent_file.js". - at file://[WILDCARD]/fs_promises.js:1:16 From 349804c4eea5cdb3d22c504e45bf93f5577ceded Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 19:30:31 +0200 Subject: [PATCH 16/40] port errors --- cli/compat/errors.rs | 75 +++++++++++++++++++++++++ cli/compat/mod.rs | 5 +- cli/compat/node_module_loader.rs | 94 ++++++++++++++++++-------------- 3 files changed, 132 insertions(+), 42 deletions(-) create mode 100644 cli/compat/errors.rs diff --git a/cli/compat/errors.rs b/cli/compat/errors.rs new file mode 100644 index 00000000000000..108414e8e66c7e --- /dev/null +++ b/cli/compat/errors.rs @@ -0,0 +1,75 @@ +// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. + +use deno_core::error::generic_error; +use deno_core::error::type_error; +use deno_core::error::AnyError; +use deno_core::url::Url; + +pub(crate) fn err_invalid_module_specifier( + request: &str, + reason: &str, + maybe_base: Option, +) -> AnyError { + let mut msg = format!( + "[ERR_INVALID_MODULE_SPECIFIER] Invalid module \"{}\" {}", + request, reason + ); + + if let Some(base) = maybe_base { + msg = format!("{} imported from {}", msg, base); + } + + type_error(msg) +} + +pub(crate) fn err_invalid_package_config( + path: &str, + maybe_base: Option, + maybe_message: Option, +) -> AnyError { + let mut msg = format!( + "[ERR_INVALID_PACKAGE_CONFIG] Invalid package config {}", + path + ); + + if let Some(base) = maybe_base { + msg = format!("{} while importing {}", msg, base); + } + + if let Some(message) = maybe_message { + msg = format!("{}. {}", msg, message); + } + + generic_error(msg) +} + +pub(crate) fn err_module_not_found( + path: &str, + base: &str, + typ: &str, +) -> AnyError { + generic_error(format!( + "[ERR_MODULE_NOT_FOUND] Cannot find {} '{}' imported from {}", + typ, path, base + )) +} + +pub(crate) fn err_unsupported_dir_import(path: &str, base: &str) -> AnyError { + generic_error(format!("[ERR_UNSUPPORTED_DIR_IMPORT] Directory import '{}' is not supported resolving ES modules imported from {}", path, base)) +} + +pub(crate) fn err_unsupported_esm_url_scheme(url: &Url) -> AnyError { + let mut msg = + "[ERR_UNSUPPORTED_ESM_URL_SCHEME] Only file and data URLS are supported by the default ESM loader" + .to_string(); + + if cfg!(window) && url.scheme().len() == 2 { + msg = format!( + "{}. On Windows, absolute path must be valid file:// URLs", + msg + ); + } + + msg = format!("{}. Received protocol '{}'", msg, url.scheme()); + generic_error(msg) +} diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 7b58ffc054174c..60de72944b229d 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -1,5 +1,6 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. +mod errors; mod node_module_loader; use deno_core::url::Url; @@ -10,10 +11,10 @@ pub use node_module_loader::NodeEsmResolver; // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded -pub(crate) static STD_URL: &str = "https://deno.land/std@0.111.0/"; +pub static STD_URL: &str = "https://deno.land/std@0.111.0/"; static GLOBAL_MODULE: &str = "global.ts"; -pub(crate) static SUPPORTED_MODULES: &[&str] = &[ +pub static SUPPORTED_MODULES: &[&str] = &[ "assert", "assert/strict", "async_hooks", diff --git a/cli/compat/node_module_loader.rs b/cli/compat/node_module_loader.rs index 2b766f8e083476..34aed929cbc3d3 100644 --- a/cli/compat/node_module_loader.rs +++ b/cli/compat/node_module_loader.rs @@ -1,5 +1,6 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. +use super::errors; use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::serde_json; @@ -73,7 +74,7 @@ fn node_resolve( } if protocol != "file" && protocol != "data" { - return Err(generic_error(format!("Only file and data URLs are supported by the default ESM loader. Received protocol '{}'", protocol))); + return Err(errors::err_unsupported_esm_url_scheme(&url)); } if referrer.starts_with("data:") { @@ -157,11 +158,11 @@ fn finalize_resolution( let encoded_sep_re = Regex::new(r"%2F|%2C").expect("bad regex"); if encoded_sep_re.is_match(resolved.path()) { - return Err(generic_error(format!( - "{} must not include encoded \"/\" or \"\\\\\" characters {}", + return Err(errors::err_invalid_module_specifier( resolved.path(), - base.to_file_path().unwrap().display() - ))); + "must not include encoded \"/\" or \"\\\\\" characters", + Some(base.to_file_path().unwrap().display().to_string()), + )); } let path = resolved.to_file_path().unwrap(); @@ -184,18 +185,16 @@ fn finalize_resolution( (false, false) }; if is_dir { - return Err( - generic_error( - format!("Directory import {} is not supported resolving ES modules imported from {}", - path.display(), base.to_file_path().unwrap().display() - ) + return Err(errors::err_unsupported_dir_import( + &path.display().to_string(), + &base.to_file_path().unwrap().display().to_string(), )); } else if !is_file { - return Err(generic_error(format!( - "Cannot find module {} imported from {}", - path.display(), - base.to_file_path().unwrap().display() - ))); + return Err(errors::err_module_not_found( + &path.display().to_string(), + &base.to_file_path().unwrap().display().to_string(), + "module", + )); } Ok(resolved) @@ -230,15 +229,13 @@ fn is_conditional_exports_main_sugar( is_conditional_sugar = cur_is_conditional_sugar; i += 1; } else if is_conditional_sugar != cur_is_conditional_sugar { - let msg = format!( - "Invalid package config {} while importing {}. - \"exports\" cannot contains some keys starting with \'.\' and some not. - The exports object must either be an object of package subpath keys - or an object of main entry condition name keys only.", - package_json_url.to_file_path().unwrap().display(), - base.as_str() - ); - return Err(generic_error(msg)); + return Err(errors::err_invalid_package_config( + &package_json_url.to_file_path().unwrap().display().to_string(), + Some(base.as_str().to_string()), + Some("\"exports\" cannot contains some keys starting with \'.\' and some not. + The exports object must either be an object of package subpath keys + or an object of main entry condition name keys only.".to_string()) + )); } } @@ -333,6 +330,11 @@ fn resolve_package_target( } else if let Some(target_obj) = target.as_object() { for key in target_obj.keys() { // TODO(bartlomieju): verify that keys are not numeric + // return Err(errors::err_invalid_package_config( + // package_json_url.to_file_path().unwrap().display().unwrap(), + // Some(base.as_str().to_string()), + // Some("\"exports\" cannot contain numeric property keys.".to_string()), + // )); if key == "default" || conditions.contains(&key.as_str()) { let condition_target = target_obj.get(key).unwrap().to_owned(); @@ -483,7 +485,17 @@ fn package_resolve( .map_err(AnyError::from); } - Err(generic_error(format!("module not found {}", specifier))) + Err(errors::err_module_not_found( + &package_json_url + .join(".") + .unwrap() + .to_file_path() + .unwrap() + .display() + .to_string(), + &base.to_file_path().unwrap().display().to_string(), + "package", + )) } fn parse_package_name( @@ -519,11 +531,11 @@ fn parse_package_name( } if !valid_package_name { - return Err(generic_error(format!( - "{} is not a valid package name {}", + return Err(errors::err_invalid_module_specifier( specifier, - base.to_file_path().unwrap().display() - ))); + "is not a valid package name", + Some(base.to_file_path().unwrap().display().to_string()), + )); } let package_subpath = if let Some(index) = separator_index { @@ -574,19 +586,21 @@ fn get_package_config( return Ok(package_config); } - let package_json: Value = serde_json::from_str(&source).map_err(|_err| { - let mut msg = format!("Invalid package config {}", path.display()); - - if let Some(base) = maybe_base { - msg = format!( - "{} \"{}\" from {}", - msg, + let package_json: Value = serde_json::from_str(&source).map_err(|err| { + let base_msg = if let Some(base) = maybe_base { + Some(format!( + "\"{}\" from {}", specifier, base.to_file_path().unwrap().display() - ); - } - - generic_error(msg) + )) + } else { + None + }; + errors::err_invalid_package_config( + &path.display().to_string(), + base_msg, + Some(err.to_string()), + ) })?; let imports_val = package_json.get("imports"); From be08d0934d32c42853a32b8ce4713952b63cf49b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 19:40:07 +0200 Subject: [PATCH 17/40] lint --- cli/compat/node_module_loader.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cli/compat/node_module_loader.rs b/cli/compat/node_module_loader.rs index 34aed929cbc3d3..dff41f89944f22 100644 --- a/cli/compat/node_module_loader.rs +++ b/cli/compat/node_module_loader.rs @@ -587,15 +587,13 @@ fn get_package_config( } let package_json: Value = serde_json::from_str(&source).map_err(|err| { - let base_msg = if let Some(base) = maybe_base { - Some(format!( + let base_msg = maybe_base.map(|base| { + format!( "\"{}\" from {}", specifier, base.to_file_path().unwrap().display() - )) - } else { - None - }; + ) + }); errors::err_invalid_package_config( &path.display().to_string(), base_msg, From fe5460f3f7e4c1608992dae8122581dd4ce54dd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 21:11:30 +0200 Subject: [PATCH 18/40] revert pub modifiers, rename module --- cli/compat/{node_module_loader.rs => esm_resolver.rs} | 0 cli/compat/mod.rs | 8 ++++---- 2 files changed, 4 insertions(+), 4 deletions(-) rename cli/compat/{node_module_loader.rs => esm_resolver.rs} (100%) diff --git a/cli/compat/node_module_loader.rs b/cli/compat/esm_resolver.rs similarity index 100% rename from cli/compat/node_module_loader.rs rename to cli/compat/esm_resolver.rs diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 60de72944b229d..139bce025b3afc 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -1,20 +1,20 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. mod errors; -mod node_module_loader; +mod esm_resolver; use deno_core::url::Url; -pub use node_module_loader::NodeEsmResolver; +pub use esm_resolver::NodeEsmResolver; // TODO(bartlomieju): this needs to be bumped manually for // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded -pub static STD_URL: &str = "https://deno.land/std@0.111.0/"; +static STD_URL: &str = "https://deno.land/std@0.111.0/"; static GLOBAL_MODULE: &str = "global.ts"; -pub static SUPPORTED_MODULES: &[&str] = &[ +static SUPPORTED_MODULES: &[&str] = &[ "assert", "assert/strict", "async_hooks", From ef6f6d33047ad561a60610e2b94fe15c2ae5191f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 21:47:44 +0200 Subject: [PATCH 19/40] fix tests --- cli/proc_state.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 0b4938d50bd710..ec69611e652d3b 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -299,6 +299,15 @@ impl ProcState { } else { import_map_resolver.as_ref().map(|im| im.as_resolver()) }; + // TODO(bartlomieju): this is very make-shift, is there an existing API + // that we could include it like with "maybe_imports"? + let roots = if self.flags.compat { + let mut r = vec![compat::GLOBAL_URL.clone()]; + r.extend(roots); + r + } else { + roots + }; let graph = deno_graph::create_graph( roots, is_dynamic, From 28efb3e6ad6b248bcd702c0428e8aa4c45099908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 13 Oct 2021 22:14:06 +0200 Subject: [PATCH 20/40] add helpers --- cli/compat/esm_resolver.rs | 42 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index dff41f89944f22..42b5c11b02c71d 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -98,6 +98,16 @@ fn node_resolve( Ok(url) } +fn to_file_path(url: &Url) -> PathBuf { + url + .to_file_path() + .expect("Provided URL was not file:// URL") +} + +fn to_file_path_string(url: &Url) -> String { + to_file_path(url).display().to_string() +} + fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { if specifier.is_empty() { return false; @@ -161,11 +171,11 @@ fn finalize_resolution( return Err(errors::err_invalid_module_specifier( resolved.path(), "must not include encoded \"/\" or \"\\\\\" characters", - Some(base.to_file_path().unwrap().display().to_string()), + Some(to_file_path_string(base)), )); } - let path = resolved.to_file_path().unwrap(); + let path = to_file_path(&resolved); // TODO(bartlomieju): currently not supported // if (getOptionValue('--experimental-specifier-resolution') === 'node') { @@ -187,12 +197,12 @@ fn finalize_resolution( if is_dir { return Err(errors::err_unsupported_dir_import( &path.display().to_string(), - &base.to_file_path().unwrap().display().to_string(), + &to_file_path_string(base), )); } else if !is_file { return Err(errors::err_module_not_found( &path.display().to_string(), - &base.to_file_path().unwrap().display().to_string(), + &to_file_path_string(base), "module", )); } @@ -230,7 +240,7 @@ fn is_conditional_exports_main_sugar( i += 1; } else if is_conditional_sugar != cur_is_conditional_sugar { return Err(errors::err_invalid_package_config( - &package_json_url.to_file_path().unwrap().display().to_string(), + &to_file_path_string(package_json_url), Some(base.as_str().to_string()), Some("\"exports\" cannot contains some keys starting with \'.\' and some not. The exports object must either be an object of package subpath keys @@ -331,7 +341,7 @@ fn resolve_package_target( for key in target_obj.keys() { // TODO(bartlomieju): verify that keys are not numeric // return Err(errors::err_invalid_package_config( - // package_json_url.to_file_path().unwrap().display().unwrap(), + // to_file_path_string(package_json_url), // Some(base.as_str().to_string()), // Some("\"exports\" cannot contain numeric property keys.".to_string()), // )); @@ -434,7 +444,7 @@ fn package_resolve( let mut package_json_url = base.join(&format!("./node_modules/{}/package.json", package_name))?; - let mut package_json_path = package_json_url.to_file_path().unwrap(); + let mut package_json_path = to_file_path(&package_json_url); let mut last_path; loop { let p_str = package_json_path.to_str().unwrap(); @@ -454,7 +464,7 @@ fn package_resolve( }; package_json_url = package_json_url .join(&format!("{}{}/package.json", prefix, package_name))?; - package_json_path = package_json_url.to_file_path().unwrap(); + package_json_path = to_file_path(&package_json_url); if package_json_path.to_str().unwrap().len() == last_path.to_str().unwrap().len() { @@ -493,7 +503,7 @@ fn package_resolve( .unwrap() .display() .to_string(), - &base.to_file_path().unwrap().display().to_string(), + &to_file_path_string(base), "package", )) } @@ -534,7 +544,7 @@ fn parse_package_name( return Err(errors::err_invalid_module_specifier( specifier, "is not a valid package name", - Some(base.to_file_path().unwrap().display().to_string()), + Some(to_file_path_string(base)), )); } @@ -588,11 +598,7 @@ fn get_package_config( let package_json: Value = serde_json::from_str(&source).map_err(|err| { let base_msg = maybe_base.map(|base| { - format!( - "\"{}\" from {}", - specifier, - base.to_file_path().unwrap().display() - ) + format!("\"{}\" from {}", specifier, to_file_path(base).display()) }); errors::err_invalid_package_config( &path.display().to_string(), @@ -666,7 +672,7 @@ fn get_package_scope_config( } let package_config = get_package_config( - package_json_url.to_file_path().unwrap(), + to_file_path(&package_json_url), resolved.as_str(), None, )?; @@ -686,7 +692,7 @@ fn get_package_scope_config( } } - let package_json_path = package_json_url.to_file_path().unwrap(); + let package_json_path = to_file_path(&package_json_url); let package_config = PackageConfig { pjsonpath: package_json_path, exists: false, @@ -704,7 +710,7 @@ fn get_package_scope_config( } fn file_exists(path_url: &Url) -> bool { - if let Ok(stats) = std::fs::metadata(path_url.to_file_path().unwrap()) { + if let Ok(stats) = std::fs::metadata(to_file_path(path_url)) { stats.is_file() } else { false From 13079df8cf78b1ec617d29107cf0d188c9dc5fef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Oct 2021 13:18:47 +0200 Subject: [PATCH 21/40] finish resolve_package_target_string --- cli/compat/errors.rs | 27 ++++++++++ cli/compat/esm_resolver.rs | 100 ++++++++++++++++++++++++++++++++----- 2 files changed, 114 insertions(+), 13 deletions(-) diff --git a/cli/compat/errors.rs b/cli/compat/errors.rs index 108414e8e66c7e..93beb92ed40310 100644 --- a/cli/compat/errors.rs +++ b/cli/compat/errors.rs @@ -73,3 +73,30 @@ pub(crate) fn err_unsupported_esm_url_scheme(url: &Url) -> AnyError { msg = format!("{}. Received protocol '{}'", msg, url.scheme()); generic_error(msg) } + +pub(crate) fn err_invalid_package_target( + pkg_path: String, + key: String, + target: String, + is_import: bool, + maybe_base: Option, +) -> AnyError { + let rel_error = !is_import && !target.is_empty() && !target.starts_with("./"); + + let mut msg = if key == "." { + assert!(!is_import); + format!("Invalid \"exports\" main target {} defined in the package config {}package.json", target, pkg_path) + } else { + let ie = if is_import { "imports" } else { "exports" }; + format!("Invalid \"{}\" target {} defined for '{}' in the package config {}package.json", ie, target, key, pkg_path) + }; + + if let Some(base) = maybe_base { + msg = format!("{} imported from {}", msg, base); + }; + if rel_error { + msg = format!("{}; target must start with \"./\"", msg); + } + + generic_error(msg) +} diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index 42b5c11b02c71d..e8dfd3c717cef0 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -252,34 +252,97 @@ fn is_conditional_exports_main_sugar( Ok(is_conditional_sugar) } +fn invalid_package_target( + subpath: String, + target: String, + package_json_url: &Url, + internal: bool, + base: &Url, +) -> AnyError { + errors::err_invalid_package_target( + to_file_path_string(&package_json_url.join(".").unwrap()), + subpath, + target, + internal, + Some(base.as_str().to_string()), + ) +} + +fn invalid_subpath( + subpath: String, + package_json_url: &Url, + internal: bool, + base: &Url, +) -> AnyError { + let ie = if internal { "imports" } else { "exports" }; + let reason = format!( + "request is not a valid subpath for the \"{}\" resolution of {}", + ie, + to_file_path_string(package_json_url) + ); + errors::err_invalid_module_specifier( + &subpath, + &reason, + Some(to_file_path_string(base)), + ) +} + #[allow(clippy::too_many_arguments)] fn resolve_package_target_string( target: String, subpath: String, - _match_: String, + match_: String, package_json_url: Url, - _base: &ModuleSpecifier, + base: &ModuleSpecifier, pattern: bool, internal: bool, - _conditions: &[&str], + conditions: &[&str], ) -> Result { if !subpath.is_empty() && !pattern && !target.ends_with('/') { - todo!() + return Err(invalid_package_target( + match_, + target, + &package_json_url, + internal, + base, + )); } + let invalid_segment_re = + Regex::new(r"(^|\|/)(..?|node_modules)(\|/|$)").expect("bad regex"); + let pattern_re = Regex::new(r"\*").expect("bad regex"); + if !target.starts_with("./") { if internal && !target.starts_with("../") && !target.starts_with('/') { - todo!() + let is_url = Url::parse(&target).is_ok(); + if !is_url { + let export_target = if pattern { + pattern_re + .replace(&target, |_caps: ®ex::Captures| subpath.clone()) + .to_string() + } else { + format!("{}{}", target, subpath) + }; + return package_resolve(&export_target, &package_json_url, conditions); + } } - todo!() + return Err(invalid_package_target( + match_, + target, + &package_json_url, + internal, + base, + )); } - let invalid_segment_re = - Regex::new(r"(^|\|/)(..?|node_modules)(\|/|$)").expect("bad regex"); - let pattern_re = Regex::new(r"\*").expect("bad regex"); - if invalid_segment_re.is_match(&target[2..]) { - todo!() + return Err(invalid_package_target( + match_, + target, + &package_json_url, + internal, + base, + )); } let resolved = package_json_url.join(&target)?; @@ -288,7 +351,13 @@ fn resolve_package_target_string( let package_path = package_url.path(); if !resolved_path.starts_with(package_path) { - todo!() + return Err(invalid_package_target( + match_, + target, + &package_json_url, + internal, + base, + )); } if subpath.is_empty() { @@ -296,7 +365,12 @@ fn resolve_package_target_string( } if invalid_segment_re.is_match(&subpath) { - todo!() + let request = if pattern { + match_.replace("*", &subpath) + } else { + format!("{}{}", match_, subpath) + }; + return Err(invalid_subpath(request, &package_json_url, internal, base)); } if pattern { From 57f3323468d1f09a69fe9fee744a65d1d3a8a0ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Oct 2021 13:44:24 +0200 Subject: [PATCH 22/40] err_package_not_exported --- cli/compat/errors.rs | 30 +++++++++++-- cli/compat/esm_resolver.rs | 87 +++++++++++++++++++++++++++++++------- 2 files changed, 99 insertions(+), 18 deletions(-) diff --git a/cli/compat/errors.rs b/cli/compat/errors.rs index 93beb92ed40310..050f1c2e120520 100644 --- a/cli/compat/errors.rs +++ b/cli/compat/errors.rs @@ -82,13 +82,14 @@ pub(crate) fn err_invalid_package_target( maybe_base: Option, ) -> AnyError { let rel_error = !is_import && !target.is_empty() && !target.starts_with("./"); + let mut msg = "[ERR_INVALID_PACKAGE_TARGET]".to_string(); - let mut msg = if key == "." { + if key == "." { assert!(!is_import); - format!("Invalid \"exports\" main target {} defined in the package config {}package.json", target, pkg_path) + msg = format!("{} Invalid \"exports\" main target {} defined in the package config {}package.json", msg, target, pkg_path) } else { let ie = if is_import { "imports" } else { "exports" }; - format!("Invalid \"{}\" target {} defined for '{}' in the package config {}package.json", ie, target, key, pkg_path) + msg = format!("{} Invalid \"{}\" target {} defined for '{}' in the package config {}package.json", msg, ie, target, key, pkg_path) }; if let Some(base) = maybe_base { @@ -100,3 +101,26 @@ pub(crate) fn err_invalid_package_target( generic_error(msg) } + +pub(crate) fn err_package_path_not_exported( + pkg_path: String, + subpath: String, + maybe_base: Option, +) -> AnyError { + let mut msg = "[ERR_PACKAGE_PATH_NOT_EXPORTED]".to_string(); + + if subpath == "." { + msg = format!( + "{} No \"exports\" main defined in {}package.json", + msg, pkg_path + ); + } else { + msg = format!("{} Package subpath \'{}\' is not defined by \"exports\" in {}package.json", msg, subpath, pkg_path); + }; + + if let Some(base) = maybe_base { + msg = format!("{} imported from {}", msg, base); + } + + generic_error(msg) +} diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index e8dfd3c717cef0..9e03b4565de86b 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -252,7 +252,7 @@ fn is_conditional_exports_main_sugar( Ok(is_conditional_sugar) } -fn invalid_package_target( +fn throw_invalid_package_target( subpath: String, target: String, package_json_url: &Url, @@ -268,7 +268,7 @@ fn invalid_package_target( ) } -fn invalid_subpath( +fn throw_invalid_subpath( subpath: String, package_json_url: &Url, internal: bool, @@ -299,7 +299,7 @@ fn resolve_package_target_string( conditions: &[&str], ) -> Result { if !subpath.is_empty() && !pattern && !target.ends_with('/') { - return Err(invalid_package_target( + return Err(throw_invalid_package_target( match_, target, &package_json_url, @@ -326,7 +326,7 @@ fn resolve_package_target_string( return package_resolve(&export_target, &package_json_url, conditions); } } - return Err(invalid_package_target( + return Err(throw_invalid_package_target( match_, target, &package_json_url, @@ -336,7 +336,7 @@ fn resolve_package_target_string( } if invalid_segment_re.is_match(&target[2..]) { - return Err(invalid_package_target( + return Err(throw_invalid_package_target( match_, target, &package_json_url, @@ -351,7 +351,7 @@ fn resolve_package_target_string( let package_path = package_url.path(); if !resolved_path.starts_with(package_path) { - return Err(invalid_package_target( + return Err(throw_invalid_package_target( match_, target, &package_json_url, @@ -370,7 +370,12 @@ fn resolve_package_target_string( } else { format!("{}{}", match_, subpath) }; - return Err(invalid_subpath(request, &package_json_url, internal, base)); + return Err(throw_invalid_subpath( + request, + &package_json_url, + internal, + base, + )); } if pattern { @@ -407,10 +412,41 @@ fn resolve_package_target( )?)); } else if let Some(target_arr) = target.as_array() { if target_arr.is_empty() { - todo!() + return Ok(None); } - todo!() + let mut last_error = None; + for target_item in target_arr { + let resolved_result = resolve_package_target( + package_json_url.clone(), + target_item.to_owned(), + subpath.clone(), + package_subpath.clone(), + base, + pattern, + internal, + conditions, + ); + + if let Err(e) = resolved_result { + let err_string = e.to_string(); + last_error = Some(e); + if err_string.starts_with("[ERR_INVALID_PACKAGE_TARGET]") { + continue; + } + return Err(last_error.unwrap()); + } + let resolved = resolved_result.unwrap(); + if resolved.is_none() { + last_error = None; + continue; + } + return Ok(resolved); + } + if last_error.is_none() { + return Ok(None); + } + return Err(last_error.unwrap()); } else if let Some(target_obj) = target.as_object() { for key in target_obj.keys() { // TODO(bartlomieju): verify that keys are not numeric @@ -439,10 +475,28 @@ fn resolve_package_target( } } } else if target.is_null() { - todo!() + return Ok(None); } - todo!() + Err(throw_invalid_package_target( + package_subpath, + target.to_string(), + &package_json_url, + internal, + base, + )) +} + +fn throw_exports_not_found( + subpath: String, + package_json_url: &Url, + base: &Url, +) -> AnyError { + errors::err_package_path_not_exported( + to_file_path_string(&package_json_url.join(".").unwrap()), + subpath, + Some(to_file_path_string(base)), + ) } fn package_exports_resolve( @@ -469,18 +523,21 @@ fn package_exports_resolve( { let target = exports_map.get(&package_subpath).unwrap().to_owned(); let resolved = resolve_package_target( - package_json_url, + package_json_url.clone(), target, "".to_string(), - package_subpath, + package_subpath.to_string(), base, false, false, conditions, )?; - // TODO(bartlomieju): return error here if resolved.is_none() { - todo!() + return Err(throw_exports_not_found( + package_subpath, + &package_json_url, + base, + )); } return Ok(resolved.unwrap()); } From 8e0a80bfa444b7e188cebc767a34f74ef2adf6f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Oct 2021 14:45:38 +0200 Subject: [PATCH 23/40] add missing methods --- cli/compat/errors.rs | 19 ++++ cli/compat/esm_resolver.rs | 199 ++++++++++++++++++++++++++++++++++++- 2 files changed, 213 insertions(+), 5 deletions(-) diff --git a/cli/compat/errors.rs b/cli/compat/errors.rs index 050f1c2e120520..73da64e077169a 100644 --- a/cli/compat/errors.rs +++ b/cli/compat/errors.rs @@ -124,3 +124,22 @@ pub(crate) fn err_package_path_not_exported( generic_error(msg) } + +pub(crate) fn err_package_import_not_defined( + specifier: &str, + package_path: Option, + base: &str, +) -> AnyError { + let mut msg = format!( + "Package import specifier \"{}\" is not defined in", + specifier + ); + + if let Some(package_path) = package_path { + msg = format!("{} in package {}package.json", msg, package_path); + } + + msg = format!("{} imported from {}", msg, base); + + type_error(msg) +} diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index 9e03b4565de86b..852e29960743e6 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -145,7 +145,7 @@ fn module_resolve( let resolved = if should_be_treated_as_relative_or_absolute_path(specifier) { base.join(specifier)? } else if specifier.starts_with('#') { - package_imports_resolve(specifier, base)? + package_imports_resolve(specifier, base, conditions)? } else if let Ok(resolved) = Url::parse(specifier) { resolved } else { @@ -210,11 +210,140 @@ fn finalize_resolution( Ok(resolved) } +fn throw_import_not_defined( + specifier: &str, + package_json_url: Option, + base: &Url, +) -> AnyError { + errors::err_package_import_not_defined( + specifier, + package_json_url.map(|u| to_file_path_string(&u.join(".").unwrap())), + &to_file_path_string(base), + ) +} + +fn pattern_key_compare(a: &str, b: &str) -> i32 { + let a_pattern_index = a.find('*'); + let b_pattern_index = b.find('*'); + + let base_len_a = if let Some(index) = a_pattern_index { + index + 1 + } else { + a.len() + }; + let base_len_b = if let Some(index) = b_pattern_index { + index + 1 + } else { + b.len() + }; + + if base_len_a > base_len_b { + return -1; + } + + if base_len_b > base_len_a { + return 1; + } + + if a_pattern_index.is_none() { + return 1; + } + + if b_pattern_index.is_none() { + return -1; + } + + if a.len() > b.len() { + return -1; + } + + if b.len() > a.len() { + return 1; + } + + 0 +} + fn package_imports_resolve( - _specifier: &str, - _base: &ModuleSpecifier, + name: &str, + base: &ModuleSpecifier, + conditions: &[&str], ) -> Result { - todo!() + if name == "#" || name.starts_with("#/") || name.ends_with('/') { + let reason = "is not a valid internal imports specifier name"; + return Err(errors::err_invalid_module_specifier( + name, + reason, + Some(to_file_path_string(base)), + )); + } + + let mut package_json_url = None; + + let package_config = get_package_scope_config(base)?; + if package_config.exists { + package_json_url = + Some(Url::from_file_path(package_config.pjsonpath).unwrap()); + if let Some(imports) = &package_config.imports { + if imports.contains_key(name) && !name.contains('*') { + let maybe_resolved = resolve_package_target( + package_json_url.clone().unwrap(), + imports.get(name).unwrap().to_owned(), + "".to_string(), + name.to_string(), + base, + false, + true, + conditions, + )?; + if let Some(resolved) = maybe_resolved { + return Ok(resolved); + } + } else { + let mut best_match = ""; + let mut best_match_subpath = None; + for key in imports.keys() { + let pattern_index = key.find('*'); + if let Some(pattern_index) = pattern_index { + let key_sub = &key[0..=pattern_index]; + if name.starts_with(key_sub) { + let pattern_trailer = &key[pattern_index + 1..]; + if name.len() > key.len() + && name.ends_with(&pattern_trailer) + && pattern_key_compare(best_match, key) == 1 + && key.rfind('*') == Some(pattern_index) + { + best_match = key; + best_match_subpath = Some( + name[pattern_index..=(name.len() - pattern_trailer.len())] + .to_string(), + ); + } + } + } + } + + if !best_match.is_empty() { + let target = imports.get(best_match).unwrap().to_owned(); + let maybe_resolved = resolve_package_target( + package_json_url.clone().unwrap(), + target, + best_match_subpath.unwrap(), + best_match.to_string(), + base, + true, + true, + conditions, + )?; + if let Some(resolved) = maybe_resolved { + return Ok(resolved); + } + } + } + } + } + + Err(throw_import_not_defined(name, package_json_url, base)) } fn is_conditional_exports_main_sugar( @@ -542,7 +671,67 @@ fn package_exports_resolve( return Ok(resolved.unwrap()); } - todo!() + let mut best_match = ""; + let mut best_match_subpath = None; + for key in exports_map.keys() { + let pattern_index = key.find('*'); + if let Some(pattern_index) = pattern_index { + let key_sub = &key[0..=pattern_index]; + if package_subpath.starts_with(key_sub) { + // When this reaches EOL, this can throw at the top of the whole function: + // + // if (StringPrototypeEndsWith(packageSubpath, '/')) + // throwInvalidSubpath(packageSubpath) + // + // To match "imports" and the spec. + if package_subpath.ends_with('/') { + // emitTrailingSlashPatternDeprecation(); + } + let pattern_trailer = &key[pattern_index + 1..]; + if package_subpath.len() > key.len() + && package_subpath.ends_with(&pattern_trailer) + && pattern_key_compare(best_match, key) == 1 + && key.rfind('*') == Some(pattern_index) + { + best_match = key; + best_match_subpath = Some( + package_subpath + [pattern_index..=(package_subpath.len() - pattern_trailer.len())] + .to_string(), + ); + } + } + } + } + + if !best_match.is_empty() { + let target = exports.get(best_match).unwrap().to_owned(); + let maybe_resolved = resolve_package_target( + package_json_url.clone(), + target, + best_match_subpath.unwrap(), + best_match.to_string(), + base, + true, + false, + conditions, + )?; + if let Some(resolved) = maybe_resolved { + return Ok(resolved); + } else { + return Err(throw_exports_not_found( + package_subpath, + &package_json_url, + base, + )); + } + } + + Err(throw_exports_not_found( + package_subpath, + &package_json_url, + base, + )) } fn package_resolve( From efdfd6ae6d6579fccb4ae3892f607c88c5427950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Oct 2021 15:18:34 +0200 Subject: [PATCH 24/40] add error code --- cli/compat/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/compat/errors.rs b/cli/compat/errors.rs index 73da64e077169a..3a44b2c88fdc61 100644 --- a/cli/compat/errors.rs +++ b/cli/compat/errors.rs @@ -131,7 +131,7 @@ pub(crate) fn err_package_import_not_defined( base: &str, ) -> AnyError { let mut msg = format!( - "Package import specifier \"{}\" is not defined in", + "[ERR_PACKAGE_IMPORT_NOT_DEFINED] Package import specifier \"{}\" is not defined in", specifier ); From 3696f952a40d55c3644a847c593c45d7788f2936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 14 Oct 2021 16:16:17 +0200 Subject: [PATCH 25/40] reset CI From a12f3e943b7b375898aeba04ca72e957827e3860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Oct 2021 00:39:00 +0200 Subject: [PATCH 26/40] add CJS resolver --- cli/main.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/cli/main.rs b/cli/main.rs index 8a85cacfbc44c1..ad5774c4621aa0 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -1114,10 +1114,49 @@ async fn run_command( }; debug!("main_module {}", main_module); + let mut use_esm_loader = true; if flags.compat { + assert_eq!(main_module.scheme(), "file"); worker.execute_side_module(&compat::GLOBAL_URL).await?; + // Decide if we're running with Node ESM loader or CJS loader. + let result = worker.js_runtime.execute_script( + &located_script_name!(), + &format!( + r#"(async function checkIfEsm(main) {{ + const {{ resolveMainPath, shouldUseESMLoader }} = await import("file:///Users/biwanczuk/dev/deno_std/node/module.ts"); + const resolvedMain = resolveMainPath(main); + const useESMLoader = shouldUseESMLoader(resolvedMain); + return useESMLoader; + }})('{}');"#, + main_module.to_file_path().unwrap().display().to_string() + ), + )?; + let use_esm_loader_global = worker.js_runtime.resolve_value(result).await?; + use_esm_loader = { + let scope = &mut worker.js_runtime.handle_scope(); + let ues_esm_loader_local = use_esm_loader_global.get(scope); + ues_esm_loader_local.boolean_value(scope) + }; + if use_esm_loader { + eprintln!("Using compat ESM resolver"); + } else { + eprintln!("Using compat CJS resolver"); + } + } + if use_esm_loader { + worker.execute_main_module(&main_module).await?; + } else { + worker.execute_script( + &located_script_name!(), + &format!( + r#"(async function loadCjsModule(main) {{ + const Module = await import("file:///Users/biwanczuk/dev/deno_std/node/module.ts"); + Module.default._load(main, null, true); + }})('{}');"#, + main_module.to_file_path().unwrap().display().to_string() + ), + )?; } - worker.execute_main_module(&main_module).await?; worker.execute_script( &located_script_name!(), "window.dispatchEvent(new Event('load'))", From adb3fc40fa32cdaf2771ba209fea51fe308fa9ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Oct 2021 02:42:29 +0200 Subject: [PATCH 27/40] reorganize code --- cli/compat/esm_resolver.rs | 16 ++++------ cli/compat/mod.rs | 47 ++++++++++++++++++++++++++++- cli/main.rs | 61 ++++++++++++++++---------------------- 3 files changed, 78 insertions(+), 46 deletions(-) diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index 852e29960743e6..1207a7b483a944 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -27,13 +27,6 @@ impl Resolver for NodeEsmResolver { specifier: &str, referrer: &ModuleSpecifier, ) -> Result { - // TODO(bartlomieju): this is hacky, remove - // needed to add it here because `deno_std/node` has - // triple-slash references and they should still resolve - // the regular way (I think) - if referrer.as_str().starts_with("https://deno.land/std") { - return referrer.join(specifier).map_err(AnyError::from); - } node_resolve(specifier, referrer.as_str(), &std::env::current_dir()?) } } @@ -158,9 +151,12 @@ fn finalize_resolution( resolved: ModuleSpecifier, base: &ModuleSpecifier, ) -> Result { - // TODO(bartlomieju): this is not part of Node resolution - // (as it doesn't support http/https); - // but I had to short circuit for remote modules to avoid errors + // TODO(bartlomieju): this is not part of Node resolution algorithm + // (as it doesn't support http/https); but I had to short circuit here + // for remote modules because they are mainly used to polyfill `node` built + // in modules. Another option would be to leave the resolved URLs + // as `node:` and do the actual remapping to std's polyfill + // in module loader. I'm not sure which approach is better. if resolved.scheme().starts_with("http") { return Ok(resolved); } diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 139bce025b3afc..25f8535cc0cc36 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -3,7 +3,10 @@ mod errors; mod esm_resolver; +use deno_core::error::AnyError; +use deno_core::located_script_name; use deno_core::url::Url; +use deno_core::JsRuntime; pub use esm_resolver::NodeEsmResolver; @@ -11,7 +14,7 @@ pub use esm_resolver::NodeEsmResolver; // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded -static STD_URL: &str = "https://deno.land/std@0.111.0/"; +static STD_URL: &str = "file:///Users/biwanczuk/dev/deno_std/"; static GLOBAL_MODULE: &str = "global.ts"; static SUPPORTED_MODULES: &[&str] = &[ @@ -78,3 +81,45 @@ pub(crate) fn try_resolve_builtin_module(specifier: &str) -> Option { None } } + +pub(crate) async fn check_if_should_use_esm_loader( + js_runtime: &mut JsRuntime, + main_module: &str, +) -> Result { + // Decide if we're running with Node ESM loader or CJS loader. + let source_code = &format!( + r#"(async function checkIfEsm(main) {{ + const {{ resolveMainPath, shouldUseESMLoader }} = await import("{}node/module.ts"); + const resolvedMain = resolveMainPath(main); + const useESMLoader = shouldUseESMLoader(resolvedMain); + return useESMLoader; + }})('{}');"#, + STD_URL, main_module + ); + let result = + js_runtime.execute_script(&located_script_name!(), source_code)?; + let use_esm_loader_global = js_runtime.resolve_value(result).await?; + let use_esm_loader = { + let scope = &mut js_runtime.handle_scope(); + let use_esm_loader_local = use_esm_loader_global.get(scope); + use_esm_loader_local.boolean_value(scope) + }; + + Ok(use_esm_loader) +} + +pub(crate) fn load_cjs_module( + js_runtime: &mut JsRuntime, + main_module: &str, +) -> Result<(), AnyError> { + let source_code = &format!( + r#"(async function loadCjsModule(main) {{ + const Module = await import("{}node/module.ts"); + Module.default._load(main, null, true); + }})('{}');"#, + STD_URL, main_module, + ); + + js_runtime.execute_script(&located_script_name!(), source_code)?; + Ok(()) +} diff --git a/cli/main.rs b/cli/main.rs index ad5774c4621aa0..dee1f64aff9c75 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -1092,6 +1092,11 @@ async fn run_command( return run_with_watch(flags, run_flags.script).await; } + // TODO(bartlomieju): it should not be resolved here if we're in compat mode + // because it might be a bare specifier + // TODO(bartlomieju): actually I think it will also fail if there's an import + // map specified and bare specifier is used on the command line - this should + // probably call `ProcState::resolve` instead let main_module = resolve_url_or_path(&run_flags.script)?; let ps = ProcState::build(flags.clone()).await?; let permissions = Permissions::from_options(&flags.clone().into()); @@ -1114,49 +1119,35 @@ async fn run_command( }; debug!("main_module {}", main_module); - let mut use_esm_loader = true; + if flags.compat { + // TODO(bartlomieju): fix me assert_eq!(main_module.scheme(), "file"); + + // Set up Node globals worker.execute_side_module(&compat::GLOBAL_URL).await?; - // Decide if we're running with Node ESM loader or CJS loader. - let result = worker.js_runtime.execute_script( - &located_script_name!(), - &format!( - r#"(async function checkIfEsm(main) {{ - const {{ resolveMainPath, shouldUseESMLoader }} = await import("file:///Users/biwanczuk/dev/deno_std/node/module.ts"); - const resolvedMain = resolveMainPath(main); - const useESMLoader = shouldUseESMLoader(resolvedMain); - return useESMLoader; - }})('{}');"#, - main_module.to_file_path().unwrap().display().to_string() - ), - )?; - let use_esm_loader_global = worker.js_runtime.resolve_value(result).await?; - use_esm_loader = { - let scope = &mut worker.js_runtime.handle_scope(); - let ues_esm_loader_local = use_esm_loader_global.get(scope); - ues_esm_loader_local.boolean_value(scope) - }; + + let use_esm_loader = compat::check_if_should_use_esm_loader( + &mut worker.js_runtime, + &main_module.to_file_path().unwrap().display().to_string(), + ) + .await?; + if use_esm_loader { - eprintln!("Using compat ESM resolver"); + // ES module execution in Node compatiblity mode + worker.execute_main_module(&main_module).await?; } else { - eprintln!("Using compat CJS resolver"); + // CJS module execution in Node compatiblity mode + compat::load_cjs_module( + &mut worker.js_runtime, + &main_module.to_file_path().unwrap().display().to_string(), + )?; } - } - if use_esm_loader { - worker.execute_main_module(&main_module).await?; } else { - worker.execute_script( - &located_script_name!(), - &format!( - r#"(async function loadCjsModule(main) {{ - const Module = await import("file:///Users/biwanczuk/dev/deno_std/node/module.ts"); - Module.default._load(main, null, true); - }})('{}');"#, - main_module.to_file_path().unwrap().display().to_string() - ), - )?; + // Regular ES module execution + worker.execute_main_module(&main_module).await?; } + worker.execute_script( &located_script_name!(), "window.dispatchEvent(new Event('load'))", From f2104faaa4a4c15b1668da09f35bc54512c8e0b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Oct 2021 17:04:17 +0200 Subject: [PATCH 28/40] add deno condition --- cli/compat/esm_resolver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index 1207a7b483a944..e373ba9f56ba2c 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -31,7 +31,7 @@ impl Resolver for NodeEsmResolver { } } -static DEFAULT_CONDITIONS: &[&str] = &["node", "import"]; +static DEFAULT_CONDITIONS: &[&str] = &["deno", "node", "import"]; /// This function is an implementation of `defaultResolve` in /// `lib/internal/modules/esm/resolve.js` from Node. From c1c49d0342bb3fbf31a9a7886d6ca0a2f06d99d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Oct 2021 17:18:02 +0200 Subject: [PATCH 29/40] use raw.githubusercontent.com --- cli/compat/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 25f8535cc0cc36..90bc2fe0a9b723 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -14,7 +14,7 @@ pub use esm_resolver::NodeEsmResolver; // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded -static STD_URL: &str = "file:///Users/biwanczuk/dev/deno_std/"; +static STD_URL: &str = "https://raw.githubusercontent.com/denoland/deno_std/acd70dced5629ed4e20ac464bdb7d498f3c51d83/"; static GLOBAL_MODULE: &str = "global.ts"; static SUPPORTED_MODULES: &[&str] = &[ From 4c855f27b18d632c45fbf862b5edb10ddbd4c4ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 18 Oct 2021 01:31:21 +0200 Subject: [PATCH 30/40] don't require --allow-net permission for compat mode --- cli/compat/mod.rs | 19 ++++++++++++------- cli/main.rs | 6 ++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 90bc2fe0a9b723..dd99cd59a0164b 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -14,8 +14,9 @@ pub use esm_resolver::NodeEsmResolver; // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded -static STD_URL: &str = "https://raw.githubusercontent.com/denoland/deno_std/acd70dced5629ed4e20ac464bdb7d498f3c51d83/"; +static STD_URL_STR: &str = "https://raw.githubusercontent.com/denoland/deno_std/acd70dced5629ed4e20ac464bdb7d498f3c51d83/"; static GLOBAL_MODULE: &str = "global.ts"; +static MODULE_MODULE: &str = "module.ts"; static SUPPORTED_MODULES: &[&str] = &[ "assert", @@ -63,8 +64,10 @@ static SUPPORTED_MODULES: &[&str] = &[ ]; lazy_static::lazy_static! { - static ref GLOBAL_URL_STR: String = format!("{}node/{}", STD_URL, GLOBAL_MODULE); + static ref GLOBAL_URL_STR: String = format!("{}node/{}", STD_URL_STR, GLOBAL_MODULE); pub(crate) static ref GLOBAL_URL: Url = Url::parse(&GLOBAL_URL_STR).unwrap(); + static ref MODULE_URL_STR: String = format!("{}node/{}", STD_URL_STR, MODULE_MODULE); + pub(crate) static ref MODULE_URL: Url = Url::parse(&MODULE_URL_STR).unwrap(); static ref COMPAT_IMPORT_URL: Url = Url::parse("flags:compat").unwrap(); } @@ -75,7 +78,7 @@ pub(crate) fn get_node_imports() -> Vec<(Url, Vec)> { pub(crate) fn try_resolve_builtin_module(specifier: &str) -> Option { if SUPPORTED_MODULES.contains(&specifier) { - let module_url = format!("{}node/{}.ts", crate::compat::STD_URL, specifier); + let module_url = format!("{}node/{}.ts", STD_URL_STR, specifier); Some(Url::parse(&module_url).unwrap()) } else { None @@ -89,12 +92,13 @@ pub(crate) async fn check_if_should_use_esm_loader( // Decide if we're running with Node ESM loader or CJS loader. let source_code = &format!( r#"(async function checkIfEsm(main) {{ - const {{ resolveMainPath, shouldUseESMLoader }} = await import("{}node/module.ts"); + const {{ resolveMainPath, shouldUseESMLoader }} = await import("{}"); const resolvedMain = resolveMainPath(main); const useESMLoader = shouldUseESMLoader(resolvedMain); return useESMLoader; }})('{}');"#, - STD_URL, main_module + MODULE_URL_STR.as_str(), + main_module ); let result = js_runtime.execute_script(&located_script_name!(), source_code)?; @@ -114,10 +118,11 @@ pub(crate) fn load_cjs_module( ) -> Result<(), AnyError> { let source_code = &format!( r#"(async function loadCjsModule(main) {{ - const Module = await import("{}node/module.ts"); + const Module = await import("{}"); Module.default._load(main, null, true); }})('{}');"#, - STD_URL, main_module, + MODULE_URL_STR.as_str(), + main_module, ); js_runtime.execute_script(&located_script_name!(), source_code)?; diff --git a/cli/main.rs b/cli/main.rs index dee1f64aff9c75..9dd7a354ee080d 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -1126,6 +1126,12 @@ async fn run_command( // Set up Node globals worker.execute_side_module(&compat::GLOBAL_URL).await?; + // And `module` module that we'll use for checking which + // loader to use and potentially load CJS module with. + // This allows to skip permission check for `--allow-net` + // which would otherwise be requested by dynamically importing + // this file. + worker.execute_side_module(&compat::MODULE_URL).await?; let use_esm_loader = compat::check_if_should_use_esm_loader( &mut worker.js_runtime, From 43984e4711e8894a37b7ebe44671c17e4f28bc36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 18 Oct 2021 01:38:21 +0200 Subject: [PATCH 31/40] add test for conditional exports --- cli/compat/esm_resolver.rs | 20 +++++++++++++++++++ cli/compat/testdata/conditions/main.js | 1 + .../imports_exports/import_export.js | 6 ++++++ .../imports_exports/import_polyfill.js | 3 +++ .../node_modules/imports_exports/package.json | 17 ++++++++++++++++ .../imports_exports/require_export.cjs | 6 ++++++ .../imports_exports/require_polyfill.js | 3 +++ cli/compat/testdata/conditions/package.json | 7 +++++++ 8 files changed, 63 insertions(+) create mode 100644 cli/compat/testdata/conditions/main.js create mode 100644 cli/compat/testdata/conditions/node_modules/imports_exports/import_export.js create mode 100644 cli/compat/testdata/conditions/node_modules/imports_exports/import_polyfill.js create mode 100644 cli/compat/testdata/conditions/node_modules/imports_exports/package.json create mode 100644 cli/compat/testdata/conditions/node_modules/imports_exports/require_export.cjs create mode 100644 cli/compat/testdata/conditions/node_modules/imports_exports/require_polyfill.js create mode 100644 cli/compat/testdata/conditions/package.json diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index e373ba9f56ba2c..945972e9edeb68 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -1139,4 +1139,24 @@ mod tests { println!("actual {}", actual); assert_eq!(actual, expected); } + + #[test] + fn conditional_exports() { + // check that `exports` mapping works correctly + let cwd = testdir("conditions"); + let main = Url::from_file_path(cwd.join("main.js")).unwrap(); + let actual = node_resolve("imports_exports", main.as_str(), &cwd).unwrap(); + let expected = Url::from_file_path( + cwd.join("node_modules/imports_exports/import_export.js"), + ) + .unwrap(); + assert_eq!(actual, expected); + + // check that `imports` mapping works correctly + let cwd = testdir("conditions/node_modules/imports_exports"); + let main = Url::from_file_path(cwd.join("import_export.js")).unwrap(); + let actual = node_resolve("#dep", main.as_str(), &cwd).unwrap(); + let expected = Url::from_file_path(cwd.join("import_polyfill.js")).unwrap(); + assert_eq!(actual, expected); + } } diff --git a/cli/compat/testdata/conditions/main.js b/cli/compat/testdata/conditions/main.js new file mode 100644 index 00000000000000..cafddb5d686454 --- /dev/null +++ b/cli/compat/testdata/conditions/main.js @@ -0,0 +1 @@ +import "imports_exports"; diff --git a/cli/compat/testdata/conditions/node_modules/imports_exports/import_export.js b/cli/compat/testdata/conditions/node_modules/imports_exports/import_export.js new file mode 100644 index 00000000000000..3ebd222ea901a8 --- /dev/null +++ b/cli/compat/testdata/conditions/node_modules/imports_exports/import_export.js @@ -0,0 +1,6 @@ +import dep from "#dep"; + +export default { + bar: "bar", + dep, +}; diff --git a/cli/compat/testdata/conditions/node_modules/imports_exports/import_polyfill.js b/cli/compat/testdata/conditions/node_modules/imports_exports/import_polyfill.js new file mode 100644 index 00000000000000..76716a3ef47191 --- /dev/null +++ b/cli/compat/testdata/conditions/node_modules/imports_exports/import_polyfill.js @@ -0,0 +1,3 @@ +export default { + polyfill: "import", +}; diff --git a/cli/compat/testdata/conditions/node_modules/imports_exports/package.json b/cli/compat/testdata/conditions/node_modules/imports_exports/package.json new file mode 100644 index 00000000000000..5d26359db3485a --- /dev/null +++ b/cli/compat/testdata/conditions/node_modules/imports_exports/package.json @@ -0,0 +1,17 @@ +{ + "version": "1.0.0", + "name": "imports_exports", + "main": "./require_export.cjs", + "imports": { + "#dep": { + "import": "./import_polyfill.js", + "require": "./require_polyfill.js" + } + }, + "exports": { + ".": { + "import": "./import_export.js", + "require": "./require_export.cjs" + } + } +} diff --git a/cli/compat/testdata/conditions/node_modules/imports_exports/require_export.cjs b/cli/compat/testdata/conditions/node_modules/imports_exports/require_export.cjs new file mode 100644 index 00000000000000..11648c0d71f80c --- /dev/null +++ b/cli/compat/testdata/conditions/node_modules/imports_exports/require_export.cjs @@ -0,0 +1,6 @@ +const dep = require("#dep"); + +module.exports = { + foo: "foo", + dep, +}; \ No newline at end of file diff --git a/cli/compat/testdata/conditions/node_modules/imports_exports/require_polyfill.js b/cli/compat/testdata/conditions/node_modules/imports_exports/require_polyfill.js new file mode 100644 index 00000000000000..1023fd65cbe3d0 --- /dev/null +++ b/cli/compat/testdata/conditions/node_modules/imports_exports/require_polyfill.js @@ -0,0 +1,3 @@ +module.exports = { + polyfill: "require", +}; diff --git a/cli/compat/testdata/conditions/package.json b/cli/compat/testdata/conditions/package.json new file mode 100644 index 00000000000000..4d6d004dd3ee51 --- /dev/null +++ b/cli/compat/testdata/conditions/package.json @@ -0,0 +1,7 @@ +{ + "name": "conditions", + "type": "module", + "dependencies": { + "imports_exports": "1.0.0" + } +} From cfb47bd90dbfaead6c34fef5554ba695bea7ff32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 18 Oct 2021 02:34:38 +0200 Subject: [PATCH 32/40] update reference to deno_std --- cli/compat/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index dd99cd59a0164b..f3ea65e92be3af 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -14,7 +14,7 @@ pub use esm_resolver::NodeEsmResolver; // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded -static STD_URL_STR: &str = "https://raw.githubusercontent.com/denoland/deno_std/acd70dced5629ed4e20ac464bdb7d498f3c51d83/"; +static STD_URL_STR: &str = "https://raw.githubusercontent.com/denoland/deno_std/b83eb65244c676608a69205569bd294113476464/"; static GLOBAL_MODULE: &str = "global.ts"; static MODULE_MODULE: &str = "module.ts"; From 62dd1c6fbefe6ef62573a1d69d7bcd503cdc59c9 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 18 Oct 2021 10:31:52 -0400 Subject: [PATCH 33/40] small clean ups --- cli/compat/mod.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index f3ea65e92be3af..94ee6686d80221 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -15,8 +15,6 @@ pub use esm_resolver::NodeEsmResolver; // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded static STD_URL_STR: &str = "https://raw.githubusercontent.com/denoland/deno_std/b83eb65244c676608a69205569bd294113476464/"; -static GLOBAL_MODULE: &str = "global.ts"; -static MODULE_MODULE: &str = "module.ts"; static SUPPORTED_MODULES: &[&str] = &[ "assert", @@ -64,9 +62,9 @@ static SUPPORTED_MODULES: &[&str] = &[ ]; lazy_static::lazy_static! { - static ref GLOBAL_URL_STR: String = format!("{}node/{}", STD_URL_STR, GLOBAL_MODULE); + static ref GLOBAL_URL_STR: String = format!("{}node/global.ts", STD_URL_STR); pub(crate) static ref GLOBAL_URL: Url = Url::parse(&GLOBAL_URL_STR).unwrap(); - static ref MODULE_URL_STR: String = format!("{}node/{}", STD_URL_STR, MODULE_MODULE); + static ref MODULE_URL_STR: String = format!("{}node/module.ts", STD_URL_STR); pub(crate) static ref MODULE_URL: Url = Url::parse(&MODULE_URL_STR).unwrap(); static ref COMPAT_IMPORT_URL: Url = Url::parse("flags:compat").unwrap(); } @@ -76,7 +74,7 @@ pub(crate) fn get_node_imports() -> Vec<(Url, Vec)> { vec![(COMPAT_IMPORT_URL.clone(), vec![GLOBAL_URL_STR.clone()])] } -pub(crate) fn try_resolve_builtin_module(specifier: &str) -> Option { +fn try_resolve_builtin_module(specifier: &str) -> Option { if SUPPORTED_MODULES.contains(&specifier) { let module_url = format!("{}node/{}.ts", STD_URL_STR, specifier); Some(Url::parse(&module_url).unwrap()) @@ -85,7 +83,7 @@ pub(crate) fn try_resolve_builtin_module(specifier: &str) -> Option { } } -pub(crate) async fn check_if_should_use_esm_loader( +pub async fn check_if_should_use_esm_loader( js_runtime: &mut JsRuntime, main_module: &str, ) -> Result { @@ -112,12 +110,12 @@ pub(crate) async fn check_if_should_use_esm_loader( Ok(use_esm_loader) } -pub(crate) fn load_cjs_module( +pub fn load_cjs_module( js_runtime: &mut JsRuntime, main_module: &str, ) -> Result<(), AnyError> { let source_code = &format!( - r#"(async function loadCjsModule(main) {{ + r#"(async function loadCjsModule(main) {{ const Module = await import("{}"); Module.default._load(main, null, true); }})('{}');"#, From b32b4cdd0e230b7f1d2641021125a6f305f4c20f Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 18 Oct 2021 10:41:06 -0400 Subject: [PATCH 34/40] add test_is_relative_specifier --- cli/compat/esm_resolver.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index 945972e9edeb68..c1d61f1620d39b 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -113,6 +113,7 @@ fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { is_relative_specifier(specifier) } +// TODO(ry) We very likely have this utility function elsewhere in Deno. fn is_relative_specifier(specifier: &str) -> bool { let specifier_len = specifier.len(); let specifier_chars: Vec<_> = specifier.chars().collect(); @@ -1159,4 +1160,10 @@ mod tests { let expected = Url::from_file_path(cwd.join("import_polyfill.js")).unwrap(); assert_eq!(actual, expected); } + + #[test] + fn test_is_relative_specifier() { + assert!(is_relative_specifier("./foo.js")); + assert!(!is_relative_specifier("https://deno.land/std/node/http.ts")); + } } From 09e9dae0434134690898f7942bd6376d3c6fc491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 18 Oct 2021 17:26:06 +0200 Subject: [PATCH 35/40] review comments --- cli/compat/esm_resolver.rs | 59 +++++++++++++++++++++++--------------- cli/compat/mod.rs | 2 +- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index c1d61f1620d39b..167edefa8672cb 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -47,16 +47,15 @@ fn node_resolve( } if let Ok(url) = Url::parse(specifier) { - if url.scheme() == "data:" { + if url.scheme() == "data" { return Ok(url); } let protocol = url.scheme(); if protocol == "node" { - let mut split_specifier = url.as_str().split(':'); - split_specifier.next(); - let specifier = split_specifier.collect::>().join(""); + let split_specifier = url.as_str().split(':'); + let specifier = split_specifier.skip(1).collect::>().join(""); if let Some(resolved) = crate::compat::try_resolve_builtin_module(&specifier) { @@ -91,13 +90,13 @@ fn node_resolve( Ok(url) } -fn to_file_path(url: &Url) -> PathBuf { +fn to_file_path(url: &ModuleSpecifier) -> PathBuf { url .to_file_path() - .expect("Provided URL was not file:// URL") + .unwrap_or_else(|_| panic!("Provided URL was not file:// URL: {}", url)) } -fn to_file_path_string(url: &Url) -> String { +fn to_file_path_string(url: &ModuleSpecifier) -> String { to_file_path(url).display().to_string() } @@ -209,8 +208,8 @@ fn finalize_resolution( fn throw_import_not_defined( specifier: &str, - package_json_url: Option, - base: &Url, + package_json_url: Option, + base: &ModuleSpecifier, ) -> AnyError { errors::err_package_import_not_defined( specifier, @@ -345,7 +344,7 @@ fn package_imports_resolve( fn is_conditional_exports_main_sugar( exports: &Value, - package_json_url: &Url, + package_json_url: &ModuleSpecifier, base: &ModuleSpecifier, ) -> Result { if exports.is_string() || exports.is_array() { @@ -381,9 +380,9 @@ fn is_conditional_exports_main_sugar( fn throw_invalid_package_target( subpath: String, target: String, - package_json_url: &Url, + package_json_url: &ModuleSpecifier, internal: bool, - base: &Url, + base: &ModuleSpecifier, ) -> AnyError { errors::err_invalid_package_target( to_file_path_string(&package_json_url.join(".").unwrap()), @@ -396,9 +395,9 @@ fn throw_invalid_package_target( fn throw_invalid_subpath( subpath: String, - package_json_url: &Url, + package_json_url: &ModuleSpecifier, internal: bool, - base: &Url, + base: &ModuleSpecifier, ) -> AnyError { let ie = if internal { "imports" } else { "exports" }; let reason = format!( @@ -418,7 +417,7 @@ fn resolve_package_target_string( target: String, subpath: String, match_: String, - package_json_url: Url, + package_json_url: ModuleSpecifier, base: &ModuleSpecifier, pattern: bool, internal: bool, @@ -516,7 +515,7 @@ fn resolve_package_target_string( #[allow(clippy::too_many_arguments)] fn resolve_package_target( - package_json_url: Url, + package_json_url: ModuleSpecifier, target: Value, subpath: String, package_subpath: String, @@ -615,8 +614,8 @@ fn resolve_package_target( fn throw_exports_not_found( subpath: String, - package_json_url: &Url, - base: &Url, + package_json_url: &ModuleSpecifier, + base: &ModuleSpecifier, ) -> AnyError { errors::err_package_path_not_exported( to_file_path_string(&package_json_url.join(".").unwrap()), @@ -626,7 +625,7 @@ fn throw_exports_not_found( } fn package_exports_resolve( - package_json_url: Url, + package_json_url: ModuleSpecifier, package_subpath: String, package_config: PackageConfig, base: &ModuleSpecifier, @@ -682,6 +681,7 @@ fn package_exports_resolve( // // To match "imports" and the spec. if package_subpath.ends_with('/') { + // TODO(bartlomieju): // emitTrailingSlashPatternDeprecation(); } let pattern_trailer = &key[pattern_index + 1..]; @@ -765,7 +765,8 @@ fn package_resolve( let mut last_path; loop { let p_str = package_json_path.to_str().unwrap(); - let p = p_str[0..=p_str.len() - 13].to_string(); + let package_str_len = "/package.json".len(); + let p = p_str[0..=p_str.len() - package_str_len].to_string(); let is_dir = if let Ok(stats) = std::fs::metadata(&p) { stats.is_dir() } else { @@ -930,7 +931,6 @@ fn get_package_config( let typ_val = package_json.get("type"); let exports = package_json.get("exports").map(|e| e.to_owned()); - // TODO(bartlomieju): refactor let imports = if let Some(imp) = imports_val { imp.as_object().map(|imp| imp.to_owned()) } else { @@ -1026,7 +1026,7 @@ fn get_package_scope_config( Ok(package_config) } -fn file_exists(path_url: &Url) -> bool { +fn file_exists(path_url: &ModuleSpecifier) -> bool { if let Ok(stats) = std::fs::metadata(to_file_path(path_url)) { stats.is_file() } else { @@ -1035,7 +1035,7 @@ fn file_exists(path_url: &Url) -> bool { } fn legacy_main_resolve( - package_json_url: &Url, + package_json_url: &ModuleSpecifier, package_config: &PackageConfig, _base: &ModuleSpecifier, ) -> Result { @@ -1097,6 +1097,19 @@ mod tests { let expected = Url::from_file_path(cwd.join("node_modules/foo/index.js")).unwrap(); assert_eq!(actual, expected); + + let actual = node_resolve( + "data:application/javascript,console.log(\"Hello%20Deno\");", + main.as_str(), + &cwd, + ) + .unwrap(); + eprintln!("actual {}", actual); + assert_eq!( + actual, + Url::parse("data:application/javascript,console.log(\"Hello%20Deno\");") + .unwrap() + ); } #[test] diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 94ee6686d80221..f617dd959578ca 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -14,7 +14,7 @@ pub use esm_resolver::NodeEsmResolver; // each release, a better mechanism is preferable, but it's a quick and dirty // solution to avoid printing `X-Deno-Warning` headers when the compat layer is // downloaded -static STD_URL_STR: &str = "https://raw.githubusercontent.com/denoland/deno_std/b83eb65244c676608a69205569bd294113476464/"; +static STD_URL_STR: &str = "https://deno.land/std@0.112.0/"; static SUPPORTED_MODULES: &[&str] = &[ "assert", From 8c39946199d2998f78deaa2c903db0753b3163de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 18 Oct 2021 17:52:14 +0200 Subject: [PATCH 36/40] fix test --- cli/compat/esm_resolver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/compat/esm_resolver.rs b/cli/compat/esm_resolver.rs index 167edefa8672cb..c0366c27b04bae 100644 --- a/cli/compat/esm_resolver.rs +++ b/cli/compat/esm_resolver.rs @@ -1143,7 +1143,7 @@ mod tests { let cwd = testdir("basic"); let main = Url::from_file_path(cwd.join("main.js")).unwrap(); let expected = - Url::parse("https://deno.land/std@0.111.0/node/http.ts").unwrap(); + Url::parse("https://deno.land/std@0.112.0/node/http.ts").unwrap(); let actual = node_resolve("http", main.as_str(), &cwd).unwrap(); println!("actual {}", actual); From f357f98155a0bb0e30e2353db5ac3eecbc3cf2bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 18 Oct 2021 18:12:31 +0200 Subject: [PATCH 37/40] fix test --- cli/tests/integration/compat_tests.rs | 4 ++-- cli/tests/testdata/compat/{fs_promises.js => fs_promises.mjs} | 0 cli/tests/testdata/compat/globals.out | 4 +++- .../compat/{node_fs_promises.js => node_fs_promises.mjs} | 0 4 files changed, 5 insertions(+), 3 deletions(-) rename cli/tests/testdata/compat/{fs_promises.js => fs_promises.mjs} (100%) rename cli/tests/testdata/compat/{node_fs_promises.js => node_fs_promises.mjs} (100%) diff --git a/cli/tests/integration/compat_tests.rs b/cli/tests/integration/compat_tests.rs index f0feb14e6ecbbe..17388a78e8a012 100644 --- a/cli/tests/integration/compat_tests.rs +++ b/cli/tests/integration/compat_tests.rs @@ -9,12 +9,12 @@ itest!(globals { }); itest!(fs_promises { - args: "run --compat --unstable -A compat/fs_promises.js", + args: "run --compat --unstable -A compat/fs_promises.mjs", output: "compat/fs_promises.out", }); itest!(node_prefix_fs_promises { - args: "run --compat --unstable -A compat/node_fs_promises.js", + args: "run --compat --unstable -A compat/node_fs_promises.mjs", output: "compat/fs_promises.out", }); diff --git a/cli/tests/testdata/compat/fs_promises.js b/cli/tests/testdata/compat/fs_promises.mjs similarity index 100% rename from cli/tests/testdata/compat/fs_promises.js rename to cli/tests/testdata/compat/fs_promises.mjs diff --git a/cli/tests/testdata/compat/globals.out b/cli/tests/testdata/compat/globals.out index 0bc09137be019b..32230fda37dfd3 100644 --- a/cli/tests/testdata/compat/globals.out +++ b/cli/tests/testdata/compat/globals.out @@ -2,6 +2,8 @@ process { [WILDCARD] } -[Function: Buffer] +[Function: Buffer] { +[WILDCARD] +} [Function: setImmediate] [Function: clearTimeout] diff --git a/cli/tests/testdata/compat/node_fs_promises.js b/cli/tests/testdata/compat/node_fs_promises.mjs similarity index 100% rename from cli/tests/testdata/compat/node_fs_promises.js rename to cli/tests/testdata/compat/node_fs_promises.mjs From fa59547f03f09b897c3a493c460c24340978c92a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 18 Oct 2021 13:02:16 -0400 Subject: [PATCH 38/40] Escape injected string. --- cli/compat/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index f617dd959578ca..212c8736a44348 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -120,7 +120,7 @@ pub fn load_cjs_module( Module.default._load(main, null, true); }})('{}');"#, MODULE_URL_STR.as_str(), - main_module, + main_module.replace(r"\", r"\\").replace("'", r"\'"), ); js_runtime.execute_script(&located_script_name!(), source_code)?; From cb1652ccfbf2b05058573dfd02424b4da48fe226 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 18 Oct 2021 13:12:14 -0400 Subject: [PATCH 39/40] Escape other injected single quote string too. --- cli/compat/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 212c8736a44348..37584c0ab7bda7 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -96,7 +96,7 @@ pub async fn check_if_should_use_esm_loader( return useESMLoader; }})('{}');"#, MODULE_URL_STR.as_str(), - main_module + escape_for_single_quote_string(main_module), ); let result = js_runtime.execute_script(&located_script_name!(), source_code)?; @@ -114,15 +114,20 @@ pub fn load_cjs_module( js_runtime: &mut JsRuntime, main_module: &str, ) -> Result<(), AnyError> { + println!("{}", main_module); let source_code = &format!( r#"(async function loadCjsModule(main) {{ const Module = await import("{}"); Module.default._load(main, null, true); }})('{}');"#, MODULE_URL_STR.as_str(), - main_module.replace(r"\", r"\\").replace("'", r"\'"), + escape_for_single_quote_string(main_module), ); js_runtime.execute_script(&located_script_name!(), source_code)?; Ok(()) } + +fn escape_for_single_quote_string(text: &str) -> String { + text.replace(r"\", r"\\").replace("'", r"\'") +} From f6686da9471ba0a9a22943524ee22f6ea2be5c08 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 18 Oct 2021 13:13:10 -0400 Subject: [PATCH 40/40] Remove accidental println --- cli/compat/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/compat/mod.rs b/cli/compat/mod.rs index 37584c0ab7bda7..b95b65ddb7cb7c 100644 --- a/cli/compat/mod.rs +++ b/cli/compat/mod.rs @@ -114,7 +114,6 @@ pub fn load_cjs_module( js_runtime: &mut JsRuntime, main_module: &str, ) -> Result<(), AnyError> { - println!("{}", main_module); let source_code = &format!( r#"(async function loadCjsModule(main) {{ const Module = await import("{}");