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

fix(publish): silence warnings for sloppy imports and node builtins with env var #22760

Merged
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
6 changes: 6 additions & 0 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ pub fn npm_registry_url() -> &'static Url {
&NPM_REGISTRY_DEFAULT_URL
}

pub static DENO_DISABLE_PEDANTIC_NODE_WARNINGS: Lazy<bool> = Lazy::new(|| {
std::env::var("DENO_DISABLE_PEDANTIC_NODE_WARNINGS")
.ok()
.is_some()
});
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved

pub fn jsr_url() -> &'static Url {
static JSR_URL: Lazy<Url> = Lazy::new(|| {
let env_var_name = "JSR_URL";
Expand Down
5 changes: 4 additions & 1 deletion cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::args::jsr_url;
use crate::args::CliOptions;
use crate::args::Lockfile;
use crate::args::TsTypeLib;
use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS;
use crate::cache;
use crate::cache::GlobalHttpCache;
use crate::cache::ModuleInfoCache;
Expand Down Expand Up @@ -681,9 +682,11 @@ pub fn enhanced_resolution_error_message(error: &ResolutionError) -> String {
let mut message = format!("{error}");

if let Some(specifier) = get_resolution_error_bare_node_specifier(error) {
message.push_str(&format!(
if !*DENO_DISABLE_PEDANTIC_NODE_WARNINGS {
message.push_str(&format!(
"\nIf you want to use a built-in Node module, add a \"node:\" prefix (ex. \"node:{specifier}\")."
));
}
}

message
Expand Down
30 changes: 18 additions & 12 deletions cli/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use std::sync::Arc;
use crate::args::package_json::PackageJsonDeps;
use crate::args::JsxImportSourceConfig;
use crate::args::PackageJsonDepsProvider;
use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS;
use crate::colors;
use crate::node::CliNodeCodeTranslator;
use crate::npm::ByonmCliNpmResolver;
Expand Down Expand Up @@ -725,17 +726,20 @@ fn sloppy_imports_resolve(
};
// show a warning when this happens in order to drive
// the user towards correcting these specifiers
log::warn!(
"{} Sloppy module resolution {}\n at {}",
crate::colors::yellow("Warning"),
crate::colors::gray(format!("(hint: {})", hint_message)).to_string(),
if referrer_range.end == deno_graph::Position::zeroed() {
// not worth showing the range in this case
crate::colors::cyan(referrer_range.specifier.as_str()).to_string()
} else {
format_range_with_colors(referrer_range)
},
);
if !*DENO_DISABLE_PEDANTIC_NODE_WARNINGS {
log::warn!(
"{} Sloppy module resolution {}\n at {}",
crate::colors::yellow("Warning"),
crate::colors::gray(format!("(hint: {})", hint_message)).to_string(),
if referrer_range.end == deno_graph::Position::zeroed() {
// not worth showing the range in this case
crate::colors::cyan(referrer_range.specifier.as_str()).to_string()
} else {
format_range_with_colors(referrer_range)
},
);
}

resolution.into_specifier().into_owned()
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -788,7 +792,9 @@ impl NpmResolver for CliGraphResolver {
} = range;
let line = start.line + 1;
let column = start.character + 1;
log::warn!("Warning: Resolving \"{module_name}\" as \"node:{module_name}\" at {specifier}:{line}:{column}. If you want to use a built-in Node module, add a \"node:\" prefix.")
if !*DENO_DISABLE_PEDANTIC_NODE_WARNINGS {
log::warn!("Warning: Resolving \"{module_name}\" as \"node:{module_name}\" at {specifier}:{line}:{column}. If you want to use a built-in Node module, add a \"node:\" prefix.")
}
}

fn load_and_cache_npm_package_info(
Expand Down
34 changes: 34 additions & 0 deletions tests/integration/publish_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,23 @@ itest!(bare_node_builtins {
http_server: true,
});

itest!(bare_node_builtins_warning_no_warnings {
args: "publish --token 'sadfasdf' --dry-run --unstable-bare-node-builtins",
output: "publish/bare_node_builtins_no_warnings.out",
cwd: Some("publish/bare_node_builtins"),
envs: env_vars_for_jsr_npm_tests()
.into_iter()
.chain(
vec![(
"DENO_DISABLE_PEDANTIC_NODE_WARNINGS".to_string(),
"1".to_string()
)]
.into_iter()
)
.collect(),
http_server: true,
});

itest!(sloppy_imports {
args: "publish --token 'sadfasdf' --dry-run --unstable-sloppy-imports",
output: "publish/sloppy_imports.out",
Expand All @@ -258,6 +275,23 @@ itest!(sloppy_imports {
http_server: true,
});

itest!(sloppy_imports_no_warnings {
args: "publish --token 'sadfasdf' --dry-run --unstable-sloppy-imports",
output: "publish/sloppy_imports_no_warnings.out",
cwd: Some("publish/sloppy_imports"),
envs: env_vars_for_jsr_tests()
.into_iter()
.chain(
vec![(
"DENO_DISABLE_PEDANTIC_NODE_WARNINGS".to_string(),
"1".to_string()
)]
.into_iter()
)
.collect(),
http_server: true,
});

itest!(jsr_jsonc {
args: "publish --token 'sadfasdf'",
cwd: Some("publish/jsr_jsonc"),
Expand Down
9 changes: 9 additions & 0 deletions tests/testdata/publish/bare_node_builtins_no_warnings.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Download http://localhost:4545/npm/registry/@types/node
Download http://localhost:4545/npm/registry/@types/node/node-18.8.2.tgz
Check file:///[WILDCARD]/publish/bare_node_builtins/mod.ts
Checking for slow types in the public API...
Check file:///[WILDCARD]/publish/bare_node_builtins/mod.ts
Simulating publish of @foo/bar@1.0.0 with files:
file:///[WILDCARD]/publish/bare_node_builtins/deno.json (87B)
file:///[WILDCARD]/publish/bare_node_builtins/mod.ts (121B)
Warning Aborting due to --dry-run
8 changes: 8 additions & 0 deletions tests/testdata/publish/sloppy_imports_no_warnings.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Check file:///[WILDCARD]/publish/sloppy_imports/mod.ts
Checking for slow types in the public API...
Check file:///[WILDCARD]/publish/sloppy_imports/mod.ts
Simulating publish of @foo/bar@1.0.0 with files:
file:///[WILDCARD]/publish/sloppy_imports/b/index.ts (27B)
file:///[WILDCARD]/publish/sloppy_imports/deno.json (87B)
file:///[WILDCARD]/publish/sloppy_imports/mod.ts (35B)
Warning Aborting due to --dry-run