Skip to content

Conversation

@iuioiua
Copy link
Contributor

@iuioiua iuioiua commented Nov 21, 2025

Was supposed to be removed in Deno 2.5. Proceeds #25522.

@coderabbitai
Copy link

coderabbitai bot commented Nov 21, 2025

Walkthrough

The PR removes global legacy --unstable flag handling and the legacy_flag_enabled state. Commands are no longer injected with a global --unstable flag; parsing no longer follows a legacy pathway. Per-feature unstable flags remain and are still added. References to the global --unstable flag were removed from documentation, tests, benchmarks, scripts, and CLI code paths; a runtime warning emitted when the legacy flag was present was also removed.

Estimated code review effort

Medium | ~30–60 minutes

  • Confirm legacy_flag_enabled removal has no remaining references.
  • Verify commands no longer add a global --unstable flag and per-feature flags still appear and parse correctly.
  • Ensure legacy parsing branch removal left no orphaned logic and help text/runtime behavior remain correct.
  • Check documentation, tests, benchmarks, and scripts for remaining --unstable occurrences.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately describes the main change: removing the --unstable flag as a breaking change.
Description check ✅ Passed The description relates to the changeset by referencing the removal of --unstable and linking to a related issue.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 718abaf and 36a65fb.

📒 Files selected for processing (3)
  • cli/args/flags.rs (1 hunks)
  • cli/main.rs (0 hunks)
  • cli/standalone/binary.rs (0 hunks)
💤 Files with no reviewable changes (2)
  • cli/main.rs
  • cli/standalone/binary.rs
🧰 Additional context used
📓 Path-based instructions (2)
cli/args/flags.rs

📄 CodeRabbit inference engine (CLAUDE.md)

CLI flag parsing should be defined in cli/args/flags.rs

Files:

  • cli/args/flags.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or use lldb directly
Use eprintln!() or dbg!() macros for debug prints in Rust code

Files:

  • cli/args/flags.rs

⚙️ CodeRabbit configuration file

Don't worry about coverage of Rust docstrings. Don't be nitpicky about it. Leave it to the author's judgement if such a documentation is necessary.

Files:

  • cli/args/flags.rs
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to cli/args/flags.rs : CLI flag parsing should be defined in `cli/args/flags.rs`

Applied to files:

  • cli/args/flags.rs
🔇 Additional comments (1)
cli/args/flags.rs (1)

5274-5321: with_unstable_args no longer creates an "unstable" arg, potentially breaking --help=unstable semantics

The refactored with_unstable_args now only adds per-feature flags from deno_runtime::UNSTABLE_FEATURES but no longer defines an arg with id "unstable". This creates a mismatch:

  • flags_from_vec_with_initial_cwd gates enable_unstable on subcommand.get_arguments().any(|arg| arg.get_id().as_str() == "unstable")
  • enable_unstable unhides args with help_heading == UNSTABLE_HEADING && long_help.is_some()

Without the "unstable" arg, that condition is now always false, so deno <cmd> --help=unstable and deno help <cmd> unstable will no longer expand and show unstable options—they behave like normal help while still accepting the "unstable" context value.

To preserve documented --help=unstable behavior while keeping --unstable hard-removed:

  • Reintroduce a hidden metadata arg with id "unstable" inside with_unstable_args (without .long("unstable")), solely to drive enable_unstable, or
  • Refactor the guard and enable_unstable to detect any arg with help_heading == UNSTABLE_HEADING (and long_help metadata) instead of checking for an "unstable" arg, and drop the .mut_arg("unstable", ..) call entirely.

Verify the actual --help=unstable behavior (e.g., deno run --help=unstable) before and after this change to confirm whether the UX change is intentional.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tools/wgpu_sync.js (1)

1-22: Fix typo in Deno.Command options: sdterr should be stderr

The correct option name is stderr (how stderr of the spawned process should be handled). The code uses sdterr, which is a typo and will be silently ignored, causing stderr to use its default value instead of inheriting from the parent process as intended.

Apply the fix:

-    sdterr: "inherit",
+    stderr: "inherit",
🧹 Nitpick comments (1)
cli/args/flags.rs (1)

5186-5194: --unstable help text says “removed”, but flag is still accepted as a silent no-op

Right now Arg::new("unstable") is still defined and parsed, but unstable_args_parse never looks at it, so deno … --unstable is accepted and then ignored. The new help string says “The --unstable flag has been removed…”, which reads as “this flag is no longer valid”, but the CLI doesn’t error or warn when it’s used.

To better match user expectations and the “hard remove” intent, consider one of:

  • Drop the unstable arg entirely for runtime parsing and make --unstable a genuine unknown argument, or
  • Keep the arg but fail fast with a clear error (or at least print a runtime warning) when --unstable is present, instead of silently ignoring it.

That way users who still rely on --unstable discover the change immediately, rather than having behavior subtly change with no feedback.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e59d37f and b23be3d.

⛔ Files ignored due to path filters (1)
  • tests/specs/vendor/removed/vendor_help.out is excluded by !**/*.out
📒 Files selected for processing (9)
  • .github/CONTRIBUTING.md (1 hunks)
  • cli/args/flags.rs (1 hunks)
  • cli/bench/fs/README.md (1 hunks)
  • cli/bench/main.rs (0 hunks)
  • cli/lib/args.rs (0 hunks)
  • cli/main.rs (0 hunks)
  • cli/standalone/binary.rs (0 hunks)
  • tests/wpt/README.md (1 hunks)
  • tools/wgpu_sync.js (1 hunks)
💤 Files with no reviewable changes (4)
  • cli/bench/main.rs
  • cli/lib/args.rs
  • cli/main.rs
  • cli/standalone/binary.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: test debug linux-x86_64
  • GitHub Check: test debug linux-aarch64
  • GitHub Check: test debug macos-x86_64
  • GitHub Check: test debug windows-x86_64
  • GitHub Check: build libs
  • GitHub Check: test debug macos-aarch64
  • GitHub Check: test release linux-x86_64
  • GitHub Check: lint debug windows-x86_64
  • GitHub Check: lint debug linux-x86_64
  • GitHub Check: lint debug macos-x86_64
🔇 Additional comments (3)
cli/bench/fs/README.md (1)

17-19: Bench command correctly drops removed --unstable flag

The updated deno run -A run.mjs line matches the removal of the global --unstable flag and keeps the example valid with current CLI behavior.

.github/CONTRIBUTING.md (1)

264-269: TS test invocations updated for hard removal of --unstable

Dropping --unstable from the TypeScript test commands is consistent with the flag removal and keeps the contributor docs from recommending an invalid option.

tests/wpt/README.md (1)

8-11: WPT runner prefix updated to avoid removed --unstable

Removing --unstable from the suggested deno run prefix keeps the WPT instructions compatible with the hard removal of the flag.

@dsherret dsherret changed the title chore: hard remove --unstable flag feat(BREAKING): hard remove --unstable flag Nov 21, 2025
@iuioiua
Copy link
Contributor Author

iuioiua commented Dec 4, 2025

Is it worth removing UnstableArgsConfig::None and instead using Option<UnstableArgsConfig> in .with_unstable_args()?

Actually, not worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant