fix(PackageManagerTabs): correctly handle deno run args and add -A flag#3188
fix(PackageManagerTabs): correctly handle deno run args and add -A flag#3188
Conversation
Deploying rspress-v2 with
|
| Latest commit: |
a4e5565
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ed356772.rspress-v2.pages.dev |
| Branch Preview URL: | https://fix-deno-run-args.rspress-v2.pages.dev |
Rsdoctor Bundle Diff AnalysisFound 3 projects in monorepo, 3 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 nodePath:
📦 Download Diff Report: node Bundle Diff 📁 webPath:
📦 Download Diff Report: web Bundle Diff 📁 node_mdPath:
📦 Download Diff Report: node_md Bundle Diff Generated by Rsdoctor GitHub Action |
There was a problem hiding this comment.
Pull request overview
This PR fixes how Deno run commands are generated in PackageManagerTabs, ensuring only the first positional argument is prefixed with npm: when using deno run, and adds default -A permissions. It also extends E2E coverage for dlx commands that include multiple arguments.
Changes:
- Update Deno positional-arg transformation to only prefix the first positional argument for
deno run. - Change Deno execution commands to use
deno run -Aby default. - Add E2E fixture + assertions for
dlxcommands with multiple arguments.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/theme/components/PackageManagerTabs/index.tsx | Adjusts Deno command rendering (-A) and positional arg prefixing logic. |
| e2e/fixtures/package-manager-tabs/index.test.ts | Adds assertions for multi-arg dlx commands and updates expected Deno output. |
| e2e/fixtures/package-manager-tabs/doc/index.mdx | Adds fixture content for a dlx command with multiple args. |
Comments suppressed due to low confidence (2)
packages/core/src/theme/components/PackageManagerTabs/index.tsx:66
transformDenoPositionalonly treats--...tokens as flags. With the new defaultdeno run -A,-Ais a short flag that does not start with--and will be treated as a positional argument and incorrectly rewritten tonpm:-A(and then the actual script/tool may stop being transformed). Update the positional detection to also exclude short flags (tokens starting with-), and (if applicable) exclude their flag-values similar to the existing--...logic.
const subcommand = parts[1];
const isRun = subcommand === 'run';
let transformedFirst = false;
const transformed = parts.map((tok, idx) => {
// only transform positional args (after "deno" and the subcommand)
if (
idx >= 2 &&
!tok.startsWith('-') &&
!tok.includes(':') &&
!/^https?:/.test(tok)
) {
if (isRun && transformedFirst) {
return tok;
}
transformedFirst = true;
return `npm:${tok}`;
}
return tok;
});
e2e/fixtures/package-manager-tabs/index.test.ts:1
- The fixture doc now renders an additional
<PackageManagerTabs ... />(multi-argdlx) while still keeping the originaldlxandexecexamples, so the page likely has 3 tab groups (15 tabs), not 2 (10 tabs). Either update this expectation to match the new number of rendered tab groups, or change the test query to scope to the specificPackageManagerTabsinstance(s) it intends to assert.
import { expect, test } from '@playwright/test';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Correctly handle positional arguments for
deno runinPackageManagerTabscomponent. Previously, all positional arguments were prefixed withnpm:, which caused issues when running tools that take subcommands (e.g.,skills add ...).This PR:
transformDenoPositionalto only prefix the first positional argument withnpm:when the subcommand isrun.-A(allow-all) flag todeno runby default to match the behavior of other package managers' execution commands (likenpx,bunx).dlxwith multiple arguments.Related Issue
None.
Checklist