Fix npm package installation to check appropriate files based on command (#711)

* Initial plan for issue

* Initial analysis and plan for npm package installation fix

Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>

* Fix npm package installation to check appropriate files based on command

Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>

* Clean up temporary files and restore original target frameworks

Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>

* Fixing compiler errors

---------

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>

Author: Copilot

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

@@ -46,16 +46,29 @@ private async Task PerformInstall(NodeAppResource resource, CancellationToken ca
     {
         var logger = loggerService.GetLogger(resource);
 
-        var packageJsonPath = Path.Combine(resource.WorkingDirectory, lockfile);
+        // For 'ci' command, we need the lockfile. For 'install' command, package.json is sufficient
+        string requiredFile;
+        string requiredFilePath;
+        
+        if (installCommand == "ci")
+        {
+            requiredFile = lockfile;
+            requiredFilePath = Path.Combine(resource.WorkingDirectory, lockfile);
+        }
+        else
+        {
+            requiredFile = "package.json";
+            requiredFilePath = Path.Combine(resource.WorkingDirectory, "package.json");
+        }
 
-        if (!File.Exists(packageJsonPath))
+        if (!File.Exists(requiredFilePath))
         {
             await notificationService.PublishUpdateAsync(resource, state => state with
             {
-                State = new($"No {lockfile} file found in {resource.WorkingDirectory}", KnownResourceStates.FailedToStart)
+                State = new($"No {requiredFile} file found in {resource.WorkingDirectory}", KnownResourceStates.FailedToStart)
             }).ConfigureAwait(false);
 
-            throw new InvalidOperationException($"No {lockfile} file found in {resource.WorkingDirectory}");
+            throw new InvalidOperationException($"No {requiredFile} file found in {resource.WorkingDirectory}");
         }
 
         await notificationService.PublishUpdateAsync(resource, state => state with

tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/PackageInstallationTests.cs

@@ -0,0 +1,34 @@
+using Aspire.Hosting;
+
+namespace CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests;
+
+public class PackageInstallationTests
+{
+    /// <summary>
+    /// This test validates that the WithNpmPackageInstallation method can be called
+    /// and properly registers the lifecycle hook for npm install operations.
+    /// The issue in #618 would cause failures when no package-lock.json exists
+    /// but package.json does exist when using npm install (not ci).
+    /// </summary>
+    [Fact]
+    public void WithNpmPackageInstallation_CanBeConfiguredWithInstallAndCIOptions()
+    {
+        var builder = DistributedApplication.CreateBuilder();
+
+        var nodeApp = builder.AddNpmApp("test-app", "./test-app");
+
+        // Test that both configurations can be set up without errors
+        nodeApp.WithNpmPackageInstallation(useCI: false); // Uses npm install
+
+        var nodeApp2 = builder.AddNpmApp("test-app-ci", "./test-app-ci");
+        nodeApp2.WithNpmPackageInstallation(useCI: true);  // Uses npm ci
+
+        using var app = builder.Build();
+
+        var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
+        var resources = appModel.Resources.OfType<NodeAppResource>().ToList();
+
+        Assert.Equal(2, resources.Count);
+        Assert.All(resources, resource => Assert.Equal("npm", resource.Command));
+    }
+}

tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/ResourceCreationTests.cs

@@ -159,4 +159,40 @@ public void ViteAppHasExposedExternalHttpEndpoints()
 
         Assert.Contains(endpoints, e => e.IsExternal);
     }
+
+    [Fact]
+    public void WithNpmPackageInstallationDefaultsToInstallCommand()
+    {
+        var builder = DistributedApplication.CreateBuilder();
+
+        var nodeApp = builder.AddNpmApp("test-app", "./test-app");
+
+        // Add package installation with default settings (should use npm install, not ci)
+        nodeApp.WithNpmPackageInstallation(useCI: false);
+
+        using var app = builder.Build();
+
+        // Verify that the resource was created successfully
+        var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
+        var resource = Assert.Single(appModel.Resources.OfType<NodeAppResource>());
+        Assert.Equal("npm", resource.Command);
+    }
+
+    [Fact]
+    public void WithNpmPackageInstallationCanUseCICommand()
+    {
+        var builder = DistributedApplication.CreateBuilder();
+
+        var nodeApp = builder.AddNpmApp("test-app", "./test-app");
+
+        // Add package installation with CI enabled
+        nodeApp.WithNpmPackageInstallation(useCI: true);
+
+        using var app = builder.Build();
+
+        // Verify that the resource was created successfully
+        var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
+        var resource = Assert.Single(appModel.Resources.OfType<NodeAppResource>());
+        Assert.Equal("npm", resource.Command);
+    }
 }