-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add vite #108
Add vite #108
Conversation
a03ee94
to
dd6f847
Compare
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you dropping some of the instructions here without any replacement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To run the FE only, we are pointing to the prod version. So, it is very simple to run npm install
and then npm start
. The whole docker setup needed a lot of adjustments about the database dump. Do you think it is still worth it? I can put it back and adjust the Dockerfile to use vite.
"typescript": "^4.1.2", | ||
"web-vitals": "^1.0.1" | ||
}, | ||
"version": "0.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why reset to 0.0.0
if it was 0.1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vite again. Also, this is not used.
@@ -1,7 +1,7 @@ | |||
export default interface Result { | |||
end: Date; | |||
end: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
rank: number; | ||
start: Date; | ||
start: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this entirely commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it is not currently working. We need to add vitest and make some adjustments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
docker-compose.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you deleting the whole Docker Compose setup?
Does Vite not support running with Docker?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vite supports it. Same as above about docker.
No functionality change.
Actually, just a small functionality change. We don't rely on the BE anymore to get the redirect URI. This will probably speed up login time.