Improvements for preview server configuration

[easing]

Changes

• Return HTTP 404 if requested path is not in apps routes. Fixes #137
• Add 404.html and 500.html for custom Nginx error pages
• Enable public assets cache for 1 year
• Enable gzip compression (level 1) (initial transfer size reduced from 251Kb to 82Kb)

---

Generating Nginx locations from router definition can be flaky for complex routes, so I've slightly adapt you suggestion of how it can be done and decide to export router RegExps and use NJS for requests validation.

You wrote in #137:

In another hand, I do not want to always return app’s HTML. Server should return proper HTTP status 404 on unknown page request.

According to web design basics, it's a good practice to have non-default 404 page.

Nginx now returns index.html with 404 status if route is not supported by app and not-found.svelte template will be rendered. We can use static 404.html instead, but from my point of view handling not founds in app is easier for developers and gives smoother user experience.

Small note

At debugging I discovered that nanostores/router creates too wide RegExps for patterns like /feeds/add/:url? causing path /feeds/add-random-bug to be treated as 200 instead of 404, but it's mimics application behaviour (feed adding form will be shown at this path) and seems to be 'technically' correct.

Checklist

• [x] Don’t rush. Check all changes in PR again.
• [x] Run pnpm test.
• [x] Think about changing documentation.
  • If you added a script to scripts/, add a comment with a description.
  • If you added a new folder, add its description to the project’s README.md.
  • If you added config, describe how we use this tool in the config’s comment.
  • If you added something to the project’s architecture, describe it in the project’s README.md.
  • Try to focus on “why?”, not “how?”.
• [x] If you added a new dependency, check our requirements.
• [x] Think about testing
  • If you added a feature, add unit tests.
  • If you added a new state to the UI, add visual tests.
  • If you fixed the bug, think about preventing bug regression in the future.
• If you changed web client:
  • [x] Think about moving code to core/. What code will also be useful on other platforms?
  • [x] Run pnpm size and check the difference in the JS bundle size. Is it relevant to the changes? Change the limit in web/.size-limit.json if necessary.
  • [x] Think about keyboard UX. Is it easy to use the new feature with only one hand on a keyboard? Is it easy to understand what keys to press?
  • [x] Think about HTML semantics.
  • [x] Think about accessibility. Check a11y recommendations. Think about how screen reader users will use the tool. Is it easy to use on a screen with bad contrast?
  • [x] Think about translations. Will
  • [x] Think about right-to-left languages. What parts of the screen should be mirrored for Arabic or Hebrew languages?
• If you changed the colors token in the web client:
  • [x] Think about app loading styles inlined in index.html.
• If you changed core/:
  • [x] Think about making types more precise. Can you better explain data relations by type?
  • [x] Think about conflict resolution. We don’t need some very smart changing merging; just 2 changes of the same item on different clients should not break the database. What if the user changes an item on one machine and removes it on another?
  • [x] Think about log and storage migration.
• If you changed English translations:
  • [x] Change translation ID if you change the meaning of the text.

[ai]

We can use static 404.html instead

Yes, let’s create 404.html and 500.html with just 404 and 500 text (we will add content later)

At debugging I discovered that nanostores/router creates too wide RegExps for patterns like /feeds/add/:url? causing path /feeds/add-random-bug

Wow, it is a bug. Can you send PR to nanostores/router if it is easy to fix?

I've slightly adapt you suggestion of how it can be done and decide to export router RegExps and use NJS for requests validation.

Do we call JS on /assets/* requests?

It is not the best performance way, honestly. But I am not 100% against it since we may move this JS check to Node.js server later, when we will add sync server.

But what was a rational reason of not having simple :param, :param? convert? What is the complexity of that way, can you explain with example?

[easing]

Wow, it is a bug. Can you send PR to nanostores/router if it is easy to fix?

Surely, I'll try to fix it.

Do we call JS on /assets/* requests?

Nginx will try to serve static first and only when no file at requested path was found it will pass request to @js location. So JS will called only for missed assets. But we can define location /assets/ {...} and avoid JS check for /assets at all. Also caching for assets can be added this way. Should we do it in this PR?

location /assets {
  expires 1y;
  add_header Cache-Control public;
}

---

But what was a rational reason of not having simple :param, :param? convert? What is the complexity of that way, can you explain with example?

:param substitution should work with current route set, but nanostores/router allow us to define route with RegExp and callback (without path pattern). So I prefer not to go this way, because it limits us on how routes can be defined in application.

Also, when creating location RegExp with :param convertation, we should handle cases like | in path pattern and duplicate part of router logic, so our locations will be out of sync with real router regexes if something will change in nanostores/router.

I thinked about dumping regexes from router to Nginx config locations, but for /^\/path\/(.+)$/.source JS returns ^\\/path\\/(.+)$ string which will not work in Nginx as we want. Removing \\ or replacing it with single \ is not clear in JS and smells a little bad. And even it can do the job with current routes, there are no garantees that code will work in future when new patterns will be added.

So I choose a more strainforward way to just run checks in JavaScript against the real router rules.

PS: Not important in this case, but Nginx locations order matters and I argue to avoid Nginx locations generation if it's possible.

[ai]

Should we do it in this PR?

Nice idea. But also add ui/assets

[ai]

but nanostores/router allow us to define route with RegExp and callback (without path pattern)

We are not planning to use this feature, so it is OK

So I choose a more strainforward way to just run checks in JavaScript against the real router rules.

Sure, let’s keep JS way.

[ai]

Hm, preview server doesn’t work for some reason

https://preview-154---staging-3ryqvfpd5q-ew.a.run.app/welcome

[easing]

Hm, preview server doesn’t work for some reason

Looks like wrong (default or maybe old) version of Nginx config is used for preview: our custom 404.html is on server but standard error pages rendered instead of it. https://preview-154---staging-3ryqvfpd5q-ew.a.run.app/404.html

Can you check /etc/nginx/nginx.conf in running container? Does it include recent changes?

Or maybe there is a way for me to pull this preview-154 docker image for investigation?

[ai]

Looks like wrong (default or maybe old) version of Nginx config is used for preview

Yes, it is my mistake. We deploy from main branch by sending artifacts of web/dist/, but not Dockerfile and routes.

I will fix it separately (need to think about security).

In this case I am waiting for cache changes, and we are ready to merge.

Really smart way to deal with the case.

[easing]

Really smart way to deal with the case.

Glad to hear such comment to my first commit to open source. Thanks.

I've add caching for /assets and /ui/assets and also enable gzip with minimal compression level. Initial transfer reduced from 251Kb to 82Kb.

With max compression we can save additional 12Kb but CPU will be significantly loaded because responses are compressed on the fly. For max performance without CPU load we should create .gz versions of assets on build phase but maybe 12Kb doesn't worth it.

[easing]

I've update PR title and description to reflect all changes that has been done

[ai]

Thanks. Amazing work.