Skip to content

Commit 90e07ac

Browse files
Copilotaaronpowell
Copilot
and
aaronpowell
authored
Refactor Node.js package installers from lifecycle hooks to ExecutableResource (#736)
* Initial plan for issue * Create installer resource classes and refactor extension methods Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> * Mark lifecycle hooks as obsolete and update tests for new resource approach Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> * Add comprehensive tests for installer resources and publish mode exclusion Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> * Add integration tests and documentation for resource-based package installers Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> * Refactor package installer resources to simplify constructors and remove obsolete tests * Should be waiting for the completion of the installer, not for it to be running * Cleanup to fix error in yarn install log --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> Co-authored-by: Aaron Powell <me@aaron-powell.com>
1 parent 9072de4 commit 90e07ac

File tree

13 files changed

+450
-3317
lines changed

13 files changed

+450
-3317
lines changed

‎examples/nodejs-ext/yarn-demo/package-lock.json

Lines changed: 0 additions & 3292 deletions
This file was deleted.

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/NodeJSHostingExtensions.cs

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
using Aspire.Hosting.ApplicationModel;
2-
using Aspire.Hosting.Lifecycle;
3-
using CommunityToolkit.Aspire.Hosting.NodeJS.Extensions;
42
using CommunityToolkit.Aspire.Utils;
5-
using Microsoft.Extensions.DependencyInjection;
63
using Microsoft.Extensions.Hosting;
74

85
namespace Aspire.Hosting;
@@ -116,8 +113,21 @@ public static IResourceBuilder<NodeAppResource> AddPnpmApp(this IDistributedAppl
116113
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
117114
public static IResourceBuilder<NodeAppResource> WithNpmPackageInstallation(this IResourceBuilder<NodeAppResource> resource, bool useCI = false)
118115
{
119-
resource.ApplicationBuilder.Services.TryAddLifecycleHook<NpmPackageInstallerLifecycleHook>(sp =>
120-
new(useCI, sp.GetRequiredService<ResourceLoggerService>(), sp.GetRequiredService<ResourceNotificationService>(), sp.GetRequiredService<DistributedApplicationExecutionContext>()));
116+
// Only install packages during development, not in publish mode
117+
if (!resource.ApplicationBuilder.ExecutionContext.IsPublishMode)
118+
{
119+
var installerName = $"{resource.Resource.Name}-npm-install";
120+
var installer = new NpmInstallerResource(installerName, resource.Resource.WorkingDirectory);
121+
122+
var installerBuilder = resource.ApplicationBuilder.AddResource(installer)
123+
.WithArgs([useCI ? "ci" : "install"])
124+
.WithParentRelationship(resource.Resource)
125+
.ExcludeFromManifest();
126+
127+
// Make the parent resource wait for the installer to complete
128+
resource.WaitForCompletion(installerBuilder);
129+
}
130+
121131
return resource;
122132
}
123133

@@ -128,7 +138,21 @@ public static IResourceBuilder<NodeAppResource> WithNpmPackageInstallation(this
128138
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
129139
public static IResourceBuilder<NodeAppResource> WithYarnPackageInstallation(this IResourceBuilder<NodeAppResource> resource)
130140
{
131-
resource.ApplicationBuilder.Services.TryAddLifecycleHook<YarnPackageInstallerLifecycleHook>();
141+
// Only install packages during development, not in publish mode
142+
if (!resource.ApplicationBuilder.ExecutionContext.IsPublishMode)
143+
{
144+
var installerName = $"{resource.Resource.Name}-yarn-install";
145+
var installer = new YarnInstallerResource(installerName, resource.Resource.WorkingDirectory);
146+
147+
var installerBuilder = resource.ApplicationBuilder.AddResource(installer)
148+
.WithArgs(["install"])
149+
.WithParentRelationship(resource.Resource)
150+
.ExcludeFromManifest();
151+
152+
// Make the parent resource wait for the installer to complete
153+
resource.WaitForCompletion(installerBuilder);
154+
}
155+
132156
return resource;
133157
}
134158

@@ -139,7 +163,21 @@ public static IResourceBuilder<NodeAppResource> WithYarnPackageInstallation(this
139163
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
140164
public static IResourceBuilder<NodeAppResource> WithPnpmPackageInstallation(this IResourceBuilder<NodeAppResource> resource)
141165
{
142-
resource.ApplicationBuilder.Services.TryAddLifecycleHook<PnpmPackageInstallerLifecycleHook>();
166+
// Only install packages during development, not in publish mode
167+
if (!resource.ApplicationBuilder.ExecutionContext.IsPublishMode)
168+
{
169+
var installerName = $"{resource.Resource.Name}-pnpm-install";
170+
var installer = new PnpmInstallerResource(installerName, resource.Resource.WorkingDirectory);
171+
172+
var installerBuilder = resource.ApplicationBuilder.AddResource(installer)
173+
.WithArgs(["install"])
174+
.WithParentRelationship(resource.Resource)
175+
.ExcludeFromManifest();
176+
177+
// Make the parent resource wait for the installer to complete
178+
resource.WaitForCompletion(installerBuilder);
179+
}
180+
143181
return resource;
144182
}
145183

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/NodePackageInstaller.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ namespace CommunityToolkit.Aspire.Hosting.NodeJS.Extensions;
1414
/// <param name="lockfile">The name of the lockfile to use.</param>
1515
/// <param name="loggerService">The logger service to use.</param>
1616
/// <param name="notificationService">The notification service to use.</param>
17+
[Obsolete("This class is used by deprecated lifecycle hooks. Package installation is now handled by installer resources.")]
1718
internal class NodePackageInstaller(string packageManager, string installCommand, string lockfile, ResourceLoggerService loggerService, ResourceNotificationService notificationService)
1819
{
1920
private readonly bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/NpmInstallerResource.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
namespace Aspire.Hosting.ApplicationModel;
2+
3+
/// <summary>
4+
/// A resource that represents an npm package installer.
5+
/// </summary>
6+
/// <param name="name">The name of the resource.</param>
7+
/// <param name="workingDirectory">The working directory to use for the command.</param>
8+
public class NpmInstallerResource(string name, string workingDirectory)
9+
: ExecutableResource(name, "npm", workingDirectory);

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/NpmPackageInstallerLifecycleHook.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ namespace CommunityToolkit.Aspire.Hosting.NodeJS.Extensions;
1414
/// <param name="loggerService">The logger service used for logging.</param>
1515
/// <param name="notificationService">The notification service used for sending notifications.</param>
1616
/// <param name="context">The execution context of the distributed application.</param>
17+
[Obsolete("Use WithNpmPackageInstallation which now creates installer resources instead of lifecycle hooks. This class will be removed in a future version.")]
1718
internal class NpmPackageInstallerLifecycleHook(
1819
bool useCI,
1920
ResourceLoggerService loggerService,

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/PnpmInstallerResource.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
namespace Aspire.Hosting.ApplicationModel;
2+
3+
/// <summary>
4+
/// A resource that represents a pnpm package installer.
5+
/// </summary>
6+
/// <param name="name">The name of the resource.</param>
7+
/// <param name="workingDirectory">The working directory to use for the command.</param>
8+
public class PnpmInstallerResource(string name, string workingDirectory)
9+
: ExecutableResource(name, "pnpm", workingDirectory);

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/PnpmPackageInstallerLifecycleHook.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace CommunityToolkit.Aspire.Hosting.NodeJS.Extensions;
1010
/// <param name="loggerService">The <see cref="ResourceLoggerService"/> to use for logging.</param>
1111
/// <param name="notificationService">The <see cref="ResourceNotificationService"/> to use for notifications to Aspire on install progress.</param>
1212
/// <param name="context">The <see cref="DistributedApplicationExecutionContext"/> to use for determining if the application is in publish mode.</param>
13+
[Obsolete("Use WithPnpmPackageInstallation which now creates installer resources instead of lifecycle hooks. This class will be removed in a future version.")]
1314
internal class PnpmPackageInstallerLifecycleHook(
1415
ResourceLoggerService loggerService,
1516
ResourceNotificationService notificationService,

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/REFACTORING_NOTES.md

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# Node.js Package Installer Refactoring
2+
3+
This refactoring transforms the Node.js package installers from lifecycle hooks to ExecutableResource-based resources, addressing issue #732.
4+
5+
## What Changed
6+
7+
### Before (Lifecycle Hook Approach)
8+
- Package installation was handled by lifecycle hooks during `BeforeStartAsync`
9+
- No visibility into installation progress in the dashboard
10+
- Limited logging capabilities
11+
- Process management handled manually via `Process.Start`
12+
13+
### After (Resource-Based Approach)
14+
- Package installers are now proper `ExecutableResource` instances
15+
- They appear as separate resources in the Aspire dashboard
16+
- Full console output visibility and logging
17+
- DCP (Distributed Application Control Plane) handles process management
18+
- Parent-child relationships ensure proper startup ordering
19+
20+
## New Resource Classes
21+
22+
### NpmInstallerResource
23+
```csharp
24+
var installer = new NpmInstallerResource("npm-installer", "/path/to/project", useCI: true);
25+
// Supports both 'npm install' and 'npm ci' commands
26+
```
27+
28+
### YarnInstallerResource
29+
```csharp
30+
var installer = new YarnInstallerResource("yarn-installer", "/path/to/project");
31+
// Executes 'yarn install' command
32+
```
33+
34+
### PnpmInstallerResource
35+
```csharp
36+
var installer = new PnpmInstallerResource("pnpm-installer", "/path/to/project");
37+
// Executes 'pnpm install' command
38+
```
39+
40+
## Usage Examples
41+
42+
### Basic Usage (No API Changes)
43+
```csharp
44+
var builder = DistributedApplication.CreateBuilder();
45+
46+
// API remains the same - behavior is now resource-based
47+
var viteApp = builder.AddViteApp("frontend", "./frontend")
48+
.WithNpmPackageInstallation(useCI: true);
49+
50+
var backendApp = builder.AddYarnApp("backend", "./backend")
51+
.WithYarnPackageInstallation();
52+
```
53+
54+
### What Happens Under the Hood
55+
```csharp
56+
// This now creates:
57+
// 1. NodeAppResource named "frontend"
58+
// 2. NpmInstallerResource named "frontend-npm-install" (child of frontend)
59+
// 3. WaitAnnotation on frontend to wait for installer completion
60+
// 4. ResourceRelationshipAnnotation linking installer to parent
61+
```
62+
63+
## Benefits
64+
65+
### Dashboard Visibility
66+
- Installer resources appear as separate items in the Aspire dashboard
67+
- Real-time console output from package installation
68+
- Clear status indication (starting, running, completed, failed)
69+
- Ability to re-run installations if needed
70+
71+
### Better Resource Management
72+
- DCP handles process lifecycle instead of manual `Process.Start`
73+
- Proper resource cleanup and error handling
74+
- Integration with Aspire's logging and monitoring systems
75+
76+
### Improved Startup Ordering
77+
- Parent resources automatically wait for installer completion
78+
- Failed installations prevent app startup (fail-fast behavior)
79+
- Clear dependency visualization in the dashboard
80+
81+
### Development vs Production
82+
- Installers only run during development (excluded from publish mode)
83+
- No overhead in production deployments
84+
- Maintains backward compatibility
85+
86+
## Migration Guide
87+
88+
### For Users
89+
No changes required! The existing APIs (`WithNpmPackageInstallation`, `WithYarnPackageInstallation`, `WithPnpmPackageInstallation`) work exactly the same.
90+
91+
### For Contributors
92+
The lifecycle hook classes are marked as `[Obsolete]` but remain functional for backward compatibility:
93+
- `NpmPackageInstallerLifecycleHook`
94+
- `YarnPackageInstallerLifecycleHook`
95+
- `PnpmPackageInstallerLifecycleHook`
96+
- `NodePackageInstaller`
97+
98+
These will be removed in a future version once all usage has migrated to the resource-based approach.
99+
100+
## Testing
101+
102+
Comprehensive test coverage includes:
103+
- Unit tests for installer resource properties and command generation
104+
- Integration tests for parent-child relationships
105+
- Cross-platform compatibility (Windows vs Unix commands)
106+
- Publish mode exclusion verification
107+
- Wait annotation and resource relationship validation

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/YarnInstallerResource.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
using System.Runtime.InteropServices;
2+
3+
namespace Aspire.Hosting.ApplicationModel;
4+
5+
/// <summary>
6+
/// A resource that represents a yarn package installer.
7+
/// </summary>
8+
/// <param name="name">The name of the resource.</param>
9+
/// <param name="workingDirectory">The working directory to use for the command.</param>
10+
public class YarnInstallerResource(string name, string workingDirectory)
11+
: ExecutableResource(name, "yarn", workingDirectory);

‎src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/YarnPackageInstallerLifecycleHook.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace CommunityToolkit.Aspire.Hosting.NodeJS.Extensions;
1010
/// <param name="loggerService">The <see cref="ResourceLoggerService"/> to use for logging.</param>
1111
/// <param name="notificationService">The <see cref="ResourceNotificationService"/> to use for notifications to Aspire on install progress.</param>
1212
/// <param name="context">The <see cref="DistributedApplicationExecutionContext"/> to use for determining if the application is in publish mode.</param>
13+
[Obsolete("Use WithYarnPackageInstallation which now creates installer resources instead of lifecycle hooks. This class will be removed in a future version.")]
1314
internal class YarnPackageInstallerLifecycleHook(
1415
ResourceLoggerService loggerService,
1516
ResourceNotificationService notificationService,

0 commit comments

Comments
 (0)