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

feat: Add Vite configuration files and main.tsx #415

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

michaelto20
Copy link
Contributor

This adds the necessary Vite configuration files and the main.tsx file to the AspireJavaScript.Vite sample project. The Vite configuration files include vite.config.ts, tsconfig.json and default.conf.template, which are essential for setting up the Vite development environment. This commit sets up the basic structure of the Vite project.

This adds the necessary Vite configuration files and the main.tsx file to the AspireJavaScript.Vite sample project. The Vite configuration files include vite.config.ts, tsconfig.json and default.conf.template, which are essential for setting up the Vite development environment.  This commit sets up the basic structure of the Vite project.
Copy link
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good! Thank you, did you test the containerization aspect of this? Also, do you have a screen capture of the weather loading both locally and from the container?

@michaelto20
Copy link
Contributor Author

@IEvangelist I did test it locally and in Azure and it worked so I assume the containerization aspect worked. Is there a different way to double check? I've just run it and here are the screen captures for both the dashboard and weather app when running locally and in Azure using azd up. Is this what you were looking for?
AspireDashboardAzure
AspireViteLocalDashboard
AspireViteWeatherLocal
ViteAspireWeatherAzure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be cool to perhaps add this image to the UI to help further differentiate between the other React variation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry just seeing this, what image are you referring to?

@IEvangelist
Copy link
Member

@IEvangelist I did test it locally and in Azure and it worked so I assume the containerization aspect worked. Is there a different way to double check? I've just run it and here are the screen captures for both the dashboard and weather app when running locally and in Azure using azd up. Is this what you were looking for? AspireDashboardAzure AspireViteLocalDashboard AspireViteWeatherLocal ViteAspireWeatherAzure

Yeah, this is great! Thank you @michaelto20 - much appreciated.

Added "(Vite)" to header to disambiguate from the other React examples

Co-authored-by: David Pine <[email protected]>
@michaelto20 michaelto20 marked this pull request as ready for review August 21, 2024 17:30
@michaelto20
Copy link
Contributor Author

@IEvangelist I got distracted with work for a bit but trying to get this across the finish line. The build broke but I think it was related to an environmental error and not my code, as far as I can tell. Do you mind kicking off another build to see if it works? I tried to figure out how to do that but couldn't figure it out.

Copy link
Member

@DamianEdwards DamianEdwards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much!

@DamianEdwards DamianEdwards merged commit ceedd73 into dotnet:main Nov 14, 2024
4 of 6 checks passed
@michaelto20 michaelto20 deleted the mitownsend/reactVite branch November 14, 2024 19:17
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.

4 participants