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: Start warning on each use of a deprecated API #21939

Merged
merged 17 commits into from Jan 18, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions cli/args/mod.rs
Expand Up @@ -682,6 +682,7 @@ pub struct CliOptions {
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
overrides: CliOptionOverrides,
maybe_workspace_config: Option<WorkspaceConfig>,
pub disable_deprecated_api_warning: bool,
}

impl CliOptions {
Expand Down Expand Up @@ -728,6 +729,10 @@ impl CliOptions {
}
}

let disable_deprecated_api_warning = flags.log_level
== Some(log::Level::Error)
|| std::env::var("DENO_NO_DEPRECATION_WARNINGS").ok().is_some();
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC, this accepts DENO_NO_DEPRECATION_WARNINGS=0. Is that deliberate?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just noticed this comment - yeah this is deliberate. We don't want to announce and encourage use of this env var - let's consider it for internal testing purposes. We want to "force" users to move away from these APIs.


Ok(Self {
flags,
initial_cwd,
Expand All @@ -738,6 +743,7 @@ impl CliOptions {
maybe_vendor_folder,
overrides: Default::default(),
maybe_workspace_config,
disable_deprecated_api_warning,
})
}

Expand Down Expand Up @@ -1058,6 +1064,7 @@ impl CliOptions {
maybe_lockfile: self.maybe_lockfile.clone(),
maybe_workspace_config: self.maybe_workspace_config.clone(),
overrides: self.overrides.clone(),
disable_deprecated_api_warning: self.disable_deprecated_api_warning,
}
}

Expand Down
1 change: 1 addition & 0 deletions cli/factory.rs
Expand Up @@ -673,6 +673,7 @@ impl CliFactory {
self.feature_checker().clone(),
self.create_cli_main_worker_options()?,
self.options.node_ipc_fd(),
self.options.disable_deprecated_api_warning,
))
}

Expand Down
3 changes: 3 additions & 0 deletions cli/standalone/binary.rs
Expand Up @@ -150,6 +150,7 @@ pub struct Metadata {
pub maybe_import_map: Option<(Url, String)>,
pub entrypoint: ModuleSpecifier,
pub node_modules: Option<NodeModules>,
pub disable_deprecated_api_warning: bool,
}

pub fn load_npm_vfs(root_dir_path: PathBuf) -> Result<FileBackedVfs, AnyError> {
Expand Down Expand Up @@ -557,6 +558,8 @@ impl<'a> DenoCompileBinaryWriter<'a> {
entrypoint: entrypoint.clone(),
maybe_import_map,
node_modules,
disable_deprecated_api_warning: cli_options
.disable_deprecated_api_warning,
};

write_binary_bytes(
Expand Down
1 change: 1 addition & 0 deletions cli/standalone/mod.rs
Expand Up @@ -539,6 +539,7 @@ pub async fn run(
maybe_root_package_json_deps: package_json_deps_provider.deps().cloned(),
},
None,
metadata.disable_deprecated_api_warning,
);

v8_set_flags(construct_v8_flags(&[], &metadata.v8_flags, vec![]));
Expand Down
22 changes: 22 additions & 0 deletions cli/tests/integration/run_tests.rs
Expand Up @@ -4939,3 +4939,25 @@ itest!(unstable_temporal_api_missing_flag {
http_server: false,
exit_code: 1,
});

itest!(warn_on_deprecated_api {
args: "run -A run/warn_on_deprecated_api/main.js",
output: "run/warn_on_deprecated_api/main.out",
http_server: true,
exit_code: 0,
});

itest!(warn_on_deprecated_api_with_flag {
args: "run -A --quiet run/warn_on_deprecated_api/main.js",
output: "run/warn_on_deprecated_api/main_disabled_flag.out",
http_server: true,
exit_code: 0,
});

itest!(warn_on_deprecated_api_with_env_var {
args: "run -A run/warn_on_deprecated_api/main.js",
envs: vec![("DENO_NO_DEPRECATION_WARNINGS".to_string(), "1".to_string())],
output: "run/warn_on_deprecated_api/main_disabled_env.out",
http_server: true,
exit_code: 0,
});
32 changes: 32 additions & 0 deletions cli/tests/testdata/run/warn_on_deprecated_api/main.js
@@ -0,0 +1,32 @@
import { runEcho as runEcho2 } from "http://localhost:4545/run/warn_on_deprecated_api/mod.ts";

const p = Deno.run({
cmd: [
Deno.execPath(),
"eval",
"console.log('hello world')",
],
});
await p.status();
p.close();

async function runEcho() {
const p = Deno.run({
cmd: [
Deno.execPath(),
"eval",
"console.log('hello world')",
],
});
await p.status();
p.close();
}

await runEcho();
await runEcho();

for (let i = 0; i < 10; i++) {
await runEcho();
}

await runEcho2();
72 changes: 72 additions & 0 deletions cli/tests/testdata/run/warn_on_deprecated_api/main.out
@@ -0,0 +1,72 @@
Download http://localhost:4545/run/warn_on_deprecated_api/mod.ts
Warning
├ Use of deprecated "Deno.run()" API.
├ This API will be removed in Deno 2.0. Make sure to upgrade to a stable API before then.
├ Suggestion: Use "Deno.Command()" API instead.
└ Stack trace:
└─ at [WILDCARD]warn_on_deprecated_api/main.js:3:16

hello world
Warning
├ Use of deprecated "Deno.run()" API.
├ This API will be removed in Deno 2.0. Make sure to upgrade to a stable API before then.
├ Suggestion: Use "Deno.Command()" API instead.
└ Stack trace:
├─ at runEcho ([WILDCARD]warn_on_deprecated_api/main.js:14:18)
└─ at [WILDCARD]warn_on_deprecated_api/main.js:25:7

hello world
Warning
├ Use of deprecated "Deno.run()" API.
├ This API will be removed in Deno 2.0. Make sure to upgrade to a stable API before then.
├ Suggestion: Use "Deno.Command()" API instead.
└ Stack trace:
├─ at runEcho ([WILDCARD]warn_on_deprecated_api/main.js:14:18)
└─ at [WILDCARD]warn_on_deprecated_api/main.js:26:7

hello world
Warning
├ Use of deprecated "Deno.run()" API.
├ This API will be removed in Deno 2.0. Make sure to upgrade to a stable API before then.
├ Suggestion: Use "Deno.Command()" API instead.
└ Stack trace:
├─ at runEcho ([WILDCARD]warn_on_deprecated_api/main.js:14:18)
└─ at [WILDCARD]warn_on_deprecated_api/main.js:29:9

hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
Warning
├ Use of deprecated "Deno.run()" API.
├ This API will be removed in Deno 2.0. Make sure to upgrade to a stable API before then.
├ Suggestion: Use "Deno.Command()" API instead.
├ Suggestion: It appears this API is used by a remote dependency.
│ Try upgrading to the latest version of that dependency.
└ Stack trace:
├─ at runEcho (http://localhost:4545/run/warn_on_deprecated_api/mod.ts:2:18)
└─ at [WILDCARD]warn_on_deprecated_api/main.js:32:7

hello world
@@ -0,0 +1,15 @@
Download http://localhost:4545/run/warn_on_deprecated_api/mod.ts
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
@@ -0,0 +1,14 @@
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
hello world
11 changes: 11 additions & 0 deletions cli/tests/testdata/run/warn_on_deprecated_api/mod.ts
@@ -0,0 +1,11 @@
export async function runEcho() {
const p = Deno.run({
cmd: [
Deno.execPath(),
"eval",
"console.log('hello world')",
],
});
await p.status();
p.close();
}
5 changes: 5 additions & 0 deletions cli/worker.rs
Expand Up @@ -125,6 +125,7 @@ struct SharedWorkerState {
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
feature_checker: Arc<FeatureChecker>,
node_ipc: Option<i64>,
disable_deprecated_api_warning: bool,
}

impl SharedWorkerState {
Expand Down Expand Up @@ -405,6 +406,7 @@ impl CliMainWorkerFactory {
feature_checker: Arc<FeatureChecker>,
options: CliMainWorkerOptions,
node_ipc: Option<i64>,
disable_deprecated_api_warning: bool,
) -> Self {
Self {
shared: Arc::new(SharedWorkerState {
Expand All @@ -426,6 +428,7 @@ impl CliMainWorkerFactory {
maybe_lockfile,
feature_checker,
node_ipc,
disable_deprecated_api_warning,
}),
}
}
Expand Down Expand Up @@ -588,6 +591,7 @@ impl CliMainWorkerFactory {
.maybe_binary_npm_command_name
.clone(),
node_ipc_fd: shared.node_ipc,
disable_deprecated_api_warning: shared.disable_deprecated_api_warning,
},
extensions: custom_extensions,
startup_snapshot: crate::js::deno_isolate_init(),
Expand Down Expand Up @@ -786,6 +790,7 @@ fn create_web_worker_callback(
.maybe_binary_npm_command_name
.clone(),
node_ipc_fd: None,
disable_deprecated_api_warning: shared.disable_deprecated_api_warning,
},
extensions: vec![],
startup_snapshot: crate::js::deno_isolate_init(),
Expand Down
5 changes: 5 additions & 0 deletions runtime/js/40_process.js
Expand Up @@ -140,6 +140,11 @@ function run({
...new SafeArrayIterator(ArrayPrototypeSlice(cmd, 1)),
];
}
internals.warnOnDeprecatedApi(
"Deno.run()",
(new Error()).stack,
`Use "Deno.Command()" API instead.`,
);
const res = opRun({
cmd: ArrayPrototypeMap(cmd, String),
cwd,
Expand Down