Expand TypeScript brownfield workspace validation#17510
Conversation
Preserve existing Aspire package scripts during TypeScript brownfield init and add coverage for workspace subdirectory and package-manager boundary scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17510Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17510" |
There was a problem hiding this comment.
Pull request overview
This PR expands TypeScript “brownfield” workspace validation for Aspire CLI initialization and toolchain resolution. It aims to avoid overwriting user-owned package.json scripts during TypeScript brownfield init, while still adding safe delegate scripts and tightening package-manager/toolchain selection behavior in workspace layouts.
Changes:
- Preserve existing
aspire:*scripts (and only add missing delegates) when augmenting a TypeScript brownfield package.json. - Update package.json merge behavior so existing
aspire:-prefixed scripts are preserved rather than overwritten. - Add/adjust unit and E2E coverage for workspace-subdirectory brownfield init and AppHost-vs-parent toolchain precedence.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Scaffolding/ScaffoldingService.cs | Adds logic to only add missing root delegate scripts and warn when existing ones are preserved. |
| src/Aspire.Cli/Scaffolding/PackageJsonMerger.cs | Changes merge semantics to preserve existing aspire:-prefixed scripts; updates docs accordingly. |
| tests/Aspire.Cli.Tests/Scaffolding/ScaffoldingServiceTests.cs | Adds tests for delegate-script addition and preservation behavior. |
| tests/Aspire.Cli.Tests/Scaffolding/PackageJsonMergerTests.cs | Updates expectations to validate preservation of existing aspire: scripts. |
| tests/Aspire.Cli.Tests/Projects/TypeScriptAppHostToolchainResolverTests.cs | Adds coverage for AppHost vs parent toolchain marker/lockfile precedence. |
| tests/Aspire.Cli.EndToEnd.Tests/TypeScriptPolyglotTests.cs | Adds E2E coverage for mixed AppHost/guest toolchains and workspace-subdirectory brownfield init behavior. |
Resolve brownfield root delegate scripts from the nested AppHost package boundary so generated delegates match the AppHost package manager. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
❓ CLI E2E Tests unknown — 107 passed, 0 failed, 3 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26470329909 |
…script preservation Documents the behavior introduced in microsoft/aspire#17510: - Workspace subdirectory layout when running aspire init from a subdirectory - Root package.json delegate scripts (aspire:start, aspire:build, aspire:dev) - Script preservation: existing aspire:-prefixed scripts are not overwritten - Package-manager isolation between the AppHost and guest packages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #1080
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1080 targeting Updated Note This draft PR needs human review before merging. |
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: Version verificationObjective: Verify dogfood CLI matches the PR head commit. Steps:
Evidence:
Observations:
Scenario 2: Brownfield workspace script preservationObjective: Verify TypeScript brownfield init preserves custom aspire:* scripts and adds missing delegates. Steps:
Evidence:
Observations:
Expected Unhappy-Path Outcome: Existing custom aspire:start must not be overwritten; init should warn and continue safely. Scenario 3: AppHost toolchain delegate selectionObjective: Verify brownfield root delegate scripts use the selected pnpm AppHost toolchain. Steps:
Evidence:
Observations:
Scenario 4: Mixed package-manager guest appObjective: Verify a pnpm TypeScript AppHost can restore/type-check with an npm Vite guest app. Steps:
Evidence:
Observations:
Summary
Overall ResultPR VERIFIED Recommendations
|
Description
TypeScript brownfield AppHosts need to work in realistic workspace layouts without taking over scripts that users already own. This change preserves existing root
aspire:*scripts during TypeScript brownfield init, adds missing AppHost delegate scripts only when safe, and warns when a delegate script was intentionally left untouched.It also locks down the package-manager boundary for TypeScript AppHosts: AppHost package-manager markers take precedence inside the AppHost package, while guest apps can keep their own package manager and lockfile. The new coverage exercises workspace-subdirectory brownfield init, script preservation, and mixed AppHost/guest package-manager scenarios.
User-facing usage
In a brownfield TypeScript app under a workspace subdirectory, users can run:
The generated AppHost stays under
aspire-apphost, existing app scripts such asdev,build,preview, and existingaspire:startvalues are preserved, and missing root delegates such asaspire:buildandaspire:devare added to call the nested AppHost package.Fixes: #17036
Validation:
./restore.shgit diff --checkdotnet test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.ScaffoldingServiceTests" --filter-class "*.PackageJsonMergerTests" --filter-class "*.TypeScriptAppHostToolchainResolverTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"13.3.5instead of the workspace build and failed on olderapphost.tsoutput before exercising this branch.Checklist
<remarks />and<code />elements on your triple slash comments?