Skip to content

Adjust TypeScript AppHost package scaffolding#16895

Closed
maddymontaquila wants to merge 8 commits into
mainfrom
maddymontaquila/aspireify-feedback
Closed

Adjust TypeScript AppHost package scaffolding#16895
maddymontaquila wants to merge 8 commits into
mainfrom
maddymontaquila/aspireify-feedback

Conversation

@maddymontaquila
Copy link
Copy Markdown
Contributor

@maddymontaquila maddymontaquila commented May 8, 2026

Summary

This updates TypeScript AppHost scaffolding so brownfield aspire init --language typescript no longer turns the existing app package into the AppHost package.

  • Existing JS/TS apps now get an isolated aspire-apphost/ package that owns apphost.ts, .modules/, tsconfig.apphost.json, AppHost dependencies, ESM metadata, and ESLint configuration.
  • The root package.json keeps the user's app semantics and only gets aspire:* delegate scripts, e.g. npm --prefix aspire-apphost run aspire:start.
  • pnpm AppHost installs run with --ignore-workspace so a nested aspire-apphost/ package can install its own dependencies even when the user's pnpm-workspace.yaml does not include it.
  • The nested brownfield AppHost package uses the distinct private package name aspire-apphost to avoid duplicate package names if it is later included in a workspace.
  • aspire init now reports the nested AppHost path in success output, e.g. Created aspire-apphost/apphost.ts.
  • Greenfield TypeScript AppHost scaffolding remains root-level and still includes the complete AppHost package metadata/tooling.
  • The earlier scaffold-options/RPC plumbing for toggling TypeScript linting was removed because the isolated package boundary solves the brownfield leakage directly.
  • Package merging still avoids duplicating dependencies across dependencies and devDependencies.

Validation

  • ./restore.sh

  • ./dotnet.sh test --project tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"

  • ./dotnet.sh test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.ScaffoldingServiceTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"

  • ./dotnet.sh test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.PackageJsonMergerTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"

  • ./dotnet.sh 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"

  • ./dotnet.sh test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-method "*.InitCommand_WhenTypeScriptSelected_CreatesAppHostAndAspireConfig" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"

  • ./dotnet.sh test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-method "*.InitCommand_WhenTypeScriptSelected_CreatesAppHostAndAspireConfig" --filter-method "*.InitCommand_WhenBrownfieldTypeScriptSelected_DisplaysNestedAppHostPath" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"

  • ./dotnet.sh test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-method "*.NewCommandWithoutTemplateCanCreateTypeScriptEmptyTemplate" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"

  • ./dotnet.sh test --project tests/Aspire.Cli.EndToEnd.Tests/Aspire.Cli.EndToEnd.Tests.csproj --no-launch-profile -- --filter-method "*.InitTypeScriptAppHost_AugmentsExistingViteRepoAtRoot" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true" --hangdump --hangdump-timeout 8m

  • LocalHive manual smoke matrix with the rebuilt CLI archive: CommonJS npm turborepo, ESM npm turborepo, and CommonJS pnpm turborepo.

  • ./dotnet.sh build tests/Aspire.Cli.EndToEnd.Tests/Aspire.Cli.EndToEnd.Tests.csproj --no-restore

Remove generated ESLint configuration and lint scripts from TypeScript AppHost scaffolding so brownfield projects do not get new lint behavior injected by aspire init. Move vscode-jsonrpc to devDependencies with the rest of the AppHost tooling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 8, 2026 21:24
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16895

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16895"

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adjusts the TypeScript AppHost scaffolding output so aspire init doesn’t inject ESLint/linting behavior into existing JS/TS repositories, and aligns the generated package.json dependency shape with “tooling-only” expectations.

Changes:

  • Removed generation of eslint.config.mjs, removed aspire:lint (and related lint/pre*) scripts, and stopped scaffolding eslint / typescript-eslint.
  • Moved vscode-jsonrpc from dependencies to devDependencies in the scaffolded package.json output.
  • Updated scaffold and package merge tests to reflect the new package shape and script set.
Show a summary per file
File Description
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptLanguageSupport.cs Updates TypeScript AppHost scaffold output to remove ESLint config/scripts and move vscode-jsonrpc to devDependencies.
src/Aspire.Cli/Scaffolding/PackageJsonMerger.cs Updates comments describing Node engines enforcement rationale (now focused on TS AppHost tooling).
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/TypeScriptLanguageSupportTests.cs Updates scaffold expectations: no ESLint files/scripts/deps; dependencies omitted; vscode-jsonrpc in devDependencies.
tests/Aspire.Cli.Tests/Scaffolding/PackageJsonMergerTests.cs Updates merge tests to expect vscode-jsonrpc under devDependencies and no aspire:lint in relevant scaffolds.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 1

Comment thread src/Aspire.Cli/Scaffolding/PackageJsonMerger.cs
Preserve existing package dependency placement when scaffolded devDependencies already exist under dependencies, upgrading in place instead of adding a duplicate entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
davidfowl previously approved these changes May 9, 2026
@davidfowl davidfowl requested a review from sebastienros as a code owner May 9, 2026 07:05
@davidfowl davidfowl force-pushed the maddymontaquila/aspireify-feedback branch 4 times, most recently from 4cf6f27 to 41ba145 Compare May 9, 2026 14:55
Pass an open scaffold options bag through the CLI/RPC scaffolding path and let TypeScript project it into typed options. Disable linting for init while keeping it enabled for greenfield TypeScript empty AppHost creation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@davidfowl davidfowl force-pushed the maddymontaquila/aspireify-feedback branch from 41ba145 to 67d4274 Compare May 9, 2026 22:30
@davidfowl davidfowl dismissed their stale review May 10, 2026 20:09

change of heart

davidfowl and others added 2 commits May 10, 2026 13:34
Create TypeScript AppHosts for existing package.json projects in a nested aspire-apphost package and add root scripts that delegate to it. Remove the scaffold options RPC plumbing that only toggled linting because the isolated package now owns AppHost package semantics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use pnpm install --ignore-workspace for generated TypeScript AppHost dependency installation so nested brownfield AppHost packages do not depend on being listed in a parent pnpm workspace.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
GitHub was asked to rerun all failed jobs for that attempt, and the rerun is being tracked in the rerun attempt.
The job links below point to the failed attempt jobs that matched the retry-safe transient failure rules.

@github-actions
Copy link
Copy Markdown
Contributor

Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
GitHub was asked to rerun all failed jobs for that attempt, and the rerun is being tracked in the rerun attempt.
The job links below point to the failed attempt jobs that matched the retry-safe transient failure rules.

@davidfowl
Copy link
Copy Markdown
Contributor

Brownfield TypeScript monorepo smoke results

I tested aspire init --language typescript plus the aspireify flow against a few React-style monorepo layouts using a LocalHive build from this PR.

Scenario Result Notes
CommonJS npm turborepo ✅ Pass Root package semantics are preserved; aspire-apphost/ owns the ESM AppHost package metadata.
ESM npm turborepo ✅ Pass Root "type": "module" is preserved; nested AppHost still works.
CommonJS pnpm turborepo ✅ Pass after latest fix Initially exposed a pnpm workspace install gap; fixed by installing the generated AppHost package with pnpm install --ignore-workspace.

The main brownfield issue this PR avoids is package-scope leakage. The generated TypeScript AppHost currently uses top-level await, so if apphost.ts lives directly under a CommonJS package scope, Node/tsx treats the file as CommonJS and top-level await fails. We should not solve that by adding "type": "module" to the user’s root package because that can break existing monorepos. The nested aspire-apphost/ package gives the AppHost its own ESM boundary without changing the user’s app package.

The pnpm-specific wrinkle is that pnpm install from aspire-apphost/ can still discover a parent pnpm-workspace.yaml. If that workspace does not include aspire-apphost, pnpm does not install the AppHost package’s dev dependencies, so npm run aspire:build -> pnpm --dir aspire-apphost run aspire:build fails with missing tsc. Using pnpm install --ignore-workspace keeps the generated AppHost isolated instead of mutating the user’s workspace config. The tradeoff is that users who want aspire-apphost to be a first-class workspace package should opt into that manually by adding it to pnpm-workspace.yaml.

One product question remains separate from this PR shape: generated apphost.ts still relies on top-level await. The nested brownfield package makes that safe for brownfield CommonJS roots, but if we want the generated file to be robust regardless of package scope, we should decide whether to stop using top-level await or emit an explicitly ESM file such as apphost.mts.

@sebastienros sebastienros self-assigned this May 12, 2026
Copy link
Copy Markdown
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

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

Reviewed the changes for isolating the brownfield TypeScript AppHost into a nested aspire-apphost/ package. Core design is solid — the package-boundary approach cleanly fixes the JS/TS leakage problem, the merger dedup logic correctly handles vscode-jsonrpc moving from dependencies to devDependencies, and the test coverage (including the new DevDependencyAlreadyInDependencies_IsNotDuplicated regression test and the ScaffoldingService directory-selection tests) is good.

Approving with 2 minor follow-up suggestions inline — both are minor and non-blocking.

Comment thread src/Aspire.Cli/Commands/InitCommand.cs
Comment thread src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptLanguageSupport.cs Outdated
Show the nested AppHost path in init success output and give generated brownfield AppHost packages a distinct package name to avoid workspace name collisions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@davidfowl
Copy link
Copy Markdown
Contributor

@IEvangelist thanks for the review. I accepted both follow-up suggestions and pushed fixes for the nested init success path and distinct brownfield AppHost package name in 214f4fb.

davidfowl and others added 2 commits May 11, 2026 22:21
Accept the nested AppHost creation message in the brownfield TypeScript CLI E2E test while keeping compatibility with the older root-level message used by released CLI runs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wait directly for the nested AppHost creation message now that brownfield TypeScript init reports the generated relative path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🎬 CLI E2E Test Recordings — 78 recordings uploaded (commit 9d8b6e4)

View all recordings
Status Test Recording
AddPackageInteractiveWhileAppHostRunningDetached ▶️ View Recording
AddPackageWhileAppHostRunningDetached ▶️ View Recording
AgentCommands_AllHelpOutputs_AreCorrect ▶️ View Recording
AgentInitCommand_DefaultSelection_InstallsSkillOnly ▶️ View Recording
AgentInitCommand_MigratesDeprecatedConfig ▶️ View Recording
AspireAddPackageVersionToDirectoryPackagesProps ▶️ View Recording
AspireInitSingleFileAppHostRunsViaDotnetRunAppHost ▶️ View Recording
AspireUpdateRemovesAppHostPackageVersionFromDirectoryPackagesProps ▶️ View Recording
Banner_DisplayedOnFirstRun ▶️ View Recording
Banner_DisplayedWithExplicitFlag ▶️ View Recording
Banner_NotDisplayedWithNoLogoFlag ▶️ View Recording
CertificatesClean_RemovesCertificates ▶️ View Recording
CertificatesTrust_WithNoCert_CreatesAndTrustsCertificate ▶️ View Recording
CertificatesTrust_WithUntrustedCert_TrustsCertificate ▶️ View Recording
ConfigSetGet_CreatesNestedJsonFormat ▶️ View Recording
CreateAndRunAspireStarterProject ▶️ View Recording
CreateAndRunAspireStarterProjectWithBundle ▶️ View Recording
CreateAndRunEmptyAppHostProject ▶️ View Recording
CreateAndRunJavaEmptyAppHostProject ▶️ View Recording
CreateAndRunJsReactProject ▶️ View Recording
CreateAndRunPythonReactProject ▶️ View Recording
CreateAndRunTypeScriptEmptyAppHostProject ▶️ View Recording
CreateAndRunTypeScriptStarterProject ▶️ View Recording
CreateJavaAppHostWithViteApp ▶️ View Recording
CreateTypeScriptAppHostWithViteApp_UsesConfiguredToolchain ▶️ View Recording
DashboardRunWithOtelTracesReturnsNoTraces ▶️ View Recording
DeployK8sBasicApiService ▶️ View Recording
DeployK8sWithExternalHelmChart ▶️ View Recording
DeployK8sWithGarnet ▶️ View Recording
DeployK8sWithMongoDB ▶️ View Recording
DeployK8sWithMySql ▶️ View Recording
DeployK8sWithPostgres ▶️ View Recording
DeployK8sWithRabbitMQ ▶️ View Recording
DeployK8sWithRedis ▶️ View Recording
DeployK8sWithSqlServer ▶️ View Recording
DeployK8sWithValkey ▶️ View Recording
DeployTypeScriptAppToKubernetes ▶️ View Recording
DescribeCommandResolvesReplicaNames ▶️ View Recording
DescribeCommandShowsRunningResources ▶️ View Recording
DetachFormatJsonProducesValidJson ▶️ View Recording
DetachFormatJsonProducesValidJsonWhenRestartingExistingInstance ▶️ View Recording
DoListStepsShowsPipelineSteps ▶️ View Recording
DocsCommand_RendersInteractiveMarkdownFromLocalSource ▶️ View Recording
DoctorCommand_DetectsDeprecatedAgentConfig ▶️ View Recording
DoctorCommand_TypeScriptAppHostReportsMissingConfiguredToolchain ▶️ View Recording
DoctorCommand_WithSslCertDir_ShowsTrusted ▶️ View Recording
DoctorCommand_WithoutSslCertDir_ShowsPartiallyTrusted ▶️ View Recording
GlobalMigration_HandlesCommentsAndTrailingCommas ▶️ View Recording
GlobalMigration_HandlesMalformedLegacyJson ▶️ View Recording
GlobalMigration_PreservesAllValueTypes ▶️ View Recording
GlobalMigration_SkipsWhenNewConfigExists ▶️ View Recording
GlobalSettings_MigratedFromLegacyFormat ▶️ View Recording
InitTypeScriptAppHost_AugmentsExistingViteRepoAtRoot ▶️ View Recording
InteractiveCSharpInitCreatesExpectedFiles ▶️ View Recording
InvalidAppHostPathWithComments_IsHealedOnRun ▶️ View Recording
LatestCliCanStartStableChannelAppHost ▶️ View Recording
LatestCliCanStartStableChannelTypeScriptAppHost ▶️ View Recording
LegacySettingsMigration_AdjustsRelativeAppHostPath ▶️ View Recording
LogsCommandShowsResourceLogs ▶️ View Recording
OtelLogsReturnsStructuredLogsFromStarterAppCore ▶️ View Recording
PsCommandListsRunningAppHost ▶️ View Recording
PsFormatJsonOutputsOnlyJsonToStdout ▶️ View Recording
PublishWithConfigureEnvFileUpdatesEnvOutput ▶️ View Recording
PublishWithDockerComposeServiceCallbackSucceeds ▶️ View Recording
PublishWithoutOutputPathUsesAppHostDirectoryDefault ▶️ View Recording
RestoreGeneratesSdkFiles ▶️ View Recording
RestoreGeneratesSdkFiles_WithConfiguredToolchain ▶️ View Recording
RestoreRefreshesGeneratedSdkAfterAddingIntegration ▶️ View Recording
RestoreSupportsConfigOnlyHelperPackageAndCrossPackageTypes ▶️ View Recording
RunFromParentDirectory_UsesExistingConfigNearAppHost ▶️ View Recording
SecretCrudOnDotNetAppHost ▶️ View Recording
SecretCrudOnTypeScriptAppHost ▶️ View Recording
StagingChannel_ConfigureAndVerifySettings_ThenSwitchChannels ▶️ View Recording
StartAndWaitForTypeScriptSqlServerAppHostWithNativeAssets ▶️ View Recording
StopAllAppHostsFromAppHostDirectory ▶️ View Recording
StopNonInteractiveSingleAppHost ▶️ View Recording
StopWithNoRunningAppHostExitsSuccessfully ▶️ View Recording
UnAwaitedChainsCompileWithAutoResolvePromises ▶️ View Recording

📹 Recordings uploaded automatically from CI run #25715542248

@davidfowl
Copy link
Copy Markdown
Contributor

End-to-end test of this PR against vbenjs/vue-vben-admin (pnpm monorepo). Wired the AppHost up to backend-mock + 6 Vite frontends; all 7 resources came up Running/Healthy with the PR CLI 13.4.0-pr.16895.g9d8b6e4f. Init + run path work, but a few rough edges showed up:

1. Trailing newline stripped from root package.json

The JSON-merge step that adds delegate scripts removes the final newline. Triggers prettier/editorconfig violations on most repos.

2. aspire.config.json is created lazily by aspire run, not by aspire init — and it lands in aspire-apphost/, not at the repo root

After aspire init, only aspire-apphost/apphost.run.json exists (profiles/ports). The first aspire run writes:

{ "appHost": { "path": "apphost.ts", "language": "typescript/nodejs" },
  "sdk": { "version": "13.4.0-pr.16895.g9d8b6e4f" },
  "channel": "pr-16895" }

…but inside the AppHost folder. If that's intentional (config co-located with AppHost), worth documenting; if not, init should write it eagerly.

3. Generated aspire-apphost/package.json lifecycle scripts hardcode npm run …

The CLI correctly detects pnpm (Selected TypeScript AppHost package manager 'pnpm' because packageManager 'pnpm@10.33.0' found in …) and uses pnpm for the install, but the script bodies are still:

"predev": "npm run aspire:lint",
"dev":    "npm run aspire:start",
...

In a pnpm-only repo ("preinstall": "npx only-allow pnpm") those will fail. Package-manager detection should propagate into the script bodies too.

4. Init isn't idempotent on partial failure

When my first run was interrupted during pnpm install, re-running aspire init printed ✅ aspire-apphost/apphost.ts already exists — skipping. and exited 0 without finishing the install. Had to manually nuke aspire-apphost/ and revert root package.json before retrying.

5. aspire init --language typescript doesn't add Aspire.Hosting.JavaScript 🚨

The most common reason to pick --language typescript is to model JS apps — but the skeleton apphost.ts and the generated .modules/aspire.ts only expose container/database/SQL APIs. addNodeApp/addViteApp are missing until the user manually runs aspire add javascript. Symptom when wiring a JS monorepo without that step:

apphost.ts(27,6): error TS2339: Property 'addNodeApp' does not exist on type 'DistributedApplicationBuilder'.
apphost.ts(43,10): error TS2339: Property 'addViteApp' does not exist on type 'DistributedApplicationBuilder'.
❌ An unexpected error occurred: The TypeScript (Node.js) apphost failed.

Suggest auto-detecting package.json files during init and pre-adding Aspire.Hosting.JavaScript, or shipping those types in the base SDK module for TS inits.

6. aspire run purges AppHost node_modules and aborts without a TTY

On aspire run the CLI tries to delete + recreate aspire-apphost/node_modules. In CI / docker exec -d / agent contexts this fails immediately with:

ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY
  Aborted removal of modules directory due to no TTY
❌ An unexpected error occurred: Failed to install TypeScript (Node.js) dependencies.

Workaround: CI=true. Real fix: the CLI should pass --config.confirmModulesPurge=false (or set CI=true) on the pnpm shellout it owns, or skip the purge when existing modules already satisfy the lockfile — the user didn't ask pnpm to purge anything, Aspire did.

7. Dashboard binds loopback with no flag to bind elsewhere

aspire run prints https://localhost:17030/login?t=… and binds the dashboard to 127.0.0.1. Inside a container with -p 17030:17030, host browsers can't reach it. Would be useful to have a --dashboard-host 0.0.0.0 flag (or a documented env var).


Net result: the PR's init scaffolding works on a real-world pnpm monorepo, and once you aspire add javascript the wired AppHost runs cleanly. #5 is the most painful for the target scenario; #6 is the most painful for non-interactive use.

Happy to share the full validation report if useful.

@sebastienros
Copy link
Copy Markdown
Contributor

Attempting to solve it while introducing .mts files in this PR

@davidfowl davidfowl closed this May 13, 2026
@microsoft-github-policy-service microsoft-github-policy-service Bot added this to the 13.4 milestone May 13, 2026
@sebastienros sebastienros removed this from the 13.4 milestone May 13, 2026
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.

5 participants