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.

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: infer dependencies from package.json #22563

Merged
merged 5 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,12 +756,12 @@ impl Flags {
.ok()
}
Task(_) | Check(_) | Coverage(_) | Cache(_) | Info(_) | Eval(_)
| Test(_) | Bench(_) | Repl(_) | Compile(_) => {
| Test(_) | Bench(_) | Repl(_) | Compile(_) | Publish(_) => {
std::env::current_dir().ok()
}
Bundle(_) | Completions(_) | Doc(_) | Fmt(_) | Init(_) | Install(_)
| Uninstall(_) | Jupyter(_) | Lsp | Lint(_) | Types | Upgrade(_)
| Vendor(_) | Publish(_) => None,
| Vendor(_) => None,
}
}

Expand Down
18 changes: 12 additions & 6 deletions cli/tools/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::cache::ParsedSourceCache;
use crate::factory::CliFactory;
use crate::graph_util::ModuleGraphCreator;
use crate::http_util::HttpClient;
use crate::resolver::MappedSpecifierResolver;
use crate::tools::check::CheckOptions;
use crate::tools::lint::no_slow_types;
use crate::tools::registry::diagnostics::PublishDiagnostic;
Expand Down Expand Up @@ -85,7 +86,7 @@ async fn prepare_publish(
deno_json: &ConfigFile,
source_cache: Arc<ParsedSourceCache>,
graph: Arc<deno_graph::ModuleGraph>,
import_map: Arc<ImportMap>,
mapped_resolver: Arc<MappedSpecifierResolver>,
diagnostics_collector: &PublishDiagnosticsCollector,
) -> Result<Rc<PreparedPublishPackage>, AnyError> {
let config_path = deno_json.specifier.to_file_path().unwrap();
Expand Down Expand Up @@ -131,7 +132,7 @@ async fn prepare_publish(

let diagnostics_collector = diagnostics_collector.clone();
let tarball = deno_core::unsync::spawn_blocking(move || {
let unfurler = ImportMapUnfurler::new(&import_map);
let unfurler = ImportMapUnfurler::new(&mapped_resolver);
tar::create_gzipped_tarball(
&dir_path,
LazyGraphSourceParser::new(&source_cache, &graph),
Expand Down Expand Up @@ -654,7 +655,7 @@ async fn prepare_packages_for_publishing(
allow_slow_types: bool,
diagnostics_collector: &PublishDiagnosticsCollector,
deno_json: ConfigFile,
import_map: Arc<ImportMap>,
mapped_resolver: Arc<MappedSpecifierResolver>,
) -> Result<PreparePackagesData, AnyError> {
let members = deno_json.to_workspace_members()?;
let module_graph_creator = cli_factory.module_graph_creator().await?.as_ref();
Expand Down Expand Up @@ -684,15 +685,15 @@ async fn prepare_packages_for_publishing(
let results = members
.into_iter()
.map(|member| {
let import_map = import_map.clone();
let mapped_resolver = mapped_resolver.clone();
let graph = graph.clone();
async move {
let package = prepare_publish(
&member.package_name,
&member.config_file,
source_cache.clone(),
graph,
import_map,
mapped_resolver,
diagnostics_collector,
)
.await
Expand Down Expand Up @@ -806,6 +807,11 @@ pub async fn publish(
Arc::new(ImportMap::new(Url::parse("file:///dev/null").unwrap()))
});

let mapped_resolver = Arc::new(MappedSpecifierResolver::new(
Some(import_map),
cli_factory.package_json_deps_provider().clone(),
));

let directory_path = cli_factory.cli_options().initial_cwd();

let cli_options = cli_factory.cli_options();
Expand All @@ -823,7 +829,7 @@ pub async fn publish(
publish_flags.allow_slow_types,
&diagnostics_collector,
config_file.clone(),
import_map,
mapped_resolver,
)
.await?;

Expand Down
39 changes: 28 additions & 11 deletions cli/util/import_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use deno_graph::TypeScriptReference;
use deno_semver::jsr::JsrDepPackageReq;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::npm::NpmPackageReqReference;
use import_map::ImportMap;

use crate::resolver::MappedSpecifierResolver;

pub fn import_map_deps(value: &serde_json::Value) -> HashSet<JsrDepPackageReq> {
let Some(obj) = value.as_object() else {
Expand Down Expand Up @@ -95,11 +96,11 @@ impl ImportMapUnfurlDiagnostic {
}

pub struct ImportMapUnfurler<'a> {
import_map: &'a ImportMap,
import_map: &'a MappedSpecifierResolver,
}

impl<'a> ImportMapUnfurler<'a> {
pub fn new(import_map: &'a ImportMap) -> Self {
pub fn new(import_map: &'a MappedSpecifierResolver) -> Self {
Self { import_map }
}

Expand All @@ -117,10 +118,12 @@ impl<'a> ImportMapUnfurler<'a> {
text_changes: &mut Vec<deno_ast::TextChange>| {
let resolved = self.import_map.resolve(specifier, url);
if let Ok(resolved) = resolved {
text_changes.push(deno_ast::TextChange {
range: to_range(parsed_source, range),
new_text: make_relative_to(url, &resolved),
});
if let Some(resolved) = resolved.into_specifier() {
text_changes.push(deno_ast::TextChange {
range: to_range(parsed_source, range),
new_text: make_relative_to(url, &resolved),
});
}
}
};
for dep in &module_info.dependencies {
Expand Down Expand Up @@ -206,7 +209,7 @@ fn make_relative_to(from: &ModuleSpecifier, to: &ModuleSpecifier) -> String {
/// Attempts to unfurl the dynamic dependency returning `true` on success
/// or `false` when the import was not analyzable.
fn try_unfurl_dynamic_dep(
import_map: &ImportMap,
mapped_resolver: &MappedSpecifierResolver,
module_url: &lsp_types::Url,
parsed_source: &ParsedSource,
dep: &deno_graph::DynamicDependencyDescriptor,
Expand All @@ -220,10 +223,13 @@ fn try_unfurl_dynamic_dep(
let Some(relative_index) = maybe_relative_index else {
return false;
};
let resolved = import_map.resolve(value, module_url);
let resolved = mapped_resolver.resolve(value, module_url);
let Ok(resolved) = resolved else {
return false;
};
let Some(resolved) = resolved.into_specifier() else {
return false;
};
let start = range.start + relative_index;
text_changes.push(deno_ast::TextChange {
range: start..start + value.len(),
Expand All @@ -241,7 +247,10 @@ fn try_unfurl_dynamic_dep(
if !value.ends_with('/') {
return false;
}
let Ok(resolved) = import_map.resolve(value, module_url) else {
let Ok(resolved) = mapped_resolver.resolve(value, module_url) else {
return false;
};
let Some(resolved) = resolved.into_specifier() else {
return false;
};
let range = to_range(parsed_source, &dep.argument_range);
Expand Down Expand Up @@ -289,6 +298,10 @@ fn to_range(

#[cfg(test)]
mod tests {
use std::sync::Arc;

use crate::args::PackageJsonDepsProvider;

use super::*;
use deno_ast::MediaType;
use deno_ast::ModuleSpecifier;
Expand Down Expand Up @@ -323,7 +336,11 @@ mod tests {
});
let ImportMapWithDiagnostics { import_map, .. } =
import_map::parse_from_value(&deno_json_url, value).unwrap();
let unfurler = ImportMapUnfurler::new(&import_map);
let mapped_resolved = MappedSpecifierResolver::new(
Some(Arc::new(import_map)),
Arc::new(PackageJsonDepsProvider::new(None)),
);
let unfurler = ImportMapUnfurler::new(&mapped_resolved);

// Unfurling TS file should apply changes.
{
Expand Down
1 change: 1 addition & 0 deletions test_util/std
Submodule std added at e0ef24
1 change: 1 addition & 0 deletions test_util/wpt
Submodule wpt added at 00f2d3
9 changes: 9 additions & 0 deletions tests/integration/publish_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use deno_core::serde_json::json;
use test_util::assert_contains;
use test_util::assert_not_contains;
use test_util::env_vars_for_jsr_npm_tests;
use test_util::env_vars_for_jsr_tests;
use test_util::env_vars_for_npm_tests;
use test_util::itest;
Expand Down Expand Up @@ -147,6 +148,14 @@ itest!(javascript_decl_file {
exit_code: 0,
});

itest!(package_json {
args: "publish --token 'sadfasdf'",
output: "publish/package_json.out",
cwd: Some("publish/package_json"),
envs: env_vars_for_jsr_npm_tests(),
http_server: true,
});

itest!(successful {
args: "publish --token 'sadfasdf'",
output: "publish/successful.out",
Expand Down
8 changes: 8 additions & 0 deletions tests/testdata/publish/package_json.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Download http://localhost:4545/npm/registry/picocolors
Download http://localhost:4545/npm/registry/picocolors/picocolors-1.0.0.tgz
Check file:///[WILDCARD]/publish/package_json/mod.ts
Checking for slow types in the public API...
Check file:///[WILDCARD]/publish/package_json/mod.ts
Publishing @foo/bar@1.0.0 ...
Successfully published @foo/bar@1.0.0
Visit http://127.0.0.1:4250/@foo/bar@1.0.0 for details
8 changes: 8 additions & 0 deletions tests/testdata/publish/package_json/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@foo/bar",
"version": "1.0.0",
"exports": {
".": "./mod.ts"
},
"nodeModulesDir": false
}
9 changes: 9 additions & 0 deletions tests/testdata/publish/package_json/mod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import pc from "picocolors";

export function add(a: number, b: number): number {
return a + b;
}

export function getValue(): string {
return pc.green("hey");
}
7 changes: 7 additions & 0 deletions tests/testdata/publish/package_json/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@deno/foo",
"version": "0.0.1",
"dependencies": {
"picocolors": "*"
}
}
8 changes: 8 additions & 0 deletions tests/util/server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ pub fn env_vars_for_jsr_tests() -> Vec<(String, String)> {
]
}

pub fn env_vars_for_jsr_npm_tests() -> Vec<(String, String)> {
vec![
("NPM_CONFIG_REGISTRY".to_string(), npm_registry_url()),
("JSR_URL".to_string(), jsr_registry_url()),
("NO_COLOR".to_string(), "1".to_string()),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to add a new function because we need both the npm env + jsr env.

]
}

pub fn root_path() -> PathRef {
PathRef::new(
PathBuf::from(concat!(env!("CARGO_MANIFEST_DIR")))
Expand Down
Loading