fix: i18n 404

[florian-lefebvre]

Changes

• Closes Custom 404 pages in localized sites #12175
• Having a 404.astro file in the root seems to be ignored atm. I think the fix is related to how we check if a route is a 404 or 500, because the pathname ends up being incorrect

Testing

Added tests

Docs

N/A

[changeset-bot]

🦋 Changeset detected

Latest commit: 7f2b88e

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ematipico]

I don't think this is the correct fix, and I don't think the test is accurate either. We have prefixDefaultLocale: true, so I would expect /404 to be prefixed, hence /en/404. However, I don't expect /404 to exist. Having both seems incorrect.

However, if you think this is acceptable, we need to update the docs. For example, pages/index.astro is required even when we have prefixDefaultLocale: true and redictToDefaultLocale: true. So we need to document that if users use 404.astro, Astro will create a 404.html under each locale.

Also, I would add more test cases, for example if prefixDefaultLocale: false, I wouldn't expect to have /en/404.html. Also, I would create a test case without fallback.

[florian-lefebvre]

Having both seems incorrect.

Why do you think that? If someone visits an invalid locale at the moment, they will see the host 404, not the user 404 no matter if it's set. So I think supporting this case is valid. But I'm not an i18n expert so let me know!

So we need to document that if users use 404.astro, Astro will create a 404.html under each locale.

This is not what this fix is about I believe, it's about supporting a custom root 404. That doesn't prevent users from having a custom 404 under a locale.

Also, I would add more test cases

Definitely agree, thanks for the pointers! I'm not super familiar with i18n in Astro so that helps a ton

[ematipico]

Why do you think that?

Because the PR description is empty, I find it difficult to understand what we're trying to fix 😅

If someone visits an invalid locale at the moment, they will see the host 404, not the user 404 no matter if it's set. So I think supporting this case is valid. But I'm not an i18n expert so let me know!

Since the host always renders /404.html, how can a developer tell the host to render /ru/404.html for an invalid URL under the /ru locale? (It's an example.)

I am not saying that the fix is incorrect, I am saying I am missing a lot of context from the PR. I believe the fix is correct, but I would add more tests to it, especially for existing fixtures where we don't use [lang] and we have fixed pages.