feat: nginx improvements #1545
No reviewers
Labels
No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
No Milestone
No project
No Assignees
3 Participants
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vikunja/frontend#1545
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "dpschen/frontend:feature/nginx-improvements"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
I have not a lot of experience with nginx — see this as foundation for
discussion.
I came up with these changes when going through the vite-plugin-pwa documentation, I think (this is from a stash from a while ago and I didn't want to submit something I wasn't sure of).
@ -68,2 +90,3 @@
}
location ~* .(txt|webmanifest|css|js|mjs|map|svg|jpg|jpeg|png|ico|ttf|woff|woff2|wav)$ {
root /usr/share/nginx/html;
See https://www.nginx.com/resources/wiki/start/topics/tutorials/config_pitfalls/#root-inside-location-block
@ -42,7 +45,6 @@ http {
# Expires map
map $sent_http_content_type $expires {
default off;
text/html max;
If we cache html, how should the browser know of new changes? Or am I missing something?
That's correct. We should not cache the html.
@ -54,3 +56,3 @@
image/x-icon max;
audio/wav max;
~image/ max;
~images/ max;
This path was wrong
What about mime types like
image/png
? Those would not get cached with the new implementation you proposed here, but would get cached with the old one iirc.Where did it get cached in the old implementation? Not sure where I would have removed that?
IIRC using
~
at the beginning tells nginx to handle this like a regular expression. That would still not cache images I think hmmmm. I'm not even sure what this does. Did you test it?These may even duplicated since we're already adding a caching header to everything in
/assets
?I didn't test.
To be honest I'm only guessing here 🤷♂️
Can you test it?
Yes, but probably my time is better spent reviewing other branches :D
I've almost no experience setting nginx up (mostly I was just messing around with an existing config).
huh yeah I get that :D
The nginx config in the repo is the one used by the docker file. That means running
should build everything and make the frontend accessible on port 10002 on your machine with the new config (at least that's what I used to test it).
Will check tomorrow (or after that) if everything works as expected.
That helps, will try.
Hi dpschen!
Thank you for creating a PR!
I've deployed the changes of this PR on a preview environment under this URL: https://1545-feature-nginx-improvements--vikunja-frontend-preview.netlify.app
You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.
Have a nice day!
@ -67,1 +69,4 @@
root /usr/share/nginx/html;
# all assets contain hash in filename, cache forever
Not sure if this is a duplication.
I remember also that we had some problems with
try_files
.I think this is fine. Back when I initially created the nginx.conf file, we didn't have vite so all assets would go in top-level
css/
,js/
, etc folders, not all bundled up inassets/
.@ -14,6 +14,10 @@ http {
include /etc/nginx/mime.types;
default_type application/octet-stream;
types {
See: https://vite-plugin-pwa.netlify.app/deployment/nginx.html#configure-manifest-webmanifest-mime-type
This seems to be necessary, since it returns:
WIP: feat: nginx improvementsto feat: nginx improvements@ -70,4 +93,4 @@
try_files $uri $uri/ =404;
}
location / {
Now there's two
location /
blocks. This breaks the config:I removed the old block
Please fix the duplicate location block.
e89b279c0c
to40bf139210
@ -35,3 +38,3 @@
gzip_http_version 1.1;
gzip_min_length 256;
gzip_types text/plain text/css application/json application/x-javascript application/javascript text/xml application/xml application/xml+rss text/javascript application/vnd.ms-fontobject application/x-font-ttf font/opentype image/svg+xml image/x-icon audio/wav;
gzip_types
This stayed the same.
It's just indented
I used tabs by accident while there where spaces everywhere before.
Shall I add this to
.editorconfig
, because my editor was just following our defined rules here?@ -77,0 +104,4 @@
try_files $uri /index.html =404;
}
location ~* .(txt|webmanifest|css|js|mjs|map|svg|jpg|jpeg|png|ico|ttf|woff|woff2|wav)$ {
Not sure if this still makes sense
Why wouldn't it?
Because all those files should be in the assets folder or have their own rule.
I guess it won't hurt to keep them as a backup.
@ -77,0 +101,4 @@
autoindex off;
expires off;
add_header Cache-Control "public, max-age=0, s-maxage=0, must-revalidate" always;
try_files $uri /index.html =404;
I hope this works =)
Looks like it :)
40bf139210
to34875e6d73
I'd go with tabs since we're using them everywhere, but it should be the same for the whole file.
Will convert
34875e6d73
to99abedb801
99abedb801
to680857c7d9
680857c7d9
toa797bdd7d1
Looks like everything works, merging.
Thanks!