-
Notifications
You must be signed in to change notification settings - Fork 65
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
[Feature] Base setup for Frontend Docker image #279
Conversation
✅ Deploy Preview for ree6-backend ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
After these changes the PR would be ready for a merge.
Frontend/docker/start.sh
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.
This file can now be removed or stripped down to write into a .env file. Because with the latest commit there is no reason to rebuild it since we now support enviroment values.
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.
Dockerfile was modified to build the website with modifying only the adapter used.
Environment variables aren't loaded though.
I've tried setting BASE_PATH
to "https://api.example.com" but from the browser console I see that it's still api.ree6.de
.
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.
I've even tried VITE_API_URL
which I saw in the commits
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.
Here's the command: docker run -it --rm --env VITE_API_URL="https://api.example.com" --env VITE_INVITE_URL="https://ree6.example.com/invite" -p 3000:3000 ree6/frontend
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.
Very weird, try writing it into a .env file into the root of the Frontend, because thats how I did it to test it. And it worked completly fine for me. Also the env paths have to contain the VITE_
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.
Ok I'll thy that one later, I'm learning for exams RN.
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.
Did it work for you?
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.
Hello. I finally have my laptop back and I've been able to look at this.
AFAIK this cannot be done with how this project is set up. It's either hardcode it (and build it at every container start) or pass it through HTML I am not familiar with svelte so IDK how to do such a thing but in theory it should be possible for some sever-side component add BASE_PATH
to the html in eg. <head><script>
.
I have tried putting it into .env but it didn't work for me. Maybe it works at build time but once the site is built it doesn't do anything.
Frontend/docker/start.sh
Outdated
echo "Patching svelte.config.js to use adapter-node..." | ||
sed -i "s@import adapter from '\@sveltejs/adapter-auto';@import adapter from '\@sveltejs/adapter-node';@g" svelte.config.js | ||
|
||
# Patch constants.ts to use our api url |
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 work with the new enviroment stuff you would just need to do this:
rm .env
echo "VITE_API_URL=${API_URL}\nVITE_INVITE_URL=${INVITE_LINK}" > .env
I did not test this code for functionality, but it should be as easy as that.
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.
AFAIK the .env
is used during build only?
Hello. Sorry I went dark for a while - my laptop released the factory installed magic smoke so it's in RMA rn. I'll finish it once I get it back. |
to avoid the need to use `--update` and remove `/var/cache/apk/*` From https://www.codefactor.io/repository/github/ree6-applications/webinterface/pull/279
There's no longer a need for the `start.sh` script Names for the docker images were changed to use the project's name
As a follow up where you able to test it out? And if so did the enviroment variable change work for you? |
I was not yet able to make the frontend use the environment variables. |
In that case I will be checking it out today and trying to get it to work. |
Tested everything out, it seems like even thou I do not want to. We have to build the app on each start to ensure the Enviroment variables are being used. So I would ship out a pre built docker image with a bash script where the bash script rebuilds everything. The prebuilt stuff should be reused which allows quicker build times. |
I have finished a up fixing up the Dockerfile and readding the start.sh script. It seems to work now, without having to replace anything except the node processor. |
Nice. I'll add the script back then. I'm on the way home so I'll look into it in cca 45 minutes. |
I already readded it, and its merged so all good! |
As we discussed on Discord I've created a Docker image for Ree6's frontend. I added a workflow so it should build automatically with your backend image (maybe these images could be merged in the future? I think it's worth looking into).
I've also modified the
README.md
in theFrontend
directory to have instructions on how to set up the frontend.Please check the workflow. I haven't tested it but it should be OK.