Skip to content
Open
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ deno_graph = { workspace = true, features = ["fast_check"] }
deno_lib.workspace = true
deno_lint.workspace = true
deno_lockfile.workspace = true
deno_maybe_sync.workspace = true
deno_media_type = { workspace = true, features = ["data_url", "decoding", "module_specifier"] }
deno_npm.workspace = true
deno_npm_cache.workspace = true
Expand Down
9 changes: 1 addition & 8 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1282,14 +1282,7 @@ fn new_workspace_factory_options(
flags: &Flags,
) -> deno_resolver::factory::WorkspaceFactoryOptions {
deno_resolver::factory::WorkspaceFactoryOptions {
additional_config_file_names: if matches!(
flags.subcommand,
DenoSubcommand::Publish(..)
) {
&["jsr.json", "jsr.jsonc"]
} else {
&[]
},
discover_jsr_config: true,
config_discovery: match &flags.config_flag {
ConfigFlag::Discover => {
if let Some(start_paths) = flags.config_path_args(initial_cwd) {
Expand Down
4 changes: 2 additions & 2 deletions cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1284,7 +1284,7 @@ impl ConfigData {
}
},
&WorkspaceDiscoverOptions {
additional_config_file_names: &[],
discover_jsr_config: true,
deno_json_cache: Some(deno_json_cache),
pkg_json_cache: Some(pkg_json_cache),
workspace_cache: Some(workspace_cache),
Expand Down Expand Up @@ -1428,7 +1428,7 @@ impl ConfigData {
CliSys::default(),
member_dir.dir_path(),
WorkspaceFactoryOptions {
additional_config_file_names: &[],
discover_jsr_config: true,
config_discovery: ConfigDiscoveryOption::DiscoverCwd,
maybe_custom_deno_dir_root: None,
is_package_manager_subcommand: false,
Expand Down
32 changes: 31 additions & 1 deletion cli/tools/publish/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ pub enum PublishDiagnostic {
MissingLicense {
config_specifier: Url,
},
ConflictingPublishConfig {
primary_specifier: Url,
specifiers: Vec<Url>,
},
}

impl PublishDiagnostic {
Expand Down Expand Up @@ -193,6 +197,7 @@ impl Diagnostic for PublishDiagnostic {
SyntaxError { .. } => DiagnosticLevel::Error,
MissingLicense { .. } => DiagnosticLevel::Error,
UnstableRawImport { .. } => DiagnosticLevel::Error,
ConflictingPublishConfig { .. } => DiagnosticLevel::Error,
}
}

Expand All @@ -214,6 +219,9 @@ impl Diagnostic for PublishDiagnostic {
SyntaxError { .. } => Cow::Borrowed("syntax-error"),
MissingLicense { .. } => Cow::Borrowed("missing-license"),
UnstableRawImport { .. } => Cow::Borrowed("unstable-raw-import"),
ConflictingPublishConfig { .. } => {
Cow::Borrowed("conflicting-publish-config")
}
}
}

Expand Down Expand Up @@ -255,6 +263,14 @@ impl Diagnostic for PublishDiagnostic {
UnstableRawImport { .. } => {
Cow::Borrowed("raw imports have not been stabilized")
}
ConflictingPublishConfig { specifiers, .. } => Cow::Owned(format!(
"Publish configuration is defined in multiple files: {}.",
specifiers
.iter()
.map(|s| s.to_string())
.collect::<Vec<_>>()
.join(", ")
)),
}
}

Expand Down Expand Up @@ -328,6 +344,11 @@ impl Diagnostic for PublishDiagnostic {
MissingLicense { config_specifier } => DiagnosticLocation::Module {
specifier: Cow::Borrowed(config_specifier),
},
ConflictingPublishConfig {
primary_specifier, ..
} => DiagnosticLocation::Module {
specifier: Cow::Borrowed(primary_specifier),
},
}
}

Expand Down Expand Up @@ -402,6 +423,7 @@ impl Diagnostic for PublishDiagnostic {
}
SyntaxError(diagnostic) => diagnostic.snippet(),
MissingLicense { .. } => None,
ConflictingPublishConfig { .. } => None,
}
}

Expand Down Expand Up @@ -448,6 +470,9 @@ impl Diagnostic for PublishDiagnostic {
UnstableRawImport { .. } => Some(Cow::Borrowed(
"for the time being, embed the data directly into a JavaScript file (ex. as encoded base64 text)",
)),
ConflictingPublishConfig { .. } => Some(Cow::Borrowed(
"Remove the conflicting configuration so that only one file defines the package metadata",
)),
}
}

Expand Down Expand Up @@ -486,7 +511,8 @@ impl Diagnostic for PublishDiagnostic {
| MissingConstraint { .. }
| BannedTripleSlashDirectives { .. }
| MissingLicense { .. }
| UnstableRawImport { .. } => None,
| UnstableRawImport { .. }
| ConflictingPublishConfig { .. } => None,
}
}

Expand Down Expand Up @@ -541,6 +567,7 @@ impl Diagnostic for PublishDiagnostic {
SyntaxError(diagnostic) => diagnostic.info(),
MissingLicense { .. } => Cow::Borrowed(&[]),
UnstableRawImport { .. } => Cow::Borrowed(&[]),
ConflictingPublishConfig { .. } => Cow::Borrowed(&[]),
}
}

Expand Down Expand Up @@ -580,6 +607,9 @@ impl Diagnostic for PublishDiagnostic {
UnstableRawImport { .. } => Some(Cow::Borrowed(
"https://github.com/denoland/deno/issues/29904",
)),
ConflictingPublishConfig { .. } => {
Some(Cow::Borrowed("https://jsr.io/docs/package-configuration"))
}
}
}
}
104 changes: 91 additions & 13 deletions cli/tools/publish/mod.rs
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look like it will work when publishing a workspace. For example, someone might publish a workspace with members containing a deno.json and jsr.json -- Maybe we need more of a first class solution in libs/config instead of trying to fix this in the cli.

Copy link
Member

@dsherret dsherret Nov 29, 2025

Choose a reason for hiding this comment

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

For example, maybe we can get rid of additional_config_file_names and instead have a more specific discover_jsr_config: true flag that will surface a WorkspaceDiagnostic for this scenario.

Related is my comment here: https://github.com/denoland/deno/pull/31383/files#r2572813359 -- fixing that will help fix this.

Copy link
Author

Choose a reason for hiding this comment

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

Ahh yes, I see what you mean with workspaces. This only works as a solution for the root package basically.

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use deno_ast::SourceTextInfo;
use deno_config::deno_json::ConfigFile;
use deno_config::workspace::JsrPackageConfig;
use deno_config::workspace::Workspace;
use deno_config::workspace::WorkspaceDiagnosticKind;
use deno_core::anyhow::Context;
use deno_core::anyhow::bail;
use deno_core::error::AnyError;
Expand Down Expand Up @@ -85,22 +86,46 @@ pub async fn publish(

let cli_options = cli_factory.cli_options()?;
let directory_path = cli_options.initial_cwd();
let diagnostics_collector = PublishDiagnosticsCollector::default();
// Convert workspace diagnostics to publish diagnostics
for workspace_diagnostic in cli_options.start_dir.workspace.diagnostics() {
if let WorkspaceDiagnosticKind::ConflictingPublishConfig {
deno_config_url,
jsr_config_url,
} = &workspace_diagnostic.kind
{
let mut specifiers =
vec![deno_config_url.clone(), jsr_config_url.clone()];
specifiers.sort();
specifiers.dedup();
diagnostics_collector.push(PublishDiagnostic::ConflictingPublishConfig {
primary_specifier: jsr_config_url.clone(),
specifiers,
});
}
}
if diagnostics_collector.has_error() {
return diagnostics_collector.print_and_error();
}
let mut publish_configs = cli_options.start_dir.jsr_packages_for_publish();
if publish_configs.is_empty() {
match cli_options.start_dir.maybe_deno_json() {
Some(deno_json) => {
debug_assert!(!deno_json.is_package());
if deno_json.json.name.is_none() {
bail!("Missing 'name' field in '{}'.", deno_json.specifier);
}
error_missing_exports_field(deno_json)?;
if let Some(deno_json) = cli_options.start_dir.maybe_deno_json() {
debug_assert!(!deno_json.is_package());
if deno_json.json.name.is_none() {
bail!("Missing 'name' field in '{}'.", deno_json.specifier);
}
None => {
bail!(
"Couldn't find a deno.json, deno.jsonc, jsr.json or jsr.jsonc configuration file in {}.",
directory_path.display()
);
error_missing_exports_field(deno_json)?;
} else if let Some(jsr_json) = cli_options.start_dir.maybe_jsr_json() {
debug_assert!(!jsr_json.is_package());
if jsr_json.json.name.is_none() {
bail!("Missing 'name' field in '{}'.", jsr_json.specifier);
}
error_missing_exports_field(jsr_json)?;
} else {
bail!(
"Couldn't find a deno.json, deno.jsonc, jsr.json or jsr.jsonc configuration file in {}.",
directory_path.display()
);
}
}

Expand All @@ -123,7 +148,6 @@ pub async fn publish(
cli_options.unstable_bare_node_builtins(),
);

let diagnostics_collector = PublishDiagnosticsCollector::default();
let parsed_source_cache = cli_factory.parsed_source_cache()?;
let module_content_provider = Arc::new(ModuleContentProvider::new(
parsed_source_cache.clone(),
Expand Down Expand Up @@ -1357,7 +1381,10 @@ mod tests {
use std::collections::HashMap;

use deno_ast::ModuleSpecifier;
use deno_ast::diagnostics::Diagnostic;
use deno_core::url::Url;

use super::PublishDiagnostic;
use super::has_license_file;
use super::tar::PublishableTarball;
use super::tar::PublishableTarballFile;
Expand Down Expand Up @@ -1486,4 +1513,55 @@ mod tests {
"file:///test/tLICENSE"
]),);
}

#[test]
fn test_conflicting_publish_config_diagnostic_lists_specifiers() {
let deno_specifier = Url::parse("file:///example/deno.json").unwrap();
let jsr_specifier = Url::parse("file:///example/jsr.json").unwrap();
let diagnostic = PublishDiagnostic::ConflictingPublishConfig {
primary_specifier: jsr_specifier.clone(),
specifiers: vec![deno_specifier.clone(), jsr_specifier.clone()],
};
let message = diagnostic.message();
assert!(message.contains("deno.json"));
assert!(message.contains("jsr.json"));
let hint = diagnostic.hint().expect("expected hint");
assert!(hint.contains("Remove the conflicting configuration"));
}

#[test]
fn test_workspace_diagnostics_converted_to_publish_diagnostics() {
use deno_ast::diagnostics::DiagnosticLevel;
use deno_config::workspace::WorkspaceDiagnosticKind;

let deno_config_url = Url::parse("file:///example/deno.json").unwrap();
let jsr_config_url = Url::parse("file:///example/jsr.json").unwrap();

let workspace_diagnostic_kind =
WorkspaceDiagnosticKind::ConflictingPublishConfig {
deno_config_url: deno_config_url.clone(),
jsr_config_url: jsr_config_url.clone(),
};

// Simulate the conversion logic from publish()
let mut specifiers = vec![deno_config_url.clone(), jsr_config_url.clone()];
specifiers.sort();
specifiers.dedup();

let publish_diagnostic = PublishDiagnostic::ConflictingPublishConfig {
primary_specifier: jsr_config_url.clone(),
specifiers: specifiers.clone(),
};

// Verify the diagnostic was created correctly
assert!(matches!(
workspace_diagnostic_kind,
WorkspaceDiagnosticKind::ConflictingPublishConfig { .. }
));
assert!(matches!(publish_diagnostic.level(), DiagnosticLevel::Error));
let message = publish_diagnostic.message();
assert!(message.contains("deno.json"));
assert!(message.contains("jsr.json"));
assert_eq!(specifiers.len(), 2);
}
}
Loading
Loading