From c62615bfe5a070c2517f3af3208d4308c72eb054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 19 Jan 2024 00:30:49 +0100 Subject: [PATCH] feat: Start warning on each use of a deprecated API (#21939) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit introduces deprecation warnings for "Deno.*" APIs. This is gonna be quite noisy, but should tremendously help with user code updates to ensure smooth migration to Deno 2.0. The warning is printed at each unique call site to help quickly identify where code needs to be adjusted. There's some stack frame filtering going on to remove frames that are not useful to the user and would only cause confusion. The warning can be silenced using "--quiet" flag or "DENO_NO_DEPRECATION_WARNINGS" env var. "Deno.run()" API is now using this warning. Other deprecated APIs will start warning in follow up PRs. Example: ```js 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(); ``` ``` $ deno run --allow-read foo.js 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 file:///Users/ib/dev/deno/foo.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 (file:///Users/ib/dev/deno/foo.js:8:18) └─ at file:///Users/ib/dev/deno/foo.js:13: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 (file:///Users/ib/dev/deno/foo.js:8:18) └─ at file:///Users/ib/dev/deno/foo.js:14: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 (file:///Users/ib/dev/deno/foo.js:8:18) └─ at file:///Users/ib/dev/deno/foo.js:17: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 file:///Users/ib/dev/deno/foo.js:20:7 hello world ``` Closes #21839 --- cli/args/mod.rs | 7 ++ cli/factory.rs | 1 + cli/standalone/binary.rs | 3 + cli/standalone/mod.rs | 1 + cli/tests/integration/run_tests.rs | 22 +++++ .../run/warn_on_deprecated_api/main.js | 32 ++++++ .../run/warn_on_deprecated_api/main.out | 72 ++++++++++++++ .../main_disabled_env.out | 15 +++ .../main_disabled_flag.out | 14 +++ .../run/warn_on_deprecated_api/mod.ts | 11 +++ cli/worker.rs | 5 + runtime/js/40_process.js | 5 + runtime/js/99_main.js | 99 ++++++++++++++++++- runtime/worker_bootstrap.rs | 5 + 14 files changed, 291 insertions(+), 1 deletion(-) create mode 100644 cli/tests/testdata/run/warn_on_deprecated_api/main.js create mode 100644 cli/tests/testdata/run/warn_on_deprecated_api/main.out create mode 100644 cli/tests/testdata/run/warn_on_deprecated_api/main_disabled_env.out create mode 100644 cli/tests/testdata/run/warn_on_deprecated_api/main_disabled_flag.out create mode 100644 cli/tests/testdata/run/warn_on_deprecated_api/mod.ts diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 3ca7b5e321dd87..c07df3a8081628 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -682,6 +682,7 @@ pub struct CliOptions { maybe_lockfile: Option>>, overrides: CliOptionOverrides, maybe_workspace_config: Option, + pub disable_deprecated_api_warning: bool, } impl CliOptions { @@ -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(); + Ok(Self { flags, initial_cwd, @@ -738,6 +743,7 @@ impl CliOptions { maybe_vendor_folder, overrides: Default::default(), maybe_workspace_config, + disable_deprecated_api_warning, }) } @@ -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, } } diff --git a/cli/factory.rs b/cli/factory.rs index 204ea7e87eb349..ed5232470abd19 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -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, )) } diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 18880b7a16a7e8..cdf86fffafc60e 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -150,6 +150,7 @@ pub struct Metadata { pub maybe_import_map: Option<(Url, String)>, pub entrypoint: ModuleSpecifier, pub node_modules: Option, + pub disable_deprecated_api_warning: bool, } pub fn load_npm_vfs(root_dir_path: PathBuf) -> Result { @@ -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( diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 55c3db48ce9719..8a98636a48708a 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -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![])); diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 8db06470d19371..3fd0133764a6d9 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -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, +}); diff --git a/cli/tests/testdata/run/warn_on_deprecated_api/main.js b/cli/tests/testdata/run/warn_on_deprecated_api/main.js new file mode 100644 index 00000000000000..a464be60a325fb --- /dev/null +++ b/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(); diff --git a/cli/tests/testdata/run/warn_on_deprecated_api/main.out b/cli/tests/testdata/run/warn_on_deprecated_api/main.out new file mode 100644 index 00000000000000..984dbc291c6949 --- /dev/null +++ b/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 diff --git a/cli/tests/testdata/run/warn_on_deprecated_api/main_disabled_env.out b/cli/tests/testdata/run/warn_on_deprecated_api/main_disabled_env.out new file mode 100644 index 00000000000000..ef85a6f99bac3f --- /dev/null +++ b/cli/tests/testdata/run/warn_on_deprecated_api/main_disabled_env.out @@ -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 diff --git a/cli/tests/testdata/run/warn_on_deprecated_api/main_disabled_flag.out b/cli/tests/testdata/run/warn_on_deprecated_api/main_disabled_flag.out new file mode 100644 index 00000000000000..ce3755d16683cc --- /dev/null +++ b/cli/tests/testdata/run/warn_on_deprecated_api/main_disabled_flag.out @@ -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 diff --git a/cli/tests/testdata/run/warn_on_deprecated_api/mod.ts b/cli/tests/testdata/run/warn_on_deprecated_api/mod.ts new file mode 100644 index 00000000000000..f74632b2cdfed0 --- /dev/null +++ b/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(); +} diff --git a/cli/worker.rs b/cli/worker.rs index 8c2eed0c6e746b..4807e2699c11a9 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -125,6 +125,7 @@ struct SharedWorkerState { maybe_lockfile: Option>>, feature_checker: Arc, node_ipc: Option, + disable_deprecated_api_warning: bool, } impl SharedWorkerState { @@ -405,6 +406,7 @@ impl CliMainWorkerFactory { feature_checker: Arc, options: CliMainWorkerOptions, node_ipc: Option, + disable_deprecated_api_warning: bool, ) -> Self { Self { shared: Arc::new(SharedWorkerState { @@ -426,6 +428,7 @@ impl CliMainWorkerFactory { maybe_lockfile, feature_checker, node_ipc, + disable_deprecated_api_warning, }), } } @@ -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(), @@ -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(), diff --git a/runtime/js/40_process.js b/runtime/js/40_process.js index c90f38664d68cb..e6a62dcf7b330f 100644 --- a/runtime/js/40_process.js +++ b/runtime/js/40_process.js @@ -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, diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index 6c5ca3b5904902..54a4406b2e2e03 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -9,6 +9,8 @@ const { ArrayPrototypeFilter, ArrayPrototypeIncludes, ArrayPrototypeMap, + ArrayPrototypePop, + ArrayPrototypeShift, DateNow, Error, ErrorPrototype, @@ -23,6 +25,10 @@ const { ObjectValues, PromisePrototypeThen, PromiseResolve, + SafeSet, + StringPrototypeIncludes, + StringPrototypeSplit, + StringPrototypeTrim, Symbol, SymbolIterator, TypeError, @@ -89,6 +95,93 @@ ObjectDefineProperties(Symbol, { let windowIsClosing = false; let globalThis_; +let deprecatedApiWarningDisabled = false; +const ALREADY_WARNED_DEPRECATED = new SafeSet(); + +function warnOnDeprecatedApi(apiName, stack, suggestion) { + if (deprecatedApiWarningDisabled) { + return; + } + + if (ALREADY_WARNED_DEPRECATED.has(apiName + stack)) { + return; + } + + // If we haven't warned yet, let's do some processing of the stack trace + // to make it more useful. + const stackLines = StringPrototypeSplit(stack, "\n"); + ArrayPrototypeShift(stackLines); + while (true) { + // Filter out internal frames at the top of the stack - they are not useful + // to the user. + if ( + StringPrototypeIncludes(stackLines[0], "(ext:") || + StringPrototypeIncludes(stackLines[0], "(node:") + ) { + ArrayPrototypeShift(stackLines); + } else { + break; + } + } + // Now remove the last frame if it's coming from "ext:core" - this is most likely + // event loop tick or promise handler calling a user function - again not + // useful to the user. + if ( + StringPrototypeIncludes(stackLines[stackLines.length - 1], "(ext:core/") + ) { + ArrayPrototypePop(stackLines); + } + + let isFromRemoteDependency = false; + const firstStackLine = stackLines[0]; + if (firstStackLine && !StringPrototypeIncludes(firstStackLine, "file:")) { + isFromRemoteDependency = true; + } + + ALREADY_WARNED_DEPRECATED.add(apiName + stack); + console.error( + "%cWarning", + "color: yellow; font-weight: bold;", + ); + console.error( + `%c\u251c Use of deprecated "${apiName}" API.`, + "color: yellow;", + ); + console.error("%c\u2502", "color: yellow;"); + console.error( + "%c\u251c This API will be removed in Deno 2.0. Make sure to upgrade to a stable API before then.", + "color: yellow;", + ); + console.error("%c\u2502", "color: yellow;"); + console.error( + `%c\u251c Suggestion: ${suggestion}`, + "color: yellow;", + ); + if (isFromRemoteDependency) { + console.error("%c\u2502", "color: yellow;"); + console.error( + `%c\u251c Suggestion: It appears this API is used by a remote dependency.`, + "color: yellow;", + ); + console.error( + "%c\u2502 Try upgrading to the latest version of that dependency.", + "color: yellow;", + ); + } + + console.error("%c\u2502", "color: yellow;"); + console.error("%c\u2514 Stack trace:", "color: yellow;"); + for (let i = 0; i < stackLines.length; i++) { + console.error( + `%c ${i == stackLines.length - 1 ? "\u2514" : "\u251c"}\u2500 ${ + StringPrototypeTrim(stackLines[i]) + }`, + "color: yellow;", + ); + } + console.error(); +} + function windowClose() { if (!windowIsClosing) { windowIsClosing = true; @@ -432,7 +525,7 @@ function exposeUnstableFeaturesForWindowOrWorkerGlobalScope(options) { // FIXME(bartlomieju): temporarily add whole `Deno.core` to // `Deno[Deno.internal]` namespace. It should be removed and only necessary // methods should be left there. -ObjectAssign(internals, { core }); +ObjectAssign(internals, { core, warnOnDeprecatedApi }); const internalSymbol = Symbol("Deno.internal"); const finalDenoNs = { internal: internalSymbol, @@ -467,8 +560,10 @@ function bootstrapMainRuntime(runtimeOptions) { 3: inspectFlag, 5: hasNodeModulesDir, 6: maybeBinaryNpmCommandName, + 7: shouldDisableDeprecatedApiWarning, } = runtimeOptions; + deprecatedApiWarningDisabled = shouldDisableDeprecatedApiWarning; performance.setTimeOrigin(DateNow()); globalThis_ = globalThis; @@ -593,8 +688,10 @@ function bootstrapWorkerRuntime( 4: enableTestingFeaturesFlag, 5: hasNodeModulesDir, 6: maybeBinaryNpmCommandName, + 7: shouldDisableDeprecatedApiWarning, } = runtimeOptions; + deprecatedApiWarningDisabled = shouldDisableDeprecatedApiWarning; performance.setTimeOrigin(DateNow()); globalThis_ = globalThis; diff --git a/runtime/worker_bootstrap.rs b/runtime/worker_bootstrap.rs index aac665b0554122..e3bfc13b911c08 100644 --- a/runtime/worker_bootstrap.rs +++ b/runtime/worker_bootstrap.rs @@ -60,6 +60,7 @@ pub struct BootstrapOptions { pub has_node_modules_dir: bool, pub maybe_binary_npm_command_name: Option, pub node_ipc_fd: Option, + pub disable_deprecated_api_warning: bool, } impl Default for BootstrapOptions { @@ -88,6 +89,7 @@ impl Default for BootstrapOptions { has_node_modules_dir: Default::default(), maybe_binary_npm_command_name: None, node_ipc_fd: None, + disable_deprecated_api_warning: false, } } } @@ -117,6 +119,8 @@ struct BootstrapV8<'a>( bool, // maybe_binary_npm_command_name Option<&'a str>, + // disable_deprecated_api_warning, + bool, ); impl BootstrapOptions { @@ -136,6 +140,7 @@ impl BootstrapOptions { self.enable_testing_features, self.has_node_modules_dir, self.maybe_binary_npm_command_name.as_deref(), + self.disable_deprecated_api_warning, ); bootstrap.serialize(ser).unwrap()