From c8eac140f9ebbd8cdaedf11aa87c99a4e9947a54 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Jun 2025 01:09:06 +0000 Subject: [PATCH 1/5] Initial plan for issue From 03bdee3c7a797b854ea544ae1524bf4436924082 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Jun 2025 01:13:00 +0000 Subject: [PATCH 2/5] Initial analysis and plan for npm package installation fix Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> --- ...pire.Hosting.NodeJS.Extensions.csproj.backup | 17 +++++++++++++++++ src/Directory.Build.props | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.csproj.backup diff --git a/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.csproj.backup b/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.csproj.backup new file mode 100644 index 000000000..324dc983e --- /dev/null +++ b/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.csproj.backup @@ -0,0 +1,17 @@ + + + + hosting nodejs vite yarn pnpm npm + A .NET Aspire for hosting NodeJS apps using Vite, Yarn, PNPM, or NPM. + + + + + + + + + + + + diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 29818175c..5cb777ae8 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -1,7 +1,7 @@ - $(AllTargetFrameworks) + $(DefaultTargetFramework) true README.md From 081e5d2c56022c6d0a61d35276310fcc98293be4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Jun 2025 01:15:21 +0000 Subject: [PATCH 3/5] Fix npm package installation to check appropriate files based on command Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> --- .../NodePackageInstaller.cs | 21 ++++++++--- .../PackageInstallationTests.cs | 34 ++++++++++++++++++ .../ResourceCreationTests.cs | 36 +++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/PackageInstallationTests.cs diff --git a/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/NodePackageInstaller.cs b/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/NodePackageInstaller.cs index b2ee27119..433f761a6 100644 --- a/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/NodePackageInstaller.cs +++ b/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 diff --git a/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/PackageInstallationTests.cs b/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/PackageInstallationTests.cs new file mode 100644 index 000000000..2781b1db2 --- /dev/null +++ b/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 +{ + /// + /// 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). + /// + [Fact] + public void WithNpmPackageInstallation_CanBeConfiguredWithInstallAndCIOptions() + { + var builder = DistributedApplication.CreateBuilder(); + + var nodeApp = builder.AddNpmApp("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"); + nodeApp2.WithNpmPackageInstallation(useCI: true); // Uses npm ci + + using var app = builder.Build(); + + var appModel = app.Services.GetRequiredService(); + var resources = appModel.Resources.OfType().ToList(); + + Assert.Equal(2, resources.Count); + Assert.All(resources, resource => Assert.Equal("npm", resource.Command)); + } +} \ No newline at end of file diff --git a/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/ResourceCreationTests.cs b/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/ResourceCreationTests.cs index e291145f0..beb49501d 100644 --- a/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/ResourceCreationTests.cs +++ b/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"); + + // 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(); + var resource = Assert.Single(appModel.Resources.OfType()); + Assert.Equal("npm", resource.Command); + } + + [Fact] + public void WithNpmPackageInstallationCanUseCICommand() + { + var builder = DistributedApplication.CreateBuilder(); + + var nodeApp = builder.AddNpmApp("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(); + var resource = Assert.Single(appModel.Resources.OfType()); + Assert.Equal("npm", resource.Command); + } } \ No newline at end of file From 15e5df28a4ef153fb5313df138d44679b1707178 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Jun 2025 01:17:42 +0000 Subject: [PATCH 4/5] Clean up temporary files and restore original target frameworks Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> --- ...pire.Hosting.NodeJS.Extensions.csproj.backup | 17 ----------------- src/Directory.Build.props | 2 +- 2 files changed, 1 insertion(+), 18 deletions(-) delete mode 100644 src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.csproj.backup diff --git a/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.csproj.backup b/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.csproj.backup deleted file mode 100644 index 324dc983e..000000000 --- a/src/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.csproj.backup +++ /dev/null @@ -1,17 +0,0 @@ - - - - hosting nodejs vite yarn pnpm npm - A .NET Aspire for hosting NodeJS apps using Vite, Yarn, PNPM, or NPM. - - - - - - - - - - - - diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 5cb777ae8..29818175c 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -1,7 +1,7 @@ - $(DefaultTargetFramework) + $(AllTargetFrameworks) true README.md From 7fec14b6f1f405a469c906fd325564539a3d7cc4 Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Wed, 4 Jun 2025 05:49:37 +0000 Subject: [PATCH 5/5] Fixing compiler errors --- .../PackageInstallationTests.cs | 12 ++++++------ .../ResourceCreationTests.cs | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/PackageInstallationTests.cs b/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/PackageInstallationTests.cs index 2781b1db2..57eb25d79 100644 --- a/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/PackageInstallationTests.cs +++ b/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/PackageInstallationTests.cs @@ -14,20 +14,20 @@ public class PackageInstallationTests public void WithNpmPackageInstallation_CanBeConfiguredWithInstallAndCIOptions() { var builder = DistributedApplication.CreateBuilder(); - - var nodeApp = builder.AddNpmApp("test-app"); - + + 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"); + + 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(); var resources = appModel.Resources.OfType().ToList(); - + Assert.Equal(2, resources.Count); Assert.All(resources, resource => Assert.Equal("npm", resource.Command)); } diff --git a/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/ResourceCreationTests.cs b/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/ResourceCreationTests.cs index beb49501d..63df06578 100644 --- a/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/ResourceCreationTests.cs +++ b/tests/CommunityToolkit.Aspire.Hosting.NodeJS.Extensions.Tests/ResourceCreationTests.cs @@ -164,9 +164,9 @@ public void ViteAppHasExposedExternalHttpEndpoints() public void WithNpmPackageInstallationDefaultsToInstallCommand() { var builder = DistributedApplication.CreateBuilder(); - - var nodeApp = builder.AddNpmApp("test-app"); - + + var nodeApp = builder.AddNpmApp("test-app", "./test-app"); + // Add package installation with default settings (should use npm install, not ci) nodeApp.WithNpmPackageInstallation(useCI: false); @@ -182,9 +182,9 @@ public void WithNpmPackageInstallationDefaultsToInstallCommand() public void WithNpmPackageInstallationCanUseCICommand() { var builder = DistributedApplication.CreateBuilder(); - - var nodeApp = builder.AddNpmApp("test-app"); - + + var nodeApp = builder.AddNpmApp("test-app", "./test-app"); + // Add package installation with CI enabled nodeApp.WithNpmPackageInstallation(useCI: true);