Skip to content

Run configureInstaller after WaitForCompletion in WithXYZPackageInstallation methods #761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 30, 2025

Conversation

afscrome
Copy link
Contributor

Closes #760

Run configureInstaller after WaitForCompletion in the WithXYZPackageInstallation methods

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • N/A New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • N/A Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

This gives consumers a chance to cancel the wait if they don't want to wait.
@aaronpowell
Copy link
Member

Would it not make more sense to introduce an overload like public static IResourceBuilder<NodeAppResource> WithNpmPackageInstallation(this IResourceBuilder<NodeAppResource> resource, bool useCI = false, bool waitForCompletion = true, Action<IResourceBuilder<NpmInstallerResource>>? configureInstaller = null), and if waitForCompletion is false then we just skip the method call.

@afscrome
Copy link
Contributor Author

Either way, I think the configureInstaller function should still be the last thing to be invoked.

As for introducing a new optional arg overload, that sort of feels like we're fighting the natural builder pattern, and hsould instead go for more of a fluent API:

frontEnd.WithNpmPackageInstallation(x => x
    .WithCi()
    .WithoutWait());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node WithXYZPackageInstallation methods should WaitForCompletion before the configure callback
2 participants