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.

Sign up for GitHub

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(i18n): allow to create 404.html and 500.html #11062

Merged
merged 5 commits into from
May 16, 2024
Merged

fix(i18n): allow to create 404.html and 500.html #11062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ea2a19
fix(i18n): allow to create 404.html and 500.html
ematipico May 16, 2024
6dde091
Update packages/astro/src/i18n/index.ts
ematipico May 16, 2024
e98712d
Update .changeset/lazy-rockets-raise.md
florian-lefebvre May 16, 2024
94c4272
chore: use better matching
ematipico May 16, 2024
12f93b2
fix linting
ematipico May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lazy-rockets-raise.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fixes a bug where `astro build` didn't create custom `404.html` and `500.html` when a certain combination of i18n options was applied
10 changes: 9 additions & 1 deletion packages/astro/src/i18n/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ export function requestHasLocale(locales: Locales) {
};
}

export function requestIs404Or500(request: Request, base = '') {
const url = new URL(request.url);

return (
url.pathname.startsWith(`${base}/404`) ||
url.pathname.startsWith(`${base}/500`)
);
}
// Checks if the pathname has any locale
export function pathHasLocale(path: string, locales: Locales): boolean {
const segments = path.split('/');
Expand Down Expand Up @@ -317,7 +325,7 @@ export function notFound({ base, locales }: MiddlewarePayload) {
if (!(isRoot || pathHasLocale(url.pathname, locales))) {
if (response) {
response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
return new Response(null, {
return new Response(response.body, {
status: 404,
headers: response.headers,
});
Expand Down
6 changes: 6 additions & 0 deletions packages/astro/src/i18n/middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
redirectToDefaultLocale,
redirectToFallback,
requestHasLocale,
requestIs404Or500,
} from './index.js';

export function createI18nMiddleware(
Expand Down Expand Up @@ -69,6 +70,11 @@ export function createI18nMiddleware(
return response;
}

// 404 and 500 are **known** routes (users can have their custom pages), so we need to let them be
if (requestIs404Or500(context.request, base)) {
return response;
}

const { currentLocale } = context;

switch (i18n.strategy) {
Expand Down
4 changes: 4 additions & 0 deletions packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/500.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<html>
<head><title>Error</title></head>
<body>Unexpected error.</body>
</html>
5 changes: 3 additions & 2 deletions packages/astro/test/i18n-routing-manual-with-default-middleware.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('Dev server manual routing', () => {
const response = await fixture.fetch('/blog');
const text = await response.text();
assert.equal(response.status, 404);
assert.equal(text.includes('Blog should not render'), false);
assert.match(text, /Blog should not render/);
});

it('should return a 200 because the custom middleware allows it', async () => {
Expand Down Expand Up @@ -83,7 +83,8 @@ describe('SSR manual routing', () => {
let request = new Request('http://example.com/blog');
let response = await app.render(request);
assert.equal(response.status, 404);
assert.equal((await response.text()).includes('Blog should not render'), false);
const text = await response.text();
assert.match(text, /Blog should not render/);
});

it('should return a 200 because the custom middleware allows it', async () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/astro/test/i18n-routing.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,16 @@ describe('[SSG] i18n routing', () => {
assert.equal($('body').text().includes('Lo siento'), true);
});

it('should create a custom 404.html and 505.html', async () => {
let html = await fixture.readFile('/404.html');
let $ = cheerio.load(html);
assert.equal($('body').text().includes("Can't find the page you're looking for."), true);

html = await fixture.readFile('/500.html');
$ = cheerio.load(html);
assert.equal($('body').text().includes('Unexpected error.'), true);
});

it("should NOT render the default locale if there isn't a fallback and the route is missing", async () => {
try {
await fixture.readFile('/it/start/index.html');
Expand Down
Loading