feat: embed import map in the config file (#17478)

This commit changes handling of config file to enable
specifying "imports" and "scopes" objects effectively making
the configuration file an import map.

"imports" and "scopes" take precedence over "importMap" configuration,
but have lower priority than "--importmap" CLI flag.

Co-authored-by: David Sherret <dsherret@users.noreply.github.com>
Co-authored-by: David Sherret <dsherret@gmail.com>
This commit is contained in:
Bartek Iwańczuk 2023-01-25 21:13:40 +01:00 committed by GitHub
parent b5b4887c4a
commit c6c8c91a6e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 418 additions and 139 deletions

18
Cargo.lock generated
View file

@ -792,7 +792,7 @@ dependencies = [
"fwdansi",
"glibc_version",
"http",
"import_map",
"import_map 0.15.0",
"indexmap",
"jsonc-parser",
"junction",
@ -980,7 +980,7 @@ dependencies = [
"deno_ast",
"deno_graph",
"futures",
"import_map",
"import_map 0.13.0",
"lazy_static",
"regex",
"serde",
@ -2308,6 +2308,20 @@ dependencies = [
"url",
]
[[package]]
name = "import_map"
version = "0.15.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "632089ec08bd62e807311104122fb26d5c911ab172e2b9864be154a575979e29"
dependencies = [
"cfg-if",
"indexmap",
"log",
"serde",
"serde_json",
"url",
]
[[package]]
name = "indexmap"
version = "1.9.2"

View file

@ -73,7 +73,7 @@ eszip = "=0.33.0"
fancy-regex = "=0.10.0"
flate2.workspace = true
http.workspace = true
import_map = "=0.13.0"
import_map = "=0.15.0"
indexmap = "=1.9.2"
jsonc-parser = { version = "=0.21.0", features = ["serde"] }
libc.workspace = true

View file

@ -465,6 +465,8 @@ pub enum LockConfig {
pub struct ConfigFileJson {
pub compiler_options: Option<Value>,
pub import_map: Option<String>,
pub imports: Option<Value>,
pub scopes: Option<Value>,
pub lint: Option<Value>,
pub fmt: Option<Value>,
pub tasks: Option<Value>,
@ -667,6 +669,21 @@ impl ConfigFile {
self.json.import_map.clone()
}
pub fn to_import_map_value(&self) -> Value {
let mut value = serde_json::Map::with_capacity(2);
if let Some(imports) = &self.json.imports {
value.insert("imports".to_string(), imports.clone());
}
if let Some(scopes) = &self.json.scopes {
value.insert("scopes".to_string(), scopes.clone());
}
value.into()
}
pub fn is_an_import_map(&self) -> bool {
self.json.imports.is_some() || self.json.scopes.is_some()
}
pub fn to_fmt_config(&self) -> Result<Option<FmtConfig>, AnyError> {
if let Some(config) = self.json.fmt.clone() {
let fmt_config: SerializedFmtConfig = serde_json::from_value(config)

35
cli/args/import_map.rs Normal file
View file

@ -0,0 +1,35 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_core::url::Url;
use import_map::ImportMap;
use import_map::ImportMapDiagnostic;
use log::warn;
pub fn import_map_from_value(
specifier: &Url,
json_value: serde_json::Value,
) -> Result<ImportMap, AnyError> {
debug_assert!(
!specifier.as_str().contains("../"),
"Import map specifier incorrectly contained ../: {}",
specifier.as_str()
);
let result = import_map::parse_from_value(specifier, json_value)?;
print_import_map_diagnostics(&result.diagnostics);
Ok(result.import_map)
}
fn print_import_map_diagnostics(diagnostics: &[ImportMapDiagnostic]) {
if !diagnostics.is_empty() {
warn!(
"Import map diagnostics:\n{}",
diagnostics
.iter()
.map(|d| format!(" - {}", d))
.collect::<Vec<_>>()
.join("\n")
);
}
}

View file

@ -2,10 +2,12 @@
mod config_file;
mod flags;
mod flags_allow_net;
mod import_map;
mod lockfile;
mod flags_allow_net;
pub use self::import_map::import_map_from_value;
use ::import_map::ImportMap;
pub use config_file::BenchConfig;
pub use config_file::CompilerOptions;
pub use config_file::ConfigFile;
@ -19,6 +21,8 @@ pub use config_file::TsConfig;
pub use config_file::TsConfigForEmit;
pub use config_file::TsConfigType;
pub use config_file::TsTypeLib;
use deno_core::serde_json;
use deno_runtime::permissions::PermissionsContainer;
pub use flags::*;
pub use lockfile::Lockfile;
pub use lockfile::LockfileError;
@ -49,6 +53,7 @@ use std::path::PathBuf;
use std::sync::Arc;
use crate::cache::DenoDir;
use crate::file_fetcher::FileFetcher;
use crate::util::fs::canonicalize_path_maybe_not_exists;
use crate::version;
@ -547,12 +552,13 @@ impl CliOptions {
}
/// Based on an optional command line import map path and an optional
/// configuration file, return a resolved module specifier to an import map.
/// configuration file, return a resolved module specifier to an import map
/// and a boolean indicating if unknown keys should not result in diagnostics.
pub fn resolve_import_map_specifier(
&self,
) -> Result<Option<ModuleSpecifier>, AnyError> {
match self.overrides.import_map_specifier.clone() {
Some(path) => Ok(path),
Some(maybe_path) => Ok(maybe_path),
None => resolve_import_map_specifier(
self.flags.import_map_path.as_deref(),
self.maybe_config_file.as_ref(),
@ -560,6 +566,45 @@ impl CliOptions {
}
}
pub async fn resolve_import_map(
&self,
file_fetcher: &FileFetcher,
) -> Result<Option<ImportMap>, AnyError> {
let import_map_specifier = match self.resolve_import_map_specifier()? {
Some(specifier) => specifier,
None => return Ok(None),
};
self
.resolve_import_map_from_specifier(&import_map_specifier, file_fetcher)
.await
.context(format!(
"Unable to load '{}' import map",
import_map_specifier
))
.map(Some)
}
async fn resolve_import_map_from_specifier(
&self,
import_map_specifier: &ModuleSpecifier,
file_fetcher: &FileFetcher,
) -> Result<ImportMap, AnyError> {
let import_map_config = self
.get_maybe_config_file()
.as_ref()
.filter(|c| c.specifier == *import_map_specifier);
let value: serde_json::Value = match import_map_config {
Some(config) => config.to_import_map_value(),
None => {
let file = file_fetcher
.fetch(import_map_specifier, PermissionsContainer::allow_all())
.await?;
serde_json::from_str(&file.source)?
}
};
import_map_from_value(import_map_specifier, value)
}
/// Overrides the import map specifier to use.
pub fn set_import_map_specifier(&mut self, path: Option<ModuleSpecifier>) {
self.overrides.import_map_specifier = Some(path);
@ -907,6 +952,16 @@ fn resolve_import_map_specifier(
.context(format!("Bad URL (\"{}\") for import map.", import_map_path))?;
return Ok(Some(specifier));
} else if let Some(config_file) = &maybe_config_file {
// if the config file is an import map we prefer to use it, over `importMap`
// field
if config_file.is_an_import_map() {
if let Some(_import_map_path) = config_file.to_import_map_path() {
log::warn!("{} \"importMap\" setting is ignored when \"imports\" or \"scopes\" are specified in the config file.", colors::yellow("Warning"));
}
return Ok(Some(config_file.specifier.clone()));
}
// when the import map is specifier in a config file, it needs to be
// resolved relative to the config file, versus the CWD like with the flag
// and with config files, we support both local and remote config files,
@ -990,7 +1045,7 @@ mod test {
let actual = actual.unwrap();
assert_eq!(
actual,
Some(ModuleSpecifier::parse("file:///deno/import_map.json").unwrap())
Some(ModuleSpecifier::parse("file:///deno/import_map.json").unwrap(),)
);
}
@ -1051,6 +1106,21 @@ mod test {
assert_eq!(actual, Some(expected_specifier));
}
#[test]
fn resolve_import_map_embedded_take_precedence() {
let config_text = r#"{
"importMap": "import_map.json",
"imports": {},
}"#;
let config_specifier =
ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap();
let config_file = ConfigFile::new(config_text, &config_specifier).unwrap();
let actual = resolve_import_map_specifier(None, Some(&config_file));
assert!(actual.is_ok());
let actual = actual.unwrap();
assert_eq!(actual, Some(config_specifier));
}
#[test]
fn resolve_import_map_none() {
let config_text = r#"{}"#;

View file

@ -57,6 +57,7 @@ use super::tsc::AssetsSnapshot;
use super::tsc::TsServer;
use super::urls;
use crate::args::get_root_cert_store;
use crate::args::import_map_from_value;
use crate::args::CaData;
use crate::args::CacheSetting;
use crate::args::CliOptions;
@ -72,7 +73,6 @@ use crate::http_util::HttpClient;
use crate::npm::NpmCache;
use crate::npm::NpmPackageResolver;
use crate::npm::RealNpmRegistryApi;
use crate::proc_state::import_map_from_text;
use crate::proc_state::ProcState;
use crate::tools::fmt::format_file;
use crate::tools::fmt::format_parsed_source;
@ -601,47 +601,84 @@ impl Inner {
pub async fn update_import_map(&mut self) -> Result<(), AnyError> {
let mark = self.performance.mark("update_import_map", None::<()>);
let maybe_import_map_url = if let Some(import_map_str) = self
.config
.get_workspace_settings()
.import_map
.and_then(|s| if s.is_empty() { None } else { Some(s) })
{
lsp_log!(
"Setting import map from workspace settings: \"{}\"",
import_map_str
);
if let Some(config_file) = &self.maybe_config_file {
if let Some(import_map_path) = config_file.to_import_map_path() {
lsp_log!("Warning: Import map \"{}\" configured in \"{}\" being ignored due to an import map being explicitly configured in workspace settings.", import_map_path, config_file.specifier);
}
}
if let Ok(url) = Url::from_file_path(&import_map_str) {
Some(url)
} else if import_map_str.starts_with("data:") {
Some(Url::parse(&import_map_str).map_err(|_| {
anyhow!("Bad data url for import map: {}", import_map_str)
})?)
} else if let Some(root_uri) = &self.config.root_uri {
let root_path = specifier_to_file_path(root_uri)?;
let import_map_path = root_path.join(&import_map_str);
Some(Url::from_file_path(import_map_path).map_err(|_| {
anyhow!("Bad file path for import map: {}", import_map_str)
})?)
} else {
return Err(anyhow!(
"The path to the import map (\"{}\") is not resolvable.",
import_map_str
));
}
} else if let Some(config_file) = &self.maybe_config_file {
if let Some(import_map_path) = config_file.to_import_map_path() {
let maybe_import_map_url = self.resolve_import_map_specifier()?;
if let Some(import_map_url) = maybe_import_map_url {
let import_map = self
.resolve_import_map_from_specifier(&import_map_url)
.await
.map_err(|err| {
anyhow!(
"Failed to load the import map at: {}. {:#}",
import_map_url,
err
)
})?;
self.maybe_import_map_uri = Some(import_map_url);
self.maybe_import_map = Some(Arc::new(import_map));
} else {
self.maybe_import_map_uri = None;
self.maybe_import_map = None;
}
self.performance.measure(mark);
Ok(())
}
fn resolve_import_map_specifier(
&self,
) -> Result<Option<ModuleSpecifier>, AnyError> {
Ok(
if let Some(import_map_str) = self
.config
.get_workspace_settings()
.import_map
.and_then(|s| if s.is_empty() { None } else { Some(s) })
{
lsp_log!(
"Setting import map from configuration file: \"{}\"",
import_map_path
"Setting import map from workspace settings: \"{}\"",
import_map_str
);
let specifier =
if let Ok(config_file_path) = config_file.specifier.to_file_path() {
if let Some(config_file) = &self.maybe_config_file {
if let Some(import_map_path) = config_file.to_import_map_path() {
lsp_log!("Warning: Import map \"{}\" configured in \"{}\" being ignored due to an import map being explicitly configured in workspace settings.", import_map_path, config_file.specifier);
}
}
if let Ok(url) = Url::from_file_path(&import_map_str) {
Some(url)
} else if import_map_str.starts_with("data:") {
let import_map_url = Url::parse(&import_map_str).map_err(|_| {
anyhow!("Bad data url for import map: {}", import_map_str)
})?;
Some(import_map_url)
} else if let Some(root_uri) = &self.config.root_uri {
let root_path = specifier_to_file_path(root_uri)?;
let import_map_path = root_path.join(&import_map_str);
let import_map_url =
Url::from_file_path(import_map_path).map_err(|_| {
anyhow!("Bad file path for import map: {}", import_map_str)
})?;
Some(import_map_url)
} else {
return Err(anyhow!(
"The path to the import map (\"{}\") is not resolvable.",
import_map_str
));
}
} else if let Some(config_file) = &self.maybe_config_file {
if config_file.is_an_import_map() {
lsp_log!(
"Setting import map defined in configuration file: \"{}\"",
config_file.specifier
);
let import_map_url = config_file.specifier.clone();
Some(import_map_url)
} else if let Some(import_map_path) = config_file.to_import_map_path() {
lsp_log!(
"Setting import map from configuration file: \"{}\"",
import_map_path
);
let specifier = if let Ok(config_file_path) =
config_file.specifier.to_file_path()
{
let import_map_file_path = config_file_path
.parent()
.ok_or_else(|| {
@ -655,39 +692,41 @@ impl Inner {
config_file.specifier.as_str(),
)?
};
Some(specifier)
Some(specifier)
} else {
None
}
} else {
None
},
)
}
async fn resolve_import_map_from_specifier(
&self,
import_map_url: &ModuleSpecifier,
) -> Result<ImportMap, AnyError> {
let import_map_json: Value = if import_map_url.scheme() == "data" {
serde_json::from_str(&get_source_from_data_url(import_map_url)?.0)?
} else {
let import_map_path = specifier_to_file_path(import_map_url)?;
lsp_log!(
" Resolved import map: \"{}\"",
import_map_path.to_string_lossy()
);
let import_map_config_file = self
.maybe_config_file
.as_ref()
.filter(|c| c.specifier == *import_map_url);
match import_map_config_file {
Some(c) => c.to_import_map_value(),
None => {
serde_json::from_str(&fs::read_to_string(import_map_path).await?)?
}
}
} else {
None
};
if let Some(import_map_url) = maybe_import_map_url {
let import_map_json = if import_map_url.scheme() == "data" {
get_source_from_data_url(&import_map_url)?.0
} else {
let import_map_path = specifier_to_file_path(&import_map_url)?;
lsp_log!(
" Resolved import map: \"{}\"",
import_map_path.to_string_lossy()
);
fs::read_to_string(import_map_path).await.map_err(|err| {
anyhow!(
"Failed to load the import map at: {}. [{}]",
import_map_url,
err
)
})?
};
let import_map = import_map_from_text(&import_map_url, &import_map_json)?;
self.maybe_import_map_uri = Some(import_map_url);
self.maybe_import_map = Some(Arc::new(import_map));
} else {
self.maybe_import_map_uri = None;
self.maybe_import_map = None;
}
self.performance.measure(mark);
Ok(())
import_map_from_value(import_map_url, import_map_json)
}
pub fn update_debug_flag(&self) {

View file

@ -43,7 +43,6 @@ use deno_core::futures;
use deno_core::parking_lot::Mutex;
use deno_core::parking_lot::RwLock;
use deno_core::resolve_url_or_path;
use deno_core::url::Url;
use deno_core::CompiledWasmModuleStore;
use deno_core::ModuleSpecifier;
use deno_core::SharedArrayBufferStore;
@ -206,25 +205,11 @@ impl ProcState {
)?;
let lockfile = cli_options.maybe_lock_file();
let maybe_import_map_specifier =
cli_options.resolve_import_map_specifier()?;
let maybe_import_map =
if let Some(import_map_specifier) = maybe_import_map_specifier {
let file = file_fetcher
.fetch(&import_map_specifier, PermissionsContainer::allow_all())
.await
.context(format!(
"Unable to load '{}' import map",
import_map_specifier
))?;
let import_map =
import_map_from_text(&import_map_specifier, &file.source)?;
Some(Arc::new(import_map))
} else {
None
};
let maybe_import_map = cli_options
.resolve_import_map(&file_fetcher)
.await?
.map(Arc::new);
let maybe_inspector_server =
cli_options.resolve_inspector_server().map(Arc::new);
@ -765,30 +750,6 @@ impl ProcState {
}
}
pub fn import_map_from_text(
specifier: &Url,
json_text: &str,
) -> Result<ImportMap, AnyError> {
debug_assert!(
!specifier.as_str().contains("../"),
"Import map specifier incorrectly contained ../: {}",
specifier.as_str()
);
let result = import_map::parse_from_json(specifier, json_text)?;
if !result.diagnostics.is_empty() {
warn!(
"Import map diagnostics:\n{}",
result
.diagnostics
.into_iter()
.map(|d| format!(" - {}", d))
.collect::<Vec<_>>()
.join("\n")
);
}
Ok(result.import_map)
}
#[derive(Clone, Debug)]
struct FileWatcherReporter {
sender: tokio::sync::mpsc::UnboundedSender<Vec<PathBuf>>,

View file

@ -198,9 +198,31 @@
}
},
"importMap": {
"description": "The location of an import map to be used when resolving modules. If an import map is explicitly specified, it will override this value.",
"description": "The location of an import map to be used when resolving modules. If an import map is specified as an `--importmap` flag or using \"imports\" and \"scopes\" properties, they will override this value.",
"type": "string"
},
"imports": {
"description": "A map of specifiers to their remapped specifiers.",
"type": "object",
"additionalProperties": {
"description": "The key is the specifier or partial specifier to match, with a value that represents the target specifier.",
"type": "string"
}
},
"scopes": {
"default": {},
"description": "Define a scope which remaps a specifier in only a specified scope",
"type": "object",
"properties": {},
"additionalProperties": {
"description": "A definition of a scoped remapping.",
"type": "object",
"additionalProperties": {
"description": "The key is the specifier or partial specifier to match within the referring scope, with a value that represents the target specifier.",
"type": "string"
}
}
},
"lint": {
"description": "Configuration for linter",
"type": "object",

View file

@ -582,6 +582,93 @@ fn lsp_import_map_config_file() {
shutdown(&mut client);
}
#[test]
fn lsp_import_map_embedded_in_config_file() {
let temp_dir = TempDir::new();
let mut params: lsp::InitializeParams =
serde_json::from_value(load_fixture("initialize_params.json")).unwrap();
let deno_import_map_jsonc = serde_json::to_string_pretty(&load_fixture(
"deno.embedded_import_map.jsonc",
))
.unwrap();
temp_dir.write("deno.embedded_import_map.jsonc", deno_import_map_jsonc);
params.root_uri = Some(Url::from_file_path(temp_dir.path()).unwrap());
if let Some(Value::Object(mut map)) = params.initialization_options {
map.insert(
"config".to_string(),
json!("./deno.embedded_import_map.jsonc"),
);
params.initialization_options = Some(Value::Object(map));
}
fs::create_dir(temp_dir.path().join("lib")).unwrap();
temp_dir.write("lib/b.ts", r#"export const b = "b";"#);
let deno_exe = deno_exe_path();
let mut client = LspClient::new(&deno_exe, false).unwrap();
client
.write_request::<_, _, Value>("initialize", params)
.unwrap();
client.write_notification("initialized", json!({})).unwrap();
let uri = Url::from_file_path(temp_dir.path().join("a.ts")).unwrap();
let diagnostics = did_open(
&mut client,
json!({
"textDocument": {
"uri": uri,
"languageId": "typescript",
"version": 1,
"text": "import { b } from \"/~/b.ts\";\n\nconsole.log(b);\n"
}
}),
);
let diagnostics = diagnostics.into_iter().flat_map(|x| x.diagnostics);
assert_eq!(diagnostics.count(), 0);
let (maybe_res, maybe_err) = client
.write_request::<_, _, Value>(
"textDocument/hover",
json!({
"textDocument": {
"uri": uri
},
"position": {
"line": 2,
"character": 12
}
}),
)
.unwrap();
assert!(maybe_err.is_none());
assert_eq!(
maybe_res,
Some(json!({
"contents": [
{
"language": "typescript",
"value":"(alias) const b: \"b\"\nimport b"
},
""
],
"range": {
"start": {
"line": 2,
"character": 12
},
"end": {
"line": 2,
"character": 13
}
}
}))
);
shutdown(&mut client);
}
#[test]
fn lsp_deno_task() {
let temp_dir = TempDir::new();

View file

@ -164,6 +164,17 @@ itest!(_033_import_map {
output: "run/033_import_map.out",
});
itest!(_033_import_map_in_config_file {
args: "run --reload --config=import_maps/config.json import_maps/test.ts",
output: "run/033_import_map_in_config_file.out",
});
itest!(_033_import_map_in_flag_has_precedence {
args: "run --quiet --reload --import-map=import_maps/import_map_invalid.json --config=import_maps/config.json import_maps/test.ts",
output: "run/033_import_map_in_flag_has_precedence.out",
exit_code: 1,
});
itest!(_033_import_map_remote {
args:
"run --quiet --reload --import-map=http://127.0.0.1:4545/import_maps/import_map_remote.json --unstable import_maps/test_remote.ts",

View file

@ -0,0 +1,15 @@
{
"importMap": "./import_map.json",
"imports": {
"moment": "./moment/moment.ts",
"moment/": "./moment/",
"lodash": "./lodash/lodash.ts",
"lodash/": "./lodash/",
"https://www.unpkg.com/vue/dist/vue.runtime.esm.js": "./vue.ts"
},
"scopes": {
"scope/": {
"moment": "./scoped_moment.ts"
}
}
}

View file

@ -0,0 +1,7 @@
{
"imports": {
"https://www.unpkg.com/vue/dist/vue.runtime.esm.js": "./vue.ts"
},
"scopes": {
}
}

View file

@ -0,0 +1,5 @@
{
"imports": {
"/~/": "./lib/"
}
}

View file

@ -0,0 +1,8 @@
Warning "importMap" setting is ignored when "imports" or "scopes" are specified in the config file.
Hello from remapped moment!
Hello from remapped moment dir!
Hello from remapped lodash!
Hello from remapped lodash dir!
Hello from remapped Vue!
Hello from scoped moment!
Hello from scoped!

View file

@ -0,0 +1 @@
error: Relative import path [WILDCARD] not prefixed with / or ./ or ../ and not in import map [WILDCARD]

View file

@ -19,10 +19,8 @@ use deno_core::error::generic_error;
use deno_core::error::AnyError;
use deno_core::resolve_url_or_path;
use deno_core::serde_json;
use deno_core::url::Url;
use deno_graph::ModuleSpecifier;
use deno_runtime::colors;
use deno_runtime::permissions::PermissionsContainer;
use std::env;
use std::fs;
use std::fs::File;
@ -166,22 +164,11 @@ async fn create_standalone_binary(
Some(CaData::Bytes(bytes)) => Some(bytes.clone()),
None => None,
};
let maybe_import_map: Option<(Url, String)> =
match ps.options.resolve_import_map_specifier()? {
None => None,
Some(import_map_specifier) => {
let file = ps
.file_fetcher
.fetch(&import_map_specifier, PermissionsContainer::allow_all())
.await
.context(format!(
"Unable to load '{}' import map",
import_map_specifier
))?;
Some((import_map_specifier, file.source.to_string()))
}
};
let maybe_import_map = ps
.options
.resolve_import_map(&ps.file_fetcher)
.await?
.map(|import_map| (import_map.base_url().clone(), import_map.to_json()));
let metadata = Metadata {
argv: compile_flags.args.clone(),
unstable: ps.options.unstable(),