feat: nginx improvements #1545
51
nginx.conf
|
@ -14,6 +14,10 @@ http {
|
|||
include /etc/nginx/mime.types;
|
||||
default_type application/octet-stream;
|
||||
|
||||
types {
|
||||
dpschen marked this conversation as resolved
Outdated
|
||||
application/manifest+json webmanifest;
|
||||
}
|
||||
|
||||
log_format main '$remote_addr - $remote_user [$time_local] "$request" '
|
||||
'$status $body_bytes_sent "$http_referer" '
|
||||
'"$http_user_agent" "$http_x_forwarded_for"';
|
||||
|
@ -26,7 +30,6 @@ http {
|
|||
keepalive_timeout 65;
|
||||
|
||||
gzip on;
|
||||
gzip_disable "msie6";
|
||||
|
||||
gzip_vary on;
|
||||
gzip_proxied any;
|
||||
|
@ -34,7 +37,22 @@ http {
|
|||
gzip_buffers 16 8k;
|
||||
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
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
This stayed the same. This stayed the same.
It's just indented
|
||||
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;
|
||||
|
||||
map_hash_max_size 128;
|
||||
map_hash_bucket_size 128;
|
||||
dpschen
commented
This path was wrong This path was wrong
konrad
commented
What about mime types like 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.
dpschen
commented
Where did it get cached in the old implementation? Not sure where I would have removed that? Where did it get cached in the old implementation? Not sure where I would have removed that?
konrad
commented
IIRC using These may even duplicated since we're already adding a caching header to everything in 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`?
dpschen
commented
I didn't test. I didn't test.
To be honest I'm only guessing here 🤷♂️
konrad
commented
Can you test it? Can you test it?
dpschen
commented
Yes, but probably my time is better spent reviewing other branches :D 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).
konrad
commented
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. huh yeah I get that :D
The nginx config in the repo is the one used by the docker file. That means running
```
docker build . -t vikunja/frontend:dev
docker run -p 10002:80 vikunja/frontend:dev
```
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.
dpschen
commented
That helps, will try. That helps, will try.
|
||||
|
@ -42,7 +60,6 @@ http {
|
|||
# Expires map
|
||||
map $sent_http_content_type $expires {
|
||||
default off;
|
||||
text/html max;
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
If we cache html, how should the browser know of new changes? Or am I missing something? If we cache html, how should the browser know of new changes? Or am I missing something?
konrad
commented
That's correct. We should not cache the html. That's correct. We should not cache the html.
|
||||
text/css max;
|
||||
application/javascript max;
|
||||
text/javascript max;
|
||||
|
@ -53,7 +70,7 @@ http {
|
|||
image/svg+xml max;
|
||||
image/x-icon max;
|
||||
audio/wav max;
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
Not sure if this is a duplication. Not sure if this is a duplication.
I remember also that we had some problems with `try_files`.
konrad
commented
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 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 in `assets/`.
|
||||
~image/ max;
|
||||
~images/ max;
|
||||
~font/ max;
|
||||
}
|
||||
|
||||
|
@ -65,20 +82,34 @@ http {
|
|||
|
||||
expires $expires;
|
||||
|
||||
location ~* .(txt|webmanifest|css|js|mjs|map|svg|jpg|jpeg|png|ico|ttf|woff|woff2|wav)$ {
|
||||
root /usr/share/nginx/html;
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
See https://www.nginx.com/resources/wiki/start/topics/tutorials/config_pitfalls/#root-inside-location-block
|
||||
try_files $uri $uri/ =404;
|
||||
|
||||
# all assets contain hash in filename, cache forever
|
||||
location ^~ /assets/ {
|
||||
add_header Cache-Control "public, max-age=31536000, s-maxage=31536000, immutable";
|
||||
try_files $uri =404;
|
||||
}
|
||||
|
||||
# all workbox scripts are compiled with hash in filename, cache forever3
|
||||
location ^~ /workbox- {
|
||||
add_header Cache-Control "public, max-age=31536000, s-maxage=31536000, immutable";
|
||||
try_files $uri =404;
|
||||
dpschen marked this conversation as resolved
Outdated
konrad
commented
Now there's two
Now there's two `location /` blocks. This breaks the config:
```
2022/02/19 21:06:32 [emerg] 30#30: duplicate location "/" in /etc/nginx/nginx.conf:96
nginx: [emerg] duplicate location "/" in /etc/nginx/nginx.conf:96
```
dpschen
commented
I removed the old block I removed the old block
|
||||
}
|
||||
|
||||
# assume that everything else is handled by the application router, by injecting the index.html.
|
||||
location / {
|
||||
root /usr/share/nginx/html;
|
||||
try_files $uri $uri/ /index.html;
|
||||
index index.html;
|
||||
autoindex off;
|
||||
expires off;
|
||||
add_header Cache-Control "public, max-age=0, s-maxage=0, must-revalidate" always;
|
||||
try_files $uri /index.html =404;
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
I hope this works =) I hope this works =)
|
||||
}
|
||||
|
||||
location ~* .(txt|webmanifest|css|js|mjs|map|svg|jpg|jpeg|png|ico|ttf|woff|woff2|wav)$ {
|
||||
dpschen
commented
Not sure if this still makes sense Not sure if this still makes sense
konrad
commented
Why wouldn't it? Why wouldn't it?
dpschen
commented
Because all those files should be in the assets folder or have their own rule. Because all those files should be in the assets folder or have their own rule.
konrad
commented
I guess it won't hurt to keep them as a backup. I guess it won't hurt to keep them as a backup.
|
||||
try_files $uri $uri/ =404;
|
||||
}
|
||||
|
||||
error_page 500 502 503 504 /50x.html;
|
||||
location = /50x.html {
|
||||
root /usr/share/nginx/html;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
See: https://vite-plugin-pwa.netlify.app/deployment/nginx.html#configure-manifest-webmanifest-mime-type
This seems to be necessary, since it returns: