Skip to content
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

Support https in AddViteApp extension #518

Closed
TechWatching opened this issue Mar 1, 2025 · 12 comments · Fixed by #521
Closed

Support https in AddViteApp extension #518

TechWatching opened this issue Mar 1, 2025 · 12 comments · Fixed by #521

Comments

@TechWatching
Copy link
Contributor

Related to an existing integration?

Yes

Existing integration

Node.js hosting extensions

Overview

Currently, the AddViteApp method in the NodeJSHostingExtensions does not support using configuring an HTTPS endpoint. Adding

Usage example

        var builder = DistributedApplication.CreateBuilder();
        builder.AddViteApp("vite", useHttps: true);

Breaking change?

I'm not sure

Alternatives

Use directly AddPnpmApp and manually configure the https endpoint.

Additional context

No response

Help us help you

Yes, I'd like to be assigned to work on this item

@TechWatching
Copy link
Contributor Author

I have made the changes on this branch https://github.com/TechWatching/Aspire/tree/feature/https-viteapp but I have trouble understanding how to manage the PublicAPI.Shipped errors even by following this documenation.

@davidfowl
Copy link

What does AddViteApp do? How is it different from AddNpmApp??

@TechWatching
Copy link
Contributor Author

It calls AddNpmApp under the hood (or AddYarnApp or AddPnpmApp depending on the package manager you specify) with the dev command that is usually used to run a Vite app, so not very different. The only other thing it does it exposing the HTTP endpoint and mark it as external like you can see here.

So there is an easy workaround by directly calling AddNpmApp (or any other) and making the call to AddHttpsEndpoint. However, using AddViteApp is most straightforward for developers using Vite that just want their app to run. So adding an useHttps optional parameter would be a good addition I think.

@davidfowl
Copy link

It's very strange that this method exists and does nothing vite specific.

@TechWatching
Copy link
Contributor Author

Yes, I I think it's just a convenient method for people using Vite that don't want to bother understanding .NET Aspire code they need to write to make their vite app work with .NET Aspire. And honestly when you start discovering .NET Aspire, it's quite nice to have things like that with defaults (here something that will run the dev script and expose the endpoint) that just work to focus on other things, and then learn exactly how they work.

@TechWatching
Copy link
Contributor Author

(the code on my branch seems to work now by the way)

@Odonno
Copy link
Contributor

Odonno commented Mar 2, 2025

It's very strange that this method exists and does nothing vite specific.

I also don't see why this exist as you generally add a start script in your package.json. Simply using AddNpmApp or AddBunApp is enough IMO.

@aaronpowell
Copy link
Member

The standard vite app used dev as the run-script you use when running locally, so it's very common to just run npm run dev to launch the app when developing.

So sure, you can add start as an additional run-script, and yes, AddViteApp is really just a convenience wrapper over the conventions that vite uses for a project - run-script dev and preparing for a HTTP endpoint (or as this issue requests, a HTTPS endpoint).

I see no issue with the extension existing.

@aaronpowell
Copy link
Member

I have made the changes on this branch https://github.com/TechWatching/Aspire/tree/feature/https-viteapp but I have trouble understanding how to manage the PublicAPI.Shipped errors even by following this documenation.

What's the problem you're hitting? It might be because you've changed the overload with the new parameter, so you have to mark the old method as removed in the Unshipped file. See https://github.com/CommunityToolkit/Aspire/pull/465/files#diff-f2743000bd274b09c37fe462e8d90168603669030d2eb615bd69dc981773a8aeR15 as an example.

@davidfowl
Copy link

Don’t take this as push back, but I don’t expect our guidance to use this method when we build out more native support for front end JavaScript framework integration (which we’re looking at).

@TechWatching
Copy link
Contributor Author

I have made the changes on this branch https://github.com/TechWatching/Aspire/tree/feature/https-viteapp but I have trouble understanding how to manage the PublicAPI.Shipped errors even by following this documenation.

What's the problem you're hitting? It might be because you've changed the overload with the new parameter, so you have to mark the old method as removed in the Unshipped file. See https://github.com/CommunityToolkit/Aspire/pull/465/files#diff-f2743000bd274b09c37fe462e8d90168603669030d2eb615bd69dc981773a8aeR15 as an example.

Yes, indeed, that is fine now. I published the PR, but if you don't agree to make this breaking change, feel free to reject it.

I've just published an article showing how to use .NET Aspire with Nuxt (Vue.js framework using Vite) and mentioned AddViteApp did not yet support HTTPS and showed how to use HTTPS with methods like AddPnpmApp. But I'm curious to know what the native support for front end JavaScript framework will look like.

@aaronpowell
Copy link
Member

Don’t take this as push back, but I don’t expect our guidance to use this method when we build out more native support for front end JavaScript framework integration (which we’re looking at).

when that lands we'd re-evaluate the role of the NodeJS.Exensions package anyway.

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 a pull request may close this issue.

4 participants