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

webapp(cli): migrate to vite #772

Merged
merged 4 commits into from
Aug 8, 2023
Merged

Conversation

Rotzbua
Copy link
Contributor

@Rotzbua Rotzbua commented Jul 20, 2023

Finally a working vite system :)

Reference

solves #771

Copy link
Member

@peterthomassen peterthomassen left a comment

Choose a reason for hiding this comment

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

Yay, many kudos to you!! 🎉 Thank you very much for the effort.

A few comments inline. I also noticed that the build failed with:

> vite build --no-clean

file:///usr/src/app/node_modules/vite/dist/node/cli.js:442
          throw new CACError(`Unknown option \`${name.length > 1 ? `--${name}` : `-${name}`}\``);
                ^

CACError: Unknown option `--clean`
    at Command.checkUnknownOptions (file:///usr/src/app/node_modules/vite/dist/node/cli.js:442:17)
    at CAC.runMatchedCommand (file:///usr/src/app/node_modules/vite/dist/node/cli.js:640:13)
    at CAC.parse (file:///usr/src/app/node_modules/vite/dist/node/cli.js:579:12)
    at file:///usr/src/app/node_modules/vite/dist/node/cli.js:906:5
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

Node.js v18.17.0
The command '/bin/sh -c npm run build -- --no-clean' returned a non-zero code: 1
Service 'www' failed to build : Build failed

www/webapp/src/App.vue Show resolved Hide resolved
www/webapp/src/router/index.js Show resolved Hide resolved
www/webapp/vite.config.js Show resolved Hide resolved
@Rotzbua Rotzbua force-pushed the migrate_vite branch 3 times, most recently from a2efc9b to 02894c2 Compare July 22, 2023 20:37
Copy link
Member

@peterthomassen peterthomassen left a comment

Choose a reason for hiding this comment

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

I think the last commit should be squashed into 97f1fd0, otherwise seems good to go.

peterthomassen
peterthomassen previously approved these changes Aug 2, 2023
@peterthomassen peterthomassen dismissed their stale review August 2, 2023 15:09

breaks dev environment

@peterthomassen
Copy link
Member

Before deploying, I tried to fire up the dev environment, and with this PR applied, the www container gives me "502 Bad Gateway". Do you know why that is?

@Rotzbua
Copy link
Contributor Author

Rotzbua commented Aug 2, 2023

file: docker-compose.dev.yml

Old

command: bash -c "npm install && npm run serve --fix"

New

command: bash -c "npm install && npm run dev"

Vite uses dev to serve dev build with reload.
serve is a test server to serve the prod build from folder dist.

@peterthomassen
Copy link
Member

The webapp container is still not listening on port 8080. I tried adding --host after npm run dev, but nothing changes. Do you have an idea what could cause this?

Vite is faster than the old `vue-cli`. For new `vue` projects it is the default build system.

https://vitejs.dev/
---
docker: change env vars to `vite` schema

* Replace prefix `VUE_` by `VITE_`.
* Update option to not delete output dir.
* Remove unused environment variables
This is a workaround. `vuelidate` should be updated to newer mayor.
@Rotzbua
Copy link
Contributor Author

Rotzbua commented Aug 8, 2023

I tried adding --host after npm run dev, but nothing changes. Do you have an idea what could cause this?

You found the reason and solution 👍 . I think you forgot the -- before --host

npm run dev -- --host

Copy link
Member

@peterthomassen peterthomassen left a comment

Choose a reason for hiding this comment

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

lgtm, thank you very much!

Comment on lines -75 to -76
- CODESANDBOX_SSE=1 # so that ws: protocol ends up as auto: in node_modules/@vue/cli-service/lib/commands/serve.js
- CYPRESS_CACHE_FOLDER=/tmp/.cache # https://github.com/cypress-io/cypress/issues/1281#issuecomment-404823001
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

@peterthomassen peterthomassen merged commit 3ced839 into desec-io:main Aug 8, 2023
4 checks passed
@Rotzbua Rotzbua deleted the migrate_vite branch August 8, 2023 16:08
@peterthomassen
Copy link
Member

Before:
image

After:
image

The JS size has decreased a little, while the CSS size has increase by about 8-9%. Also, the index page is no longer compressed (it now has a lot of whitespace).

Loading overall is faster now (but I didn't control for statistical artifacts).

@Rotzbua
Copy link
Contributor Author

Rotzbua commented Aug 8, 2023

I found an issue with CSP #788

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.

2 participants