feat: Start warning on each use of a deprecated API #21939

bartlomieju · 2024-01-14T22:49:23Z

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:

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

iuioiua · 2024-01-15T00:05:03Z

Closes #21839

bartlomieju · 2024-01-15T01:10:13Z

Note to self: add a test that calls deprecated API in a function and then call that function in a for-loop - verify that warning is only printed once.

kt3k · 2024-01-15T05:51:44Z

runtime/js/99_main.js

+
+function warnOnDeprecatedApi(apiName, stack) {
+  const stackLines = StringPrototypeSplit(stack, "\n");
+  ArrayPrototypeShift(stackLines);


Curious why we shift one item here

This is to remove the Error\n at the start of the output from new Error().stack.

I wonder if that's worth the cost of split, shift, and join

I suggest stack.slice(6) with a comment.

I also suggest we should do that slice at the line of "%cThis API was called from:\n" + stackString + "\n", to minimize the overhead of this util

This is also filtering irrelevant stack frames. Without it the stacks are very noisy and make it hard to figure out where the API is called. It's not like this API will be called in the hot path (and if it is it's yet another reason to fix user code quickly :))

~~I will change the logic to first check if such stack traces was produced before manipulating the stack.~~ Done.

iuioiua · 2024-01-15T21:48:22Z

runtime/js/99_main.js

+    "%cWarning",
+    "color: yellow; font-weight: bold;",
+  );
+  console.log(


When importing from deno_std without a pinned version (e.g. import * as http from "https://deno.land/std/http/server.ts), the warning printed has a yellow "Warning" prefix and unformatted rest-of-text. Should this warning be consistent with that?

No, I want it to stand out and be sore to the eyes :)

Good idea 😆

iuioiua · 2024-01-15T22:03:44Z

Should we also provide the option to mute these warnings by setting a NO_DEPRECATED_WARNINGS=1 in the current process?

bartlomieju · 2024-01-16T15:44:01Z

Use API from Deno.core that gets the last user stack frame.

bartlomieju · 2024-01-16T16:18:40Z

Warn on each rid getter in APIs as well.

bartlomieju · 2024-01-17T00:03:53Z

Waiting on #21961.

Might want to add an env var after all - it might be useful in tests at least for the time being we still have these deprecated APIs.

For #21939

kt3k

LGTM

dsherret

LGTM. Nice!

iuioiua · 2024-01-19T00:47:21Z

cli/args/mod.rs

@@ -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();


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

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.

iuioiua · 2024-01-22T03:23:22Z

runtime/js/99_main.js

+      "color: yellow;",
+    );
+  }
+


We should print the ability to disable these warnings with the DENO_NO_DEPRECATION_WARNINGS environment variable.

#21939 (comment)

Follow up to #21939 that adds deprecation warnings to more `Deno` APIs: - `Deno.copy()` - `Deno.iter()` - `Deno.iterSync()` - `new Deno.Buffer()` - `Deno.readAll()` - `Deno.readAllSync()` - `Deno.writeAll()` - `Deno.writeAllSync()` - `Deno.FsWatcher.return` - `Deno.serveHttp()` - `Deno.metrics()` --------- Signed-off-by: Asher Gomez <ashersaupingomez@gmail.com> Co-authored-by: Asher Gomez <ashersaupingomez@gmail.com>

feat: Start warning on each use of a deprecated API

21969fa

bartlomieju added this to the 1.40 milestone Jan 14, 2024

bartlomieju added 2 commits January 14, 2024 23:59

lint

4332d9f

change where deprecation happens

663da9d

iuioiua mentioned this pull request Jan 15, 2024

feat: print warning on use of deprecated API denoland/deno_std#4200

Merged

bartlomieju added 2 commits January 15, 2024 03:37

Merge branch 'main' into warn_on_deprecated_api

c6759ac

improve the warning

e4d9f12

kt3k reviewed Jan 15, 2024

View reviewed changes

bartlomieju added 3 commits January 15, 2024 14:25

improve message further

7a8e0c8

Merge branch 'main' into warn_on_deprecated_api

6dea3f8

make storage more efficient, eye-candy

f4d7b93

iuioiua reviewed Jan 15, 2024

View reviewed changes

bartlomieju added 4 commits January 16, 2024 22:51

Merge branch 'main' into warn_on_deprecated_api

a62e36b

revert some changes for now

4ba7120

add a test

42fb12a

primordials :)))

6732f0b

bartlomieju mentioned this pull request Jan 16, 2024

refactor: change tests to not rely on Deno.run() #21961

Merged

bartlomieju added a commit that referenced this pull request Jan 17, 2024

refactor: change tests to not rely on Deno.run() (#21961)

147bfc9

For #21939

Merge branch 'main' into warn_on_deprecated_api

6facb4f

kt3k approved these changes Jan 17, 2024

View reviewed changes

bartlomieju added 3 commits January 18, 2024 23:32

Merge branch 'main' into warn_on_deprecated_api

92ea4ba

allow to ignore warnings

7ceda6d

add more tests

226a212

dsherret approved these changes Jan 18, 2024

View reviewed changes

Merge branch 'main' into warn_on_deprecated_api

a717727

bartlomieju mentioned this pull request Jan 18, 2024

fix: remove conditional unstable type-checking from other commands #21991

Merged

bartlomieju enabled auto-merge (squash) January 18, 2024 23:11

bartlomieju merged commit c62615b into denoland:main Jan 18, 2024
14 checks passed

bartlomieju deleted the warn_on_deprecated_api branch January 18, 2024 23:30

bartlomieju mentioned this pull request Jan 18, 2024

feat: Add warnings for more deprecated APIs #21992

Merged

iuioiua reviewed Jan 22, 2024

View reviewed changes

iuioiua mentioned this pull request Jan 23, 2024

refactor: remove environment variable functionality notice in warnOnDeprecatedApi() denoland/deno_std#4227

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Start warning on each use of a deprecated API #21939

feat: Start warning on each use of a deprecated API #21939

bartlomieju commented Jan 14, 2024 •

edited

iuioiua commented Jan 15, 2024

bartlomieju commented Jan 15, 2024

kt3k Jan 15, 2024

iuioiua Jan 15, 2024

kt3k Jan 15, 2024

iuioiua Jan 15, 2024

kt3k Jan 15, 2024

bartlomieju Jan 15, 2024

bartlomieju Jan 15, 2024 •

edited

iuioiua Jan 15, 2024

bartlomieju Jan 15, 2024

iuioiua Jan 15, 2024

iuioiua commented Jan 15, 2024

bartlomieju commented Jan 16, 2024

bartlomieju commented Jan 16, 2024

bartlomieju commented Jan 17, 2024

kt3k left a comment

dsherret left a comment

iuioiua Jan 19, 2024

bartlomieju Jan 23, 2024

iuioiua Jan 22, 2024

bartlomieju Jan 23, 2024

feat: Start warning on each use of a deprecated API #21939

feat: Start warning on each use of a deprecated API #21939

Conversation

bartlomieju commented Jan 14, 2024 • edited

iuioiua commented Jan 15, 2024

bartlomieju commented Jan 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartlomieju Jan 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iuioiua commented Jan 15, 2024

bartlomieju commented Jan 16, 2024

bartlomieju commented Jan 16, 2024

bartlomieju commented Jan 17, 2024

kt3k left a comment

Choose a reason for hiding this comment

dsherret left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartlomieju commented Jan 14, 2024 •

edited

bartlomieju Jan 15, 2024 •

edited