From f96f167dc8cc733b43d67801f568b2d546bc2fb9 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 21 Mar 2024 14:42:23 -0700 Subject: [PATCH] feat(unstable/publish): error when a package's module is excluded from publishing (#22948) Closes #22657 --- cli/tools/registry/diagnostics.rs | 82 ++++++++++++------- cli/tools/registry/mod.rs | 32 ++++++++ .../jsr/excluded_export_module/__test__.jsonc | 7 ++ .../jsr/excluded_export_module/deno.json | 12 +++ .../excluded_export_module/excluded_file1.ts | 0 .../excluded_export_module/excluded_file2.ts | 0 .../specs/jsr/excluded_export_module/main.ts | 2 + .../not_imported_excluded_file.ts | 0 .../jsr/excluded_export_module/publish.out | 18 ++++ 9 files changed, 124 insertions(+), 29 deletions(-) create mode 100644 tests/specs/jsr/excluded_export_module/__test__.jsonc create mode 100644 tests/specs/jsr/excluded_export_module/deno.json create mode 100644 tests/specs/jsr/excluded_export_module/excluded_file1.ts create mode 100644 tests/specs/jsr/excluded_export_module/excluded_file2.ts create mode 100644 tests/specs/jsr/excluded_export_module/main.ts create mode 100644 tests/specs/jsr/excluded_export_module/not_imported_excluded_file.ts create mode 100644 tests/specs/jsr/excluded_export_module/publish.out diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs index 9e88e81de8..e3d24cd921 100644 --- a/cli/tools/registry/diagnostics.rs +++ b/cli/tools/registry/diagnostics.rs @@ -100,6 +100,9 @@ pub enum PublishDiagnostic { UnsupportedJsxTsx { specifier: Url, }, + ExcludedModule { + specifier: Url, + }, } impl PublishDiagnostic { @@ -145,6 +148,7 @@ impl Diagnostic for PublishDiagnostic { UnsupportedFileType { .. } => DiagnosticLevel::Warning, InvalidExternalImport { .. } => DiagnosticLevel::Error, UnsupportedJsxTsx { .. } => DiagnosticLevel::Warning, + ExcludedModule { .. } => DiagnosticLevel::Error, } } @@ -158,6 +162,7 @@ impl Diagnostic for PublishDiagnostic { UnsupportedFileType { .. } => Cow::Borrowed("unsupported-file-type"), InvalidExternalImport { .. } => Cow::Borrowed("invalid-external-import"), UnsupportedJsxTsx { .. } => Cow::Borrowed("unsupported-jsx-tsx"), + ExcludedModule { .. } => Cow::Borrowed("excluded-module"), } } @@ -175,6 +180,7 @@ impl Diagnostic for PublishDiagnostic { } InvalidExternalImport { kind, .. } => Cow::Owned(format!("invalid import to a {kind} specifier")), UnsupportedJsxTsx { .. } => Cow::Borrowed("JSX and TSX files are currently not supported"), + ExcludedModule { .. } => Cow::Borrowed("module in package's module graph was excluded from publishing"), } } @@ -217,13 +223,17 @@ impl Diagnostic for PublishDiagnostic { UnsupportedJsxTsx { specifier } => DiagnosticLocation::Module { specifier: Cow::Borrowed(specifier), }, + ExcludedModule { specifier } => DiagnosticLocation::Module { + specifier: Cow::Borrowed(specifier), + }, } } fn snippet(&self) -> Option> { + use PublishDiagnostic::*; match &self { - PublishDiagnostic::FastCheck(diagnostic) => diagnostic.snippet(), - PublishDiagnostic::SpecifierUnfurl(diagnostic) => match diagnostic { + FastCheck(diagnostic) => diagnostic.snippet(), + SpecifierUnfurl(diagnostic) => match diagnostic { SpecifierUnfurlerDiagnostic::UnanalyzableDynamicImport { text_info, range, @@ -240,10 +250,10 @@ impl Diagnostic for PublishDiagnostic { }, }), }, - PublishDiagnostic::InvalidPath { .. } => None, - PublishDiagnostic::DuplicatePath { .. } => None, - PublishDiagnostic::UnsupportedFileType { .. } => None, - PublishDiagnostic::InvalidExternalImport { + InvalidPath { .. } => None, + DuplicatePath { .. } => None, + UnsupportedFileType { .. } => None, + InvalidExternalImport { referrer, text_info, .. @@ -264,31 +274,37 @@ impl Diagnostic for PublishDiagnostic { description: Some("the specifier".into()), }, }), - PublishDiagnostic::UnsupportedJsxTsx { .. } => None, + UnsupportedJsxTsx { .. } => None, + ExcludedModule { .. } => None, } } fn hint(&self) -> Option> { + use PublishDiagnostic::*; match &self { - PublishDiagnostic::FastCheck(diagnostic) => diagnostic.hint(), - PublishDiagnostic::SpecifierUnfurl(_) => None, - PublishDiagnostic::InvalidPath { .. } => Some( + FastCheck(diagnostic) => diagnostic.hint(), + SpecifierUnfurl(_) => None, + InvalidPath { .. } => Some( Cow::Borrowed("rename or remove the file, or add it to 'publish.exclude' in the config file"), ), - PublishDiagnostic::DuplicatePath { .. } => Some( + DuplicatePath { .. } => Some( Cow::Borrowed("rename or remove the file"), ), - PublishDiagnostic::UnsupportedFileType { .. } => Some( + UnsupportedFileType { .. } => Some( Cow::Borrowed("remove the file, or add it to 'publish.exclude' in the config file"), ), - PublishDiagnostic::InvalidExternalImport { .. } => Some(Cow::Borrowed("replace this import with one from jsr or npm, or vendor the dependency into your package")), - PublishDiagnostic::UnsupportedJsxTsx { .. } => None, + InvalidExternalImport { .. } => Some(Cow::Borrowed("replace this import with one from jsr or npm, or vendor the dependency into your package")), + UnsupportedJsxTsx { .. } => None, + ExcludedModule { .. } => Some( + Cow::Borrowed("remove the module from 'exclude' and/or 'publish.exclude' in the config file"), + ), } } fn snippet_fixed(&self) -> Option> { + use PublishDiagnostic::*; match &self { - PublishDiagnostic::InvalidExternalImport { imported, .. } => { + InvalidExternalImport { imported, .. } => { match super::api::get_jsr_alternative(imported) { Some(replacement) => { let replacement = SourceTextInfo::new(replacement.into()); @@ -314,57 +330,65 @@ impl Diagnostic for PublishDiagnostic { } fn info(&self) -> Cow<'_, [Cow<'_, str>]> { + use PublishDiagnostic::*; match &self { - PublishDiagnostic::FastCheck(diagnostic) => { + FastCheck(diagnostic) => { diagnostic.info() } - PublishDiagnostic::SpecifierUnfurl(diagnostic) => match diagnostic { + SpecifierUnfurl(diagnostic) => match diagnostic { SpecifierUnfurlerDiagnostic::UnanalyzableDynamicImport { .. } => Cow::Borrowed(&[ Cow::Borrowed("after publishing this package, imports from the local import map / package.json do not work"), Cow::Borrowed("dynamic imports that can not be analyzed at publish time will not be rewritten automatically"), Cow::Borrowed("make sure the dynamic import is resolvable at runtime without an import map / package.json") ]), }, - PublishDiagnostic::InvalidPath { .. } => Cow::Borrowed(&[ + InvalidPath { .. } => Cow::Borrowed(&[ Cow::Borrowed("to portably support all platforms, including windows, the allowed characters in package paths are limited"), ]), - PublishDiagnostic::DuplicatePath { .. } => Cow::Borrowed(&[ + DuplicatePath { .. } => Cow::Borrowed(&[ Cow::Borrowed("to support case insensitive file systems, no two package paths may differ only by case"), ]), - PublishDiagnostic::UnsupportedFileType { .. } => Cow::Borrowed(&[ + UnsupportedFileType { .. } => Cow::Borrowed(&[ Cow::Borrowed("only files and directories are supported"), Cow::Borrowed("the file was ignored and will not be published") ]), - PublishDiagnostic::InvalidExternalImport { imported, .. } => Cow::Owned(vec![ + InvalidExternalImport { imported, .. } => Cow::Owned(vec![ Cow::Owned(format!("the import was resolved to '{}'", imported)), Cow::Borrowed("this specifier is not allowed to be imported on jsr"), Cow::Borrowed("jsr only supports importing `jsr:`, `npm:`, and `data:` specifiers"), ]), - PublishDiagnostic::UnsupportedJsxTsx { .. } => Cow::Owned(vec![ + UnsupportedJsxTsx { .. } => Cow::Owned(vec![ Cow::Borrowed("follow https://github.com/jsr-io/jsr/issues/24 for updates"), + ]), + ExcludedModule { .. } => Cow::Owned(vec![ + Cow::Borrowed("excluded modules referenced via a package export will error at runtime due to not existing in the package"), ]) } } fn docs_url(&self) -> Option> { + use PublishDiagnostic::*; match &self { - PublishDiagnostic::FastCheck(diagnostic) => diagnostic.docs_url(), - PublishDiagnostic::SpecifierUnfurl(diagnostic) => match diagnostic { + FastCheck(diagnostic) => diagnostic.docs_url(), + SpecifierUnfurl(diagnostic) => match diagnostic { SpecifierUnfurlerDiagnostic::UnanalyzableDynamicImport { .. } => None, }, - PublishDiagnostic::InvalidPath { .. } => { + InvalidPath { .. } => { Some(Cow::Borrowed("https://jsr.io/go/invalid-path")) } - PublishDiagnostic::DuplicatePath { .. } => Some(Cow::Borrowed( + DuplicatePath { .. } => Some(Cow::Borrowed( "https://jsr.io/go/case-insensitive-duplicate-path", )), - PublishDiagnostic::UnsupportedFileType { .. } => { + UnsupportedFileType { .. } => { Some(Cow::Borrowed("https://jsr.io/go/unsupported-file-type")) } - PublishDiagnostic::InvalidExternalImport { .. } => { + InvalidExternalImport { .. } => { Some(Cow::Borrowed("https://jsr.io/go/invalid-external-import")) } - PublishDiagnostic::UnsupportedJsxTsx { .. } => None, + UnsupportedJsxTsx { .. } => None, + ExcludedModule { .. } => { + Some(Cow::Borrowed("https://jsr.io/go/excluded-module")) + } } } } diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index cb3428b602..e323c6c794 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -9,6 +9,8 @@ use std::sync::Arc; use base64::prelude::BASE64_STANDARD; use base64::Engine; +use deno_ast::ModuleSpecifier; +use deno_config::glob::FilePatterns; use deno_config::ConfigFile; use deno_config::WorkspaceMemberConfig; use deno_core::anyhow::bail; @@ -151,6 +153,14 @@ async fn prepare_publish( sloppy_imports_resolver.as_ref(), bare_node_builtins, ); + let root_specifier = + ModuleSpecifier::from_directory_path(&dir_path).unwrap(); + collect_excluded_module_diagnostics( + &root_specifier, + &graph, + file_patterns.as_ref(), + &diagnostics_collector, + ); tar::create_gzipped_tarball( &dir_path, &cli_options, @@ -192,6 +202,28 @@ async fn prepare_publish( })) } +fn collect_excluded_module_diagnostics( + root: &ModuleSpecifier, + graph: &deno_graph::ModuleGraph, + file_patterns: Option<&FilePatterns>, + diagnostics_collector: &PublishDiagnosticsCollector, +) { + let Some(file_patterns) = file_patterns else { + return; + }; + let specifiers = graph + .specifiers() + .map(|(s, _)| s) + .filter(|s| s.as_str().starts_with(root.as_str())); + for specifier in specifiers { + if !file_patterns.matches_specifier(specifier) { + diagnostics_collector.push(PublishDiagnostic::ExcludedModule { + specifier: specifier.clone(), + }); + } + } +} + #[derive(Serialize)] #[serde(tag = "permission")] pub enum Permission<'s> { diff --git a/tests/specs/jsr/excluded_export_module/__test__.jsonc b/tests/specs/jsr/excluded_export_module/__test__.jsonc new file mode 100644 index 0000000000..2ff51f507a --- /dev/null +++ b/tests/specs/jsr/excluded_export_module/__test__.jsonc @@ -0,0 +1,7 @@ +// test for a module that's excluded from being +// published, but is found in the module graph +{ + "args": "publish --dry-run", + "output": "publish.out", + "exitCode": 1 +} diff --git a/tests/specs/jsr/excluded_export_module/deno.json b/tests/specs/jsr/excluded_export_module/deno.json new file mode 100644 index 0000000000..1254582ace --- /dev/null +++ b/tests/specs/jsr/excluded_export_module/deno.json @@ -0,0 +1,12 @@ +{ + "name": "@denotest/excluded-export-module", + "version": "0.1.0", + "exports": "./main.ts", + "publish": { + "exclude": [ + "./excluded_file1.ts", + "./excluded_file2.ts", + "./not_imported_excluded_file.ts" + ] + } +} diff --git a/tests/specs/jsr/excluded_export_module/excluded_file1.ts b/tests/specs/jsr/excluded_export_module/excluded_file1.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/specs/jsr/excluded_export_module/excluded_file2.ts b/tests/specs/jsr/excluded_export_module/excluded_file2.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/specs/jsr/excluded_export_module/main.ts b/tests/specs/jsr/excluded_export_module/main.ts new file mode 100644 index 0000000000..69bff30bf8 --- /dev/null +++ b/tests/specs/jsr/excluded_export_module/main.ts @@ -0,0 +1,2 @@ +import "./excluded_file1.ts"; +import "./excluded_file2.ts"; diff --git a/tests/specs/jsr/excluded_export_module/not_imported_excluded_file.ts b/tests/specs/jsr/excluded_export_module/not_imported_excluded_file.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/specs/jsr/excluded_export_module/publish.out b/tests/specs/jsr/excluded_export_module/publish.out new file mode 100644 index 0000000000..bcf94e5219 --- /dev/null +++ b/tests/specs/jsr/excluded_export_module/publish.out @@ -0,0 +1,18 @@ +Check file:///[WILDLINE]/main.ts +Checking for slow types in the public API... +Check file:///[WILDLINE]/main.ts +error[excluded-module]: module in package's module graph was excluded from publishing + --> [WILDLINE]excluded_file1.ts + = hint: remove the module from 'exclude' and/or 'publish.exclude' in the config file + + info: excluded modules referenced via a package export will error at runtime due to not existing in the package + docs: https://jsr.io/go/excluded-module + +error[excluded-module]: module in package's module graph was excluded from publishing + --> [WILDLINE]excluded_file2.ts + = hint: remove the module from 'exclude' and/or 'publish.exclude' in the config file + + info: excluded modules referenced via a package export will error at runtime due to not existing in the package + docs: https://jsr.io/go/excluded-module + +error: Found 2 problems