diff options
author | David Sherret <dsherret@users.noreply.github.com> | 2024-07-17 09:13:22 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-17 09:13:22 -0400 |
commit | f4b9d8586215fc07c28998e5d896fefa876139b7 (patch) | |
tree | 9ee42eb4bb62af04b1c3b049cd179dfa6fe908bb /cli | |
parent | 078def0ff8501bb07f3f286515acd8c6a2181037 (diff) |
fix(workspace): support resolving bare specifiers to npm pkgs within a workspace (#24611)
This makes bare specifiers for npm packages work when inside a
workspace, which emulates the same behaviour as when there's a
node_modules directory. The bare specifier can be overwritten by
specifying an import map entry or package.json dependency entry.
* https://github.com/denoland/deno_config/pull/88
Closes #24605
Diffstat (limited to 'cli')
-rw-r--r-- | cli/lsp/config.rs | 49 | ||||
-rw-r--r-- | cli/lsp/resolver.rs | 7 | ||||
-rw-r--r-- | cli/resolver.rs | 16 | ||||
-rw-r--r-- | cli/standalone/mod.rs | 22 | ||||
-rw-r--r-- | cli/tools/registry/unfurl.rs | 62 | ||||
-rw-r--r-- | cli/tools/vendor/test.rs | 3 |
6 files changed, 124 insertions, 35 deletions
diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index d207b81a9..861a63d0c 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -1117,6 +1117,7 @@ pub struct ConfigData { pub import_map_from_settings: bool, pub package_config: Option<Arc<LspPackageConfig>>, pub is_workspace_root: bool, + pub workspace_root_dir: ModuleSpecifier, /// Workspace member directories. For a workspace root this will be a list of /// members. For a member this will be the same list, representing self and /// siblings. For a solitary package this will be `vec![self.scope]`. These @@ -1532,28 +1533,37 @@ impl ConfigData { }) }); - let workspace = config_file + let workspace_config = config_file .as_ref() .and_then(|c| c.to_workspace_config().ok().flatten().map(|w| (c, w))); - let is_workspace_root = workspace.is_some(); - let workspace_members = if let Some((config, workspace)) = workspace { - Arc::new( - workspace - .members - .iter() - .flat_map(|p| { - let dir_specifier = config.specifier.join(p).ok()?; - let dir_path = specifier_to_file_path(&dir_specifier).ok()?; - Url::from_directory_path(normalize_path(dir_path)).ok() - }) - .collect(), - ) - } else if let Some(workspace_data) = workspace_root { - workspace_data.workspace_members.clone() - } else if config_file.as_ref().is_some_and(|c| c.json.name.is_some()) { - Arc::new(vec![scope.clone()]) + let is_workspace_root = workspace_config.is_some(); + let workspace_members = + if let Some((config, workspace_config)) = workspace_config { + Arc::new( + workspace_config + .members + .iter() + .flat_map(|p| { + let dir_specifier = config.specifier.join(p).ok()?; + let dir_path = specifier_to_file_path(&dir_specifier).ok()?; + Url::from_directory_path(normalize_path(dir_path)).ok() + }) + .collect(), + ) + } else if let Some(workspace_data) = workspace_root { + workspace_data.workspace_members.clone() + } else if config_file.as_ref().is_some_and(|c| c.json.name.is_some()) { + Arc::new(vec![scope.clone()]) + } else { + Arc::new(vec![]) + }; + let workspace_root_dir = if is_workspace_root { + scope.clone() } else { - Arc::new(vec![]) + workspace_root + .as_ref() + .map(|r| r.scope.clone()) + .unwrap_or_else(|| scope.clone()) }; ConfigData { @@ -1574,6 +1584,7 @@ impl ConfigData { import_map_from_settings, package_config: package_config.map(Arc::new), is_workspace_root, + workspace_root_dir, workspace_members, watched_files, } diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 21b46255c..2ca93114d 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -529,6 +529,13 @@ fn create_graph_resolver( node_resolver: node_resolver.cloned(), npm_resolver: npm_resolver.cloned(), workspace_resolver: Arc::new(WorkspaceResolver::new_raw( + Arc::new( + config_data + .map(|d| d.workspace_root_dir.clone()) + // this is fine because this value is only used to filter bare + // specifier resolution to workspace npm packages when in a workspace + .unwrap_or_else(|| ModuleSpecifier::parse("file:///").unwrap()), + ), config_data.and_then(|d| d.import_map.as_ref().map(|i| (**i).clone())), config_data .and_then(|d| d.package_json.clone()) diff --git a/cli/resolver.rs b/cli/resolver.rs index 6049ec273..7c47795c4 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -536,6 +536,22 @@ impl Resolver for CliGraphResolver { Ok(resolution) => match resolution { MappedResolution::Normal(specifier) | MappedResolution::ImportMap(specifier) => Ok(specifier), + MappedResolution::WorkspaceNpmPackage { + target_pkg_json: pkg_json, + sub_path, + .. + } => self + .node_resolver + .as_ref() + .unwrap() + .resolve_package_sub_path_from_deno_module( + pkg_json.dir_path(), + sub_path.as_deref(), + Some(referrer), + to_node_mode(mode), + ) + .map_err(ResolveError::Other) + .map(|res| res.into_url()), // todo(dsherret): for byonm it should do resolution solely based on // the referrer and not the package.json MappedResolution::PackageJson { diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 1df3895ef..e0c8e66ff 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -88,7 +88,7 @@ struct WorkspaceEszipModule { struct WorkspaceEszip { eszip: eszip::EszipV2, - root_dir_url: ModuleSpecifier, + root_dir_url: Arc<ModuleSpecifier>, } impl WorkspaceEszip { @@ -166,6 +166,22 @@ impl ModuleLoader for EmbeddedModuleLoader { self.shared.workspace_resolver.resolve(specifier, &referrer); match mapped_resolution { + Ok(MappedResolution::WorkspaceNpmPackage { + target_pkg_json: pkg_json, + sub_path, + .. + }) => Ok( + self + .shared + .node_resolver + .resolve_package_sub_path_from_deno_module( + pkg_json.dir_path(), + sub_path.as_deref(), + Some(&referrer), + NodeResolutionMode::Execution, + )? + .into_url(), + ), Ok(MappedResolution::PackageJson { dep_result, sub_path, @@ -427,7 +443,8 @@ pub async fn run( let npm_registry_url = ModuleSpecifier::parse("https://localhost/").unwrap(); let root_path = std::env::temp_dir().join(format!("deno-compile-{}", current_exe_name)); - let root_dir_url = ModuleSpecifier::from_directory_path(&root_path).unwrap(); + let root_dir_url = + Arc::new(ModuleSpecifier::from_directory_path(&root_path).unwrap()); let main_module = root_dir_url.join(&metadata.entrypoint_key).unwrap(); let root_node_modules_path = root_path.join("node_modules"); let npm_cache_dir = NpmCacheDir::new( @@ -579,6 +596,7 @@ pub async fn run( }) .collect(); WorkspaceResolver::new_raw( + root_dir_url.clone(), import_map, pkg_jsons, metadata.workspace_resolver.pkg_json_resolution, diff --git a/cli/tools/registry/unfurl.rs b/cli/tools/registry/unfurl.rs index 758db0796..63c773f01 100644 --- a/cli/tools/registry/unfurl.rs +++ b/cli/tools/registry/unfurl.rs @@ -75,26 +75,59 @@ impl SpecifierUnfurler { match resolved { MappedResolution::Normal(specifier) | MappedResolution::ImportMap(specifier) => Some(specifier), + MappedResolution::WorkspaceNpmPackage { + target_pkg_json: pkg_json, + pkg_name, + sub_path, + } => { + // todo(#24612): consider warning or error when this is also a jsr package? + ModuleSpecifier::parse(&format!( + "npm:{}{}{}", + pkg_name, + pkg_json + .version + .as_ref() + .map(|v| format!("@^{}", v)) + .unwrap_or_default(), + sub_path + .as_ref() + .map(|s| format!("/{}", s)) + .unwrap_or_default() + )) + .ok() + } MappedResolution::PackageJson { + alias, sub_path, dep_result, .. } => match dep_result { Ok(dep) => match dep { - PackageJsonDepValue::Req(req) => ModuleSpecifier::parse(&format!( - "npm:{}{}", - req, - sub_path - .as_ref() - .map(|s| format!("/{}", s)) - .unwrap_or_default() - )) - .ok(), - PackageJsonDepValue::Workspace(_) => { - log::warn!( - "package.json workspace entries are not implemented yet for publishing." - ); - None + PackageJsonDepValue::Req(pkg_req) => { + // todo(#24612): consider warning or error when this is an npm workspace + // member that's also a jsr package? + ModuleSpecifier::parse(&format!( + "npm:{}{}", + pkg_req, + sub_path + .as_ref() + .map(|s| format!("/{}", s)) + .unwrap_or_default() + )) + .ok() + } + PackageJsonDepValue::Workspace(version_req) => { + // todo(#24612): consider warning or error when this is also a jsr package? + ModuleSpecifier::parse(&format!( + "npm:{}@{}{}", + alias, + version_req, + sub_path + .as_ref() + .map(|s| format!("/{}", s)) + .unwrap_or_default() + )) + .ok() } }, Err(err) => { @@ -401,6 +434,7 @@ mod tests { }), ); let workspace_resolver = WorkspaceResolver::new_raw( + Arc::new(ModuleSpecifier::from_directory_path(&cwd).unwrap()), Some(import_map), vec![Arc::new(package_json)], deno_config::workspace::PackageJsonDepResolution::Enabled, diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index ac07c47d1..dc851858e 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -234,6 +234,7 @@ impl VendorTestBuilder { let loader = self.loader.clone(); let parsed_source_cache = ParsedSourceCache::default(); let resolver = Arc::new(build_resolver( + output_dir.parent().unwrap(), self.jsx_import_source_config.clone(), self.maybe_original_import_map.clone(), )); @@ -287,6 +288,7 @@ impl VendorTestBuilder { } fn build_resolver( + root_dir: &Path, maybe_jsx_import_source_config: Option<JsxImportSourceConfig>, maybe_original_import_map: Option<ImportMap>, ) -> CliGraphResolver { @@ -295,6 +297,7 @@ fn build_resolver( npm_resolver: None, sloppy_imports_resolver: None, workspace_resolver: Arc::new(WorkspaceResolver::new_raw( + Arc::new(ModuleSpecifier::from_directory_path(root_dir).unwrap()), maybe_original_import_map, Vec::new(), deno_config::workspace::PackageJsonDepResolution::Enabled, |