Refactor aspire new layering for polyglot templates#14677
Refactor aspire new layering for polyglot templates#14677davidfowl merged 8 commits intorelease/13.2from
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14677Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14677" |
93dc534 to
24df40f
Compare
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22523674347 |
102143d to
696aca4
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the aspire new command to separate .NET and polyglot template creation through cleaner abstractions. Instead of branching at the command level, templates now declare their runtime model (DotNet/Polyglot) and route through appropriate factory implementations.
Changes:
- Introduced
TemplateRuntimeenum andITemplateVersionPrompterinterface to separate template-specific concerns from command-level logic - Added
PolyglotTemplateFactorywith support for generic polyglot AppHost creation and a TypeScript starter template (Node.js + React) - Refactored
NewCommandto resolve language once and route to the appropriate template runtime - Moved .NET SDK installation gating from
NewCommandintoDotNetTemplateFactoryfor better separation of concerns
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Templating/TemplateRuntime.cs | New enum defining DotNet and Polyglot runtime models |
| src/Aspire.Cli/Templating/PolyglotTemplateFactory.cs | New factory for polyglot templates with embedded TypeScript starter template |
| src/Aspire.Cli/Templating/ITemplate.cs | Added Runtime and IncludeAsSubcommand properties for template routing |
| src/Aspire.Cli/Templating/ITemplateProvider.cs | Added GetInitTemplates() method for init-specific templates |
| src/Aspire.Cli/Templating/TemplateProvider.cs | Implemented GetInitTemplates() to delegate to factories |
| src/Aspire.Cli/Templating/TemplateInputs.cs | Added Language property for language selection |
| src/Aspire.Cli/Templating/CallbackTemplate.cs | Updated constructor to accept runtime and includeAsSubcommand parameters |
| src/Aspire.Cli/Templating/DotNetTemplateFactory.cs | Moved SDK installation check into template application; uses ITemplateVersionPrompter |
| src/Aspire.Cli/Commands/NewCommand.cs | Refactored to resolve language first, then route to appropriate template; split ITemplateVersionPrompter from INewCommandPrompter |
| src/Aspire.Cli/Commands/InitCommand.cs | Updated to use ITemplateVersionPrompter and ITemplateProvider |
| src/Aspire.Cli/Program.cs | Registered PolyglotTemplateFactory and ITemplateVersionPrompter in DI container |
| src/Aspire.Cli/Aspire.Cli.csproj | Added embedded resources for TypeScript starter template files |
| src/Aspire.Cli/Templating/Templates/ts-starter/* | TypeScript starter template files (apphost, package.json, tsconfig, API, settings) |
| tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs | Updated test service registration for new interfaces |
| tests/Aspire.Cli.Tests/Templating/DotNetTemplateFactoryTests.cs | Updated tests for new DotNetTemplateFactory constructor parameters |
| tests/Aspire.Cli.Tests/Commands/NewCommandTests.cs | Added tests for polyglot template selection and language routing |
| localhive.ps1 | Improved CLI installation robustness with backup/restore logic and channel configuration |
| } | ||
|
|
||
| // NOTE: I am using Single(...) here because if we get to this point and we are not running the 'aspire new' without a template | ||
| // specified then we should have errored out with the help text. If we get there then someting is really wrong and we should |
There was a problem hiding this comment.
Typo in comment: "someting" should be "something".
| // specified then we should have errored out with the help text. If we get there then someting is really wrong and we should | |
| // specified then we should have errored out with the help text. If we get there then something is really wrong and we should |
mitchdenny
left a comment
There was a problem hiding this comment.
Overall this is a clean refactoring that properly separates .NET and polyglot template concerns behind the ITemplate/ITemplateFactory/ITemplateProvider abstractions. A few issues noted inline.
| var outputTask = process.StandardOutput.ReadToEndAsync(cancellationToken); | ||
| var errorTask = process.StandardError.ReadToEndAsync(cancellationToken); | ||
|
|
||
| await process.WaitForExitAsync(cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
If the cancellation token fires during WaitForExitAsync, an OperationCanceledException propagates but the spawned process (npm/vite) continues running as an orphan. The using disposal on Process does not terminate the OS process.
Consider wrapping this in a try/catch to terminate the process tree on cancellation via process.TERMINATE(entireProcessTree: true).
|
|
||
| private async Task<ITemplate> GetProjectTemplateAsync(ParseResult parseResult, string selectedLanguageId, CancellationToken cancellationToken) | ||
| { | ||
| if (!selectedLanguageId.Equals(KnownLanguageId.CSharp, StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
Should the language option type be an enum? Commandline help would then automatically include available values to the user.
f9f31b9 to
f11c271
Compare
| Options.Add(_languageOption); | ||
|
|
||
| _templates = templateProvider.GetTemplates(); | ||
| _templates = templateProvider.GetTemplatesAsync(CancellationToken.None).GetAwaiter().GetResult().ToArray(); |
There was a problem hiding this comment.
Blocking inside the constructor delays all of CLI startup.
There was a problem hiding this comment.
This one is hard to fix.
| return (true, selectedLanguageId); | ||
| } | ||
|
|
||
| private Task<ITemplate[]> GetTemplatesForPromptAsync(ParseResult parseResult, CancellationToken cancellationToken) |
| private static bool ShouldResolveCliTemplateVersion(ITemplate template) | ||
| { | ||
| return template.Runtime is TemplateRuntime.Cli && | ||
| !template.Name.Equals("aspire-empty", StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
"aspire-empty" should be a const somewhere. Also replace other places in CLI that hardcodes the string
- Add CliTemplateFactory with TypeScript starter and Empty AppHost templates - Add embedded template resources with token replacement (ports, hostName, project name) - Refactor NewCommand to route to .NET or CLI template runtime - Restore templates as subcommands for proper option scoping - Move .NET SDK gating into DotNetTemplateFactory - Split template version prompting into ITemplateVersionPrompter - Add localhive.ps1 improvements (backup/rename, locked DLLs, channel config) - Use workspace directory name as default project name Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
360f0b1 to
47828b2
Compare
mitchdenny
left a comment
There was a problem hiding this comment.
I think this is a good change overall. Obviously the tests need to pass :) And agree with some of the nits that James has mentioned but assuming those get addressed good to merge. The Git template spike I am doing will be made better by this PR.
| }; | ||
| Options.Add(_languageOption); | ||
| } | ||
| Description = "The programming language for the AppHost.", |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make polyglot behavior default-on in CLI commands and tests, and remove obsolete config/schema/workflow/docs references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update empty apphost and secret E2E flows to account for template language prompt interactions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prevent CliTemplateFactory from prompting for localhost TLD when interactive input is unavailable, while preserving explicit option behavior and interactive prompts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Introduce TemplateNuGetConfigService and use it from DotNetTemplateFactory and CliTemplateFactory (empty C# apphost path). Also resolve CLI template version for all CLI templates, including aspire-empty, and update tests to cover package version resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Embed a full React frontend in the ts-starter template, update API payload/health endpoint, and remove runtime vite scaffolding. Also make template resource extraction binary-safe by stream-copying binary assets so files like Aspire.png are not corrupted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove ASPIRE_SHOW_DASHBOARD_RESOURCES from ts-starter apphost.run.json profiles to match current template defaults. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
This PR reworks
aspire newinto a template-first model with two execution paths and consistent language-aware selection behavior.Template architecture
TemplateRuntime.DotNet): executed throughdotnet newflows.TemplateRuntime.Cli): CLI templates are embedded resources copied to disk (with token replacement) by CLI callbacks.NewCommandnow resolves template + language intent first, then dispatches to the template runtime, instead of hardcoding behavior in command branches.Template discovery and filtering
DotNetTemplateFactorynow owns .NET SDK availability filtering; if SDK is unavailable, .NET templates are not offered.newandinitsurfaces are separated cleanly in factory output (e.g., singlefile remains oninitsurface).Language capability model
SelectableAppHostLanguages).SupportsLanguage) for filtering when--languageis provided.aspire-emptydeclares selectable AppHost languages (C# and TypeScript), while language-specific templates stay fixed.Prompting and selection flow
--languagelanguage)--languageonnewis recursive so both forms work with template commands.CLI template implementation updates
empty-apphostresources.Embedded resource reliability
<WithCulture>false</WithCulture>for embedded template resources to prevent*.run.jsonfrom being treated as culture resources.empty-apphost.apphost.run.jsonandts-starter.apphost.run.json).Validation
dotnet test tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"Fixes #14676
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: