Skip to content

Commit

Permalink
feat: infer dependencies from package.json (#22563)
Browse files Browse the repository at this point in the history
<!--
Before submitting a PR, please read
https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(std/http): Fix race condition in server
    - docs(console): Update docstrings
    - feat(doc): Handle nested reexports

  Examples of bad title:
    - fix #7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
7. Open as a draft PR if your work is still in progress. The CI won't
run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench'
label.
-->
This PR enhances the `deno publish` command to infer dependencies from
`package.json` if present.
  • Loading branch information
marvinhagemeister committed Feb 23, 2024
1 parent 55fa61a commit cddefec
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 19 deletions.
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()),
]
}

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

0 comments on commit cddefec

Please sign in to comment.