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

Added check for Vite app, opt to use Nginx instead of Node #47

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

anniebabannie
Copy link
Contributor

  • Checks if its a Vite app based on whether the "dev" script is defined as "vite"
  • Takes result of vite build from /dist and puts into Nginx's html directory

@anniebabannie anniebabannie requested a review from rubys November 21, 2023 17:23
@rubys
Copy link
Contributor

rubys commented Nov 21, 2023

Try running npm run eslint:fix and commit the results.

- Checks if its a Vite app based on whether the "dev" script is defined as "vite"
- Takes result of `vite build` from /dist and puts into Nginx's html directory
gdf.js Outdated Show resolved Hide resolved
gdf.js Outdated Show resolved Hide resolved
Some other frameworks may also utilize Vite, in which cases we dont want to create the Dockerfile as a Vite app, but rather a <whatever framework> app instead.
@anniebabannie
Copy link
Contributor Author

@rubys Looking into the failing tests now... I'm not able to reproduce the failing tests locally, and I've rebased my branch on master just to make sure everything is up-to-date.

@anniebabannie anniebabannie merged commit f9cb027 into main Nov 21, 2023
18 of 20 checks passed
@MichaelDeBoey MichaelDeBoey deleted the annie/check-for-vite branch November 21, 2023 21:27
@MichaelDeBoey
Copy link
Collaborator

@anniebabannie @rubys Will Remix + Vite be supported as well?

https://remix.run/docs/en/main/future/vite

@rubys
Copy link
Contributor

rubys commented Nov 22, 2023

@MichaelDeBoey Remix + Vite likely already works - full stack frameworks typically handle both the necessary build steps and the serving of static assets. If it doesn't work, let us know what the problem is and we'll fix it.

What @anniebabannie added was support for front-end-only (which includes pure static) applications -- the build step is the same, but the deploy is nginx with no JS runtime installed.

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.

3 participants