Skip to content

Commit 089b7ff

Browse files
CopiloteerhardtCopilot
authored
Refactor WithNpmPackageManager to WithNpm with install parameter (dotnet#12325)
## Refactor WithNpmPackageManager to WithNpm with install parameter ### Changes completed: - [x] Update NodeExtensions.cs to rename `WithNpmPackageManager` to `WithNpm` - [x] Change parameter name from `autoInstall` to `install` with default value `false` - [x] Update XML documentation to reflect the new parameter name and default - [x] Update example in XML documentation to use `.WithNpm(install: true)` - [x] Update all test usages to explicitly use `install: true` when installer should be created - [x] Update test for `install: false` behavior (renamed test) - [x] Update playground examples to use `.WithNpm(install: true)` - [x] Update template file in aspire-py-starter to use `.WithNpm(install: true)` - [x] Add comprehensive unit tests for new public APIs: - `WithYarn` - tests for both install=true and install=false - `WithPnpm` - tests for both install=true and install=false - `WithInstallCommand` - tests custom install command with custom args - `WithBuildCommand` - tests custom build command - Override scenarios for both install and build commands - [x] Fix existing tests to match new installer naming convention (`{name}-installer` instead of `{name}-npm-install`) - [x] Run tests to verify all changes work correctly (47 tests passed) ### Summary: Successfully added comprehensive unit tests for all new public APIs introduced in the refactoring. The tests verify: - Package manager annotations are correctly set for each package manager (npm, yarn, pnpm) - Installer resources are created/not created based on the `install` parameter - Custom install and build commands can be set using `WithInstallCommand` and `WithBuildCommand` - Existing commands can be overridden - All tests pass (47/47) * Refactor WithNpmPackageManager to WithNpm with autoInstall parameter Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> * Apply feedback: Rename autoInstall to install with default false Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> * WIP - refactor JS * First round of refactoring * Refactor and add yarn, pnmp, and WithInstall/BuildCommand APIs * Fix build * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add comprehensive unit tests for new public APIs (WithYarn, WithPnpm, WithInstallCommand, WithBuildCommand) Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> * Update AspireWithJavaScript playground app * Add more tests * Update comments. * Apply suggestion from @eerhardt * Respond to PR feedback --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 1f493e6 commit 089b7ff

File tree

13 files changed

+1292
-830
lines changed

13 files changed

+1292
-830
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ config.ps1
124124
# Node.js modules
125125
node_modules/
126126

127+
# Ignore cache created with the Angular CLI.
128+
.angular/
129+
127130
# Python Compile Outputs
128131

129132
*.pyc

playground/AspireWithJavaScript/AspireJavaScript.AppHost/AppHost.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@
44
.WithExternalHttpEndpoints();
55

66
builder.AddNpmApp("angular", "../AspireJavaScript.Angular")
7-
.WithNpmPackageManager()
7+
.WithNpm(install: true)
88
.WithReference(weatherApi)
99
.WaitFor(weatherApi)
1010
.WithHttpEndpoint(env: "PORT")
1111
.WithExternalHttpEndpoints()
1212
.PublishAsDockerFile();
1313

1414
builder.AddNpmApp("react", "../AspireJavaScript.React")
15-
.WithNpmPackageManager()
15+
.WithNpm(install: true)
1616
.WithReference(weatherApi)
1717
.WaitFor(weatherApi)
1818
.WithEnvironment("BROWSER", "none") // Disable opening browser on npm start
@@ -21,15 +21,15 @@
2121
.PublishAsDockerFile();
2222

2323
builder.AddNpmApp("vue", "../AspireJavaScript.Vue")
24-
.WithNpmPackageManager()
24+
.WithInstallCommand("npm", ["ci"]) // Use 'npm ci' for clean install
2525
.WithReference(weatherApi)
2626
.WaitFor(weatherApi)
2727
.WithHttpEndpoint(env: "PORT")
2828
.WithExternalHttpEndpoints()
2929
.PublishAsDockerFile();
3030

3131
builder.AddViteApp("reactvite", "../AspireJavaScript.Vite")
32-
.WithNpmPackageManager()
32+
.WithNpm(install: true)
3333
.WithReference(weatherApi)
3434
.WithEnvironment("BROWSER", "none")
3535
.WithExternalHttpEndpoints();

playground/AspireWithJavaScript/AspireJavaScript.Vue/package-lock.json

Lines changed: 716 additions & 596 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Aspire.Hosting.ApplicationModel;
5+
6+
namespace Aspire.Hosting.NodeJs;
7+
8+
/// <summary>
9+
/// Represents the annotation for the JavaScript package manager's build command.
10+
/// </summary>
11+
/// <param name="command">The executable command name</param>
12+
/// <param name="args">The command line arguments for the JavaScript package manager's install command.</param>
13+
public sealed class JavaScriptBuildCommandAnnotation(string command, string[] args) : IResourceAnnotation
14+
{
15+
/// <summary>
16+
/// Gets the executable command name.
17+
/// </summary>
18+
public string Command { get; } = command;
19+
20+
/// <summary>
21+
/// Gets the command-line arguments supplied to the application.
22+
/// </summary>
23+
public string[] Args { get; } = args;
24+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Aspire.Hosting.ApplicationModel;
5+
6+
namespace Aspire.Hosting.NodeJs;
7+
8+
/// <summary>
9+
/// Represents the annotation for the JavaScript package manager's install command.
10+
/// </summary>
11+
/// <param name="command">The executable command name</param>
12+
/// <param name="args">The command line arguments for the JavaScript package manager's install command.</param>
13+
public sealed class JavaScriptInstallCommandAnnotation(string command, string[] args) : IResourceAnnotation
14+
{
15+
/// <summary>
16+
/// Gets the executable command name.
17+
/// </summary>
18+
public string Command { get; } = command;
19+
20+
/// <summary>
21+
/// Gets the command-line arguments supplied to the application.
22+
/// </summary>
23+
public string[] Args { get; } = args;
24+
}

src/Aspire.Hosting.NodeJs/JavaScriptPackageManagerAnnotation.cs

Lines changed: 0 additions & 39 deletions
This file was deleted.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Aspire.Hosting.ApplicationModel;
5+
6+
namespace Aspire.Hosting.NodeJs;
7+
8+
/// <summary>
9+
/// Represents the annotation for the JavaScript resource's initial run command line arguments.
10+
/// </summary>
11+
/// <remarks>
12+
/// The Resource contains the command name, while this annotation contains only the arguments.
13+
/// These arguments are applied to the command before any user supplied arguments.
14+
/// </remarks>
15+
public sealed class JavaScriptRunCommandAnnotation(string[] args) : IResourceAnnotation
16+
{
17+
/// <summary>
18+
/// Gets the command-line arguments supplied to the application.
19+
/// </summary>
20+
public string[] Args { get; } = args;
21+
}

0 commit comments

Comments
 (0)