Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ext): expand scopes and expand when parsing #72

Merged
merged 5 commits into from
Feb 22, 2024
Merged

feat(ext): expand scopes and expand when parsing #72

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e89b0c3
feat(ext): expand scopes and expand when parsing
dsherret Feb 22, 2024
3a13910
Format
dsherret Feb 22, 2024
6ff09a0
Maybe fix
dsherret Feb 22, 2024
69cd90d
Fix variable naming
dsherret Feb 22, 2024
a584bf8
Move maybe_expand_imports down.
dsherret Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 86 additions & 67 deletions rs-lib/src/ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,64 +4,89 @@ use serde_json::json;
use serde_json::Value;
use url::Url;

/// This function can be used to modify the `import` mapping to expand
/// bare specifier imports to provide "directory" imports, eg.:
/// This function can be used to modify the `imports` and "scopes" mappings
/// to expand bare specifier imports to provide "directory" imports, eg.:
/// - `"express": "npm:express@4` -> `"express/": "npm:/express@4/`
/// - `"@std": "jsr:@std` -> `"std@/": "jsr:/@std/`
///
/// Only `npm:` and `jsr:` scheme are expanded and if there's already a
/// "directory" import, it is not overwritten.
pub fn expand_imports(import_map: ImportMapConfig) -> Value {
let mut expanded_imports = serde_json::Map::new();
pub fn expand_import_map_value(import_map: Value) -> Value {
let Value::Object(mut import_map) = import_map else {
return import_map;
};

let imports = import_map
.import_map_value
.get("imports")
.cloned()
.unwrap_or_else(|| Value::Null);
if let Some(imports) = import_map.get("imports").and_then(|i| i.as_object()) {
import_map.insert(
"imports".to_string(),
Value::Object(expand_imports(imports)),
);
}
if let Some(scope) = import_map.remove("scopes") {
match scope {
Value::Object(scope) => {
let mut expanded_scopes = serde_json::Map::with_capacity(scope.len());
for (key, value) in scope {
let expanded_value = match value {
Value::Object(value) => Value::Object(expand_imports(&value)),
_ => value,
};
expanded_scopes.insert(key, expanded_value);
}
import_map.insert("scopes".to_string(), Value::Object(expanded_scopes));
}
_ => {
import_map.insert("scopes".to_string(), scope);
}
}
}

if let Some(imports_map) = imports.as_object() {
for (key, value) in imports_map {
if !key.ends_with('/') {
expanded_imports.insert(key.to_string(), value.clone());
let key_with_trailing_slash = format!("{}/", key);
Value::Object(import_map)
}

// Don't overwrite existing keys
if imports_map.contains_key(&key_with_trailing_slash) {
continue;
}
fn expand_imports(
imports_map: &serde_json::Map<String, Value>,
) -> serde_json::Map<String, Value> {
let mut expanded_imports = serde_json::Map::new();
for (key, value) in imports_map {
if !key.ends_with('/') {
expanded_imports.insert(key.to_string(), value.clone());
let key_with_trailing_slash = format!("{}/", key);

let Some(value_str) = value.as_str() else {
continue;
};
// Don't overwrite existing keys
if imports_map.contains_key(&key_with_trailing_slash) {
continue;
}

if !value_str.ends_with('/') {
let value_with_trailing_slash =
if let Some(value_str) = value_str.strip_prefix("jsr:") {
let value_str = value_str.strip_prefix('/').unwrap_or(value_str);
Some(format!("jsr:/{}/", value_str))
} else if let Some(value_str) = value_str.strip_prefix("npm:") {
let value_str = value_str.strip_prefix('/').unwrap_or(value_str);
Some(format!("npm:/{}/", value_str))
} else {
None
};
let Some(value_str) = value.as_str() else {
continue;
};

if let Some(value_with_trailing_slash) = value_with_trailing_slash {
expanded_imports.insert(
key_with_trailing_slash,
Value::String(value_with_trailing_slash),
);
continue;
}
if !value_str.ends_with('/') {
let value_with_trailing_slash =
if let Some(value_str) = value_str.strip_prefix("jsr:") {
let value_str = value_str.strip_prefix('/').unwrap_or(value_str);
Some(format!("jsr:/{}/", value_str))
} else if let Some(value_str) = value_str.strip_prefix("npm:") {
let value_str = value_str.strip_prefix('/').unwrap_or(value_str);
Some(format!("npm:/{}/", value_str))
} else {
None
};

if let Some(value_with_trailing_slash) = value_with_trailing_slash {
expanded_imports.insert(
key_with_trailing_slash,
Value::String(value_with_trailing_slash),
);
continue;
}
}

expanded_imports.insert(key.to_string(), value.clone());
}
}

Value::Object(expanded_imports)
expanded_imports.insert(key.to_string(), value.clone());
}
expanded_imports
}

pub struct ImportMapConfig {
Expand Down Expand Up @@ -374,19 +399,16 @@ mod tests {

#[test]
fn test_expand_imports() {
let import_map = ImportMapConfig {
base_url: Url::parse("file:///import_map.json").unwrap(),
import_map_value: json!({
"imports": {
"@std": "jsr:/@std",
"@foo": "jsr:@foo",
"express": "npm:express@4",
"foo": "https://example.com/foo/bar"
},
"scopes": {}
}),
};
let value = expand_imports(import_map);
let import_map = json!({
"imports": {
"@std": "jsr:/@std",
"@foo": "jsr:@foo",
"express": "npm:express@4",
"foo": "https://example.com/foo/bar"
},
"scopes": {}
});
let value = expand_import_map_value(import_map);
assert_eq!(
value,
json!({
Expand All @@ -403,17 +425,14 @@ mod tests {

#[test]
fn test_expand_imports_with_trailing_slash() {
let import_map = ImportMapConfig {
base_url: Url::parse("file:///import_map.json").unwrap(),
import_map_value: json!({
"imports": {
"express": "npm:express@4",
"express/": "npm:/express@4/foo/bar/",
},
"scopes": {}
}),
};
let value = expand_imports(import_map);
let import_map = json!({
"imports": {
"express": "npm:express@4",
"express/": "npm:/express@4/foo/bar/",
},
"scopes": {}
});
let value = expand_import_map_value(import_map);
assert_eq!(
value,
json!({
Expand Down
117 changes: 71 additions & 46 deletions rs-lib/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,19 @@ pub struct ImportMapOptions {
/// `(parsed_address, key, maybe_scope) -> new_address`
#[allow(clippy::type_complexity)]
pub address_hook: Option<Box<dyn (Fn(&str, &str, Option<&str>) -> String)>>,
/// Whether to expand imports in the import map.
///
/// This functionality can be used to modify the import map
/// during parsing, by changing the `imports` mapping to expand
/// bare specifier imports to provide "directory" imports, eg.:
/// - `"express": "npm:express@4` -> `"express/": "npm:/express@4/`
/// - `"@std": "jsr:@std` -> `"std@/": "jsr:/@std/`
///
/// Only `npm:` and `jsr:` schemes are expanded and if there's already a
/// "directory" import, it is not overwritten.
///
/// This requires enabling the "ext" cargo feature.
pub expand_imports: bool,
}

impl Debug for ImportMapOptions {
Expand Down Expand Up @@ -565,40 +578,6 @@ impl ImportMap {
text.replace('"', "\\\"")
}
}

/// This function can be used to modify the import map in place,
/// by modifying `imports` mapping to expand
/// bare specifier imports to provide "directory" imports, eg.:
/// - `"express": "npm:express@4` -> `"express/": "npm:/express@4/`
/// - `"@std": "jsr:@std` -> `"std@/": "jsr:/@std/`
///
/// Only `npm:` and `jsr:` scheme are expanded and if there's already a
/// "directory" import, it is not overwritten.
#[cfg(feature = "ext")]
pub fn ext_expand_imports(&mut self) {
use ext::ImportMapConfig;

let json_str = self.to_json();
let json_value = serde_json::from_str(&json_str).unwrap();
let expanded_imports = ext::expand_imports(ImportMapConfig {
base_url: self.base_url.clone(),
import_map_value: json_value,
});
let expanded_imports_map = expanded_imports.as_object().unwrap();
let mut expanded_imports_im =
IndexMap::with_capacity(expanded_imports_map.len());
for (key, value) in expanded_imports_map {
expanded_imports_im
.insert(key.to_string(), Some(value.as_str().unwrap().to_string()));
}
let mut diagnostics = vec![];
let imports = parse_specifier_map(
expanded_imports_im,
&self.base_url,
&mut diagnostics,
);
self.imports = imports;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was starting to get complicated making this work. I realized we can just do this when parsing, which is more efficient and less code to make this work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't we should do it during parsing - we only want that for deno.json and deno.jsonc files - not the import map from --import-map flag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nvm, I see you added an option.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, deno_config will be able to handle it easily denoland/deno_config#41

}

pub fn parse_from_json(
Expand Down Expand Up @@ -664,6 +643,25 @@ fn parse_json(
options: &ImportMapOptions,
diagnostics: &mut Vec<ImportMapDiagnostic>,
) -> Result<(UnresolvedSpecifierMap, UnresolvedScopesMap), ImportMapError> {
fn maybe_expand_imports(value: Value, options: &ImportMapOptions) -> Value {
if options.expand_imports {
#[cfg(feature = "ext")]
{
ext::expand_import_map_value(value)
}
#[cfg(not(feature = "ext"))]
{
debug_assert!(
false,
"expand_imports was true, but the \"ext\" feature was not enabled"
);
value
}
} else {
value
}
}

let v: Value = match serde_json::from_str(json_string) {
Ok(v) => v,
Err(err) => {
Expand All @@ -673,6 +671,7 @@ fn parse_json(
)));
}
};
let v = maybe_expand_imports(v, options);
parse_value(v, options, diagnostics)
}

Expand Down Expand Up @@ -1216,21 +1215,47 @@ mod test {
"express": "npm:express@4",
"foo": "https://example.com/foo/bar"
},
"scopes": {}
"scopes": {
"./folder/": {
"@std": "jsr:/@std",
"@foo": "jsr:@foo",
"express": "npm:express@4",
"foo": "https://example.com/foo/bar"
},
}
}"#;
let im = parse_from_json(&url, json_string).unwrap();
let mut im = im.import_map;
im.ext_expand_imports();
let im = parse_from_json_with_options(
&url,
json_string,
ImportMapOptions {
address_hook: None,
expand_imports: true,
},
)
.unwrap();
assert_eq!(
serde_json::to_value(&im.imports).unwrap(),
serde_json::to_value(im.import_map).unwrap(),
serde_json::json!({
"@std": "jsr:/@std",
"@std/": "jsr:/@std/",
"@foo": "jsr:@foo",
"@foo/": "jsr:/@foo/",
"express": "npm:express@4",
"express/": "npm:/express@4/",
"foo": "https://example.com/foo/bar"
"imports": {
"@std": "jsr:/@std",
"@std/": "jsr:/@std/",
"@foo": "jsr:@foo",
"@foo/": "jsr:/@foo/",
"express": "npm:express@4",
"express/": "npm:/express@4/",
"foo": "https://example.com/foo/bar"
},
"scopes": {
"./folder/": {
"@std": "jsr:/@std",
"@std/": "jsr:/@std/",
"@foo": "jsr:@foo",
"@foo/": "jsr:/@foo/",
"express": "npm:express@4",
"express/": "npm:/express@4/",
"foo": "https://example.com/foo/bar"
},
}
})
);
}
Expand Down
1 change: 1 addition & 0 deletions rs-lib/tests/integration_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,7 @@ fn parse_with_address_hook() {
}
address.to_string()
})),
expand_imports: false,
},
)
.unwrap();
Expand Down
Loading