Reject Yarn Classic for TypeScript AppHosts - #16620
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16620Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16620" |
There was a problem hiding this comment.
Pull request overview
Updates the TypeScript AppHost runtime command specs used by the Aspire CLI during publish/deploy so that operation arguments are reliably forwarded through JavaScript package managers (notably Yarn Classic, which otherwise drops trailing args without a -- separator). This prevents TypeScript AppHosts from accidentally running in normal “run” mode (starting DCP) when invoked via aspire publish / aspire deploy.
Changes:
- Adds a publish-specific runtime command (
PublishExecute) for the default TypeScript runtime spec that ends with--to preserve forwarded publish/deploy args. - Extends the TypeScript toolchain resolver to generate publish-specific command specs for Bun/Yarn/pnpm that include the separator.
- Adds/updates unit tests validating
PublishExecuteshapes and argument appending behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/TypeScriptLanguageSupportTests.cs | Verifies the TypeScript runtime spec now provides a publish command spec with a trailing --. |
| tests/Aspire.Cli.Tests/Projects/TypeScriptAppHostToolchainResolverTests.cs | Adds coverage for Bun/Yarn/pnpm publish command shapes including the separator. |
| tests/Aspire.Cli.Tests/Projects/GuestRuntimeTests.cs | Adds coverage ensuring additional publish args are appended after an existing -- token in the spec. |
| src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptLanguageSupport.cs | Introduces PublishExecute for the default TypeScript runtime spec using npx ... {appHostFile} --. |
| src/Aspire.Cli/Projects/TypeScriptAppHostToolchainResolver.cs | Generates toolchain-specific PublishExecute commands for Bun/Yarn/pnpm that include --. |
Jose Perez Rodriguez (joperezr)
left a comment
There was a problem hiding this comment.
Looks good overall. Left a few non-blocking thoughts inline - mostly around verifying the -- actually gets consumed for non-Yarn toolchains, and a couple of small test/maintainability nits. Approving.
|
Re-running the failed jobs in the CI workflow for this pull request because 3 jobs were identified as retry-safe transient failures in the CI run attempt.
Matched test failure patterns (3 tests)
|
|
I'm updating this - the original proposal was bad. Instead, the decision is that we aren't going to support Yarn Classic v1 until we receive user signal that we need to. |
Yarn Classic is not supported for TypeScript AppHosts because its command behavior differs from supported package managers and can break Aspire argument forwarding. Detect Yarn Classic through packageManager declarations and Yarn v1 lockfiles, then fail early with guidance to use Yarn 4 or later, npm, pnpm, or Bun. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
53eb41f to
29105c8
Compare
David Pine (IEvangelist)
left a comment
There was a problem hiding this comment.
Nice, focused change — early rejection with clear upgrade guidance is a much better failure mode than the silent arg-passing breakage Yarn Classic was causing, and the version detection (yarn@1.x only, lockfile-based fallback bounded to 5 lines, exception-tolerant) looks right. The unit tests cover the new branches well.
One CI breakage worth addressing before merge — left an inline note. Otherwise LGTM, approving.
|
🎬 CLI E2E Test Recordings — 57 recordings uploaded (commit View all recordings
📹 Recordings uploaded automatically from CI run #25383652633 |
…Hosts Documents the change introduced in microsoft/aspire#16620, which rejects Yarn Classic (v1) for TypeScript AppHosts. Adds a supported package managers table and an Aside warning in the TypeScript AppHost project structure page. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #830
|
|
📝 A draft documentation PR has been opened on Target branch: What was changed: Updated the [TypeScript AppHost project structure]((aspire.dev/redacted) page to:
The draft PR needs human review before merging.
|
…Hosts Documents the change introduced in microsoft/aspire#16620, which rejects Yarn Classic (v1) for TypeScript AppHosts. Adds a supported package managers table and an Aside warning in the TypeScript AppHost project structure page. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/backport to release/13.3 |
|
Started backporting to |
* Reject Yarn Classic for TypeScript AppHosts Yarn Classic is not supported for TypeScript AppHosts because its command behavior differs from supported package managers and can break Aspire argument forwarding. Detect Yarn Classic through packageManager declarations and Yarn v1 lockfiles, then fail early with guidance to use Yarn 4 or later, npm, pnpm, or Bun. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update yarn tests to v4 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Yarn Classic is not supported for TypeScript AppHosts. Its command and argument forwarding behavior differs from supported package managers and can break Aspire publish/deploy argument handling.
This keeps the existing TypeScript AppHost command shapes for supported JavaScript package managers. Instead, the resolver now detects Yarn Classic from explicit
packageManager: "yarn@1.x"declarations and Yarn v1 lockfiles (# yarn lockfile v1) in the AppHost directory or eligible parent workspace, then fails early with guidance to use Yarn 4 or later, npm, pnpm, or Bun.Validation:
dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.TypeScriptAppHostToolchainResolverTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"aspire publish --list-stepsandaspire run --detach --isolatedagainst a TypeScript empty AppHostaspire publish --list-stepsandaspire run --detach --isolatedagainst a TypeScript empty AppHostFixes #16616
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: