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

Fix: Error 404 in staging #25

Merged
merged 3 commits into from
Jul 15, 2024
Merged

Fix: Error 404 in staging #25

merged 3 commits into from
Jul 15, 2024

Conversation

lebaudantoine
Copy link
Collaborator

@lebaudantoine lebaudantoine commented Jul 11, 2024

Purpose

It closes #24

I have added some caching on static assets, lmk @manuhabitela wdyt about it.

@lebaudantoine lebaudantoine force-pushed the fix/nginx-conf branch 2 times, most recently from 828e898 to 522c68d Compare July 11, 2024 18:31
@lebaudantoine
Copy link
Collaborator Author

lebaudantoine commented Jul 11, 2024

My fix works locally, however, I cannot deploy it to staging to test it. We need to publish the new frontend image to the registry.

One amazing enhancement would be to be able having feature branches deployed to a staging environment for review. @rouja

@lebaudantoine lebaudantoine self-assigned this Jul 11, 2024
@lebaudantoine lebaudantoine changed the title Fix/nginx conf Fix: Error 404 in staging Jul 11, 2024
Comment on lines 15 to 17
location ~* \.(css|js|json|png|jpg|jpeg|gif|ico|svg|woff|woff2|ttf|eot)$ {
expires 30d;
add_header Cache-Control "public, max-age=2592000";
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes me thinking:

  • we could explicitly say index.html wants no cache
  • everything in assets/* can be cached basically forever instead of "only" 30 days, those assets have cache-busting names
  • other assets (the ones from the public/ dir) we have to watch out because their name always stay the same. It's easy to update a file and forget that it's cached for a long time.

Copy link
Collaborator Author

@lebaudantoine lebaudantoine Jul 15, 2024

Choose a reason for hiding this comment

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

I have edited my commits to follow your recommendation. I have set no cache for the public/. I'll watch Nginx webinar on caching statics to level up my skills.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Inspecting the network tab, cache headers on response seems to be properly set. However, I was wondering why the request has a no-cache header.
Capture d’écran 2024-07-15 à 16 04 38

Configured Nginx to serve index.html for all requests, allowing
the client-side router (Wouter) to manage the routing.

Added a try_files directive to attempt to serve static files first,
falling back to index.html if the requested file is not found.

Added an error_page directive to handle 404 errors by internally
redirecting to index.html without modifying the URL path.

Wouter should make the rest.
Configured Nginx to set caching headers for static assets by adding
a location block to match common static file extensions and set
an expiration time of 30 days.

It should result in faster loading times, reduced bandwidth usage,
and a more efficient and responsive user experience.

Wdyt @manuhabitela?
ChangeLog won't be any useful before the first release.
Save us time, save the world useless computation, remove the CI steps.

They'll be added back as soon as they are necessary.
@lebaudantoine lebaudantoine merged commit 32dc582 into main Jul 15, 2024
6 of 8 checks passed
@lebaudantoine lebaudantoine deleted the fix/nginx-conf branch July 15, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Error 404 on staging
2 participants