Fix polyglot codegen: include base types in capability target expansion#15099
Fix polyglot codegen: include base types in capability target expansion#15099IEvangelist merged 6 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 -- 15099Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15099" |
…atibility map In BuildTypeCompatibilityMap, each concrete type was only registered under its interfaces and base types, but not under its own type ID. This meant that when expanding capabilities constrained to a base type like JavaScriptAppResource, the base type itself was missing from the expanded targets. For example, WithNpm<TResource>() where TResource : JavaScriptAppResource would only generate withNpm on NodeAppResource and ViteAppResource, but not on JavaScriptAppResource itself. The fix adds self-registration so base types with derived types are always included when expanding capabilities that target them directly. Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com>
f05ab53 to
c52777b
Compare
There was a problem hiding this comment.
Pull request overview
Fixes ATS/polyglot capability target expansion so capabilities constrained to a base type include the base type itself (not only derived types), ensuring TypeScript codegen emits methods on base resource builders like JavaScriptAppResource, ContainerResource, ExecutableResource, and ProjectResource.
Changes:
- Register each concrete type under its own ATS type ID in
BuildTypeCompatibilityMapto ensure base types are included in expanded targets. - Update TypeScript snapshot output to reflect newly generated methods appearing on base resource builder types.
- Add regression tests covering JavaScript package-manager capability expansion; add JS project reference for test compilation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Aspire.Hosting/Ats/AtsCapabilityScanner.cs | Fixes capability expansion by self-registering concrete types under their own type ID. |
| tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts | Updates expected generated TS output now that base builder types receive their capabilities. |
| tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/AtsTypeScriptCodeGeneratorTests.cs | Adds regression tests for JavaScript capability expansion behavior. |
| tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests.csproj | Adds project reference to Aspire.Hosting.JavaScript for the new scanner tests. |
You can also share your feedback on Copilot code review. Take the survey.
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/AtsTypeScriptCodeGeneratorTests.cs
Outdated
Show resolved
Hide resolved
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/AtsTypeScriptCodeGeneratorTests.cs
Show resolved
Hide resolved
...Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts
Show resolved
Hide resolved
|
The transient CI rerun workflow requested reruns for the following jobs after analyzing the failed attempt.
|
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #23006253745 |
sebastienros
left a comment
There was a problem hiding this comment.
Apply the copilot suggestions
|
Update all the other polyglot snapshots (go, java, ...) |
…riptCodeGeneratorTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Implemented methods for configuring container resources, including: - `withBindMount`: Adds a bind mount. - `withEntrypoint`: Sets the container entrypoint. - `withImageTag`: Sets the container image tag. - `withImageRegistry`: Sets the container image registry. - `withImage`: Sets the container image with optional tag. - `withImageSHA256`: Sets the image SHA256 digest. - `withContainerRuntimeArgs`: Adds runtime arguments for the container. - `withLifetime`: Sets the lifetime behavior of the container resource. - `withImagePullPolicy`: Sets the container image pull policy. - `publishAsContainer`: Configures the resource to be published as a container. - `withDockerfile`: Configures the resource to use a Dockerfile. - `withContainerName`: Sets the container name. - `withBuildArg`: Adds a build argument from a parameter resource. - `withBuildSecret`: Adds a build secret from a parameter resource. - `withEndpointProxySupport`: Configures endpoint proxy support. - `withContainerNetworkAlias`: Adds a network alias for the container. - `publishAsConnectionString`: Publishes the resource as a connection string. - `withVolume`: Adds a volume. - Added similar methods for executable resources and project resources, including: - `publishAsDockerFile`: Publishes the executable as a Docker container. - `publishAsDockerFileWithConfigure`: Publishes an executable as a Docker file with optional container configuration. - `withExecutableCommand`: Sets the executable command. - `withWorkingDirectory`: Sets the executable working directory. - `withReplicas`: Sets the number of replicas for a project. - `disableForwardedHeaders`: Disables forwarded headers for the project. - `publishAsDockerFile`: Publishes a project as a Docker file with optional container configuration.
…JS resource types
Description
BuildTypeCompatibilityMapregistered each concrete type under its interfaces and base types, but not under its own type ID. When expanding capabilities constrained to a base type (e.g.,WithNpm<T>() where T : JavaScriptAppResource), the base type itself was missing from expanded targets — only derived types likeNodeAppResourceandViteAppResourcereceived the method.Fix: Add self-registration in
BuildTypeCompatibilityMapso the expansion ofJavaScriptAppResourceyields[JavaScriptAppResource, NodeAppResource, ViteAppResource]instead of just[NodeAppResource, ViteAppResource].This also fixes the same class of bug for
ContainerResource,ExecutableResource, andProjectResource— their type-specific capabilities now correctly appear on the base type builders too.Checklist
Original prompt
withNpmmethod generated with wrong constraint #15098💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.