From cf0d8b08a0f16bba7310d1a92c82b5a276682e8c Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 26 Nov 2024 15:21:06 +0000 Subject: [PATCH] fix(i18n): render `404.astro` when i18n is enabled (#12525) Co-authored-by: Chris Swithinbank Co-authored-by: delucis <357379+delucis@users.noreply.github.com> Co-authored-by: bluwy <34116392+bluwy@users.noreply.github.com> --- .changeset/hot-dingos-dress.md | 5 +++ packages/astro/src/i18n/middleware.ts | 8 ++++- .../src/vite-plugin-astro-server/request.ts | 1 - .../src/vite-plugin-astro-server/route.ts | 32 ++++++------------- .../fixtures/i18n-routing/src/pages/404.astro | 1 + packages/astro/test/i18n-routing.test.js | 6 ++++ 6 files changed, 28 insertions(+), 25 deletions(-) create mode 100644 .changeset/hot-dingos-dress.md diff --git a/.changeset/hot-dingos-dress.md b/.changeset/hot-dingos-dress.md new file mode 100644 index 000000000000..8a7087643383 --- /dev/null +++ b/.changeset/hot-dingos-dress.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes an issue where with `i18n` enabled, Astro couldn't render the `404.astro` component for non-existent routes. diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts index 097332805759..f4649dce2d32 100644 --- a/packages/astro/src/i18n/middleware.ts +++ b/packages/astro/src/i18n/middleware.ts @@ -1,6 +1,6 @@ import type { APIContext, MiddlewareHandler, SSRManifest } from '../@types/astro.js'; import type { SSRManifestI18n } from '../core/app/types.js'; -import { ROUTE_TYPE_HEADER } from '../core/constants.js'; +import { REROUTE_DIRECTIVE_HEADER, ROUTE_TYPE_HEADER } from '../core/constants.js'; import { type MiddlewarePayload, normalizeTheLocale, @@ -65,6 +65,12 @@ export function createI18nMiddleware( return async (context, next) => { const response = await next(); const type = response.headers.get(ROUTE_TYPE_HEADER); + + // This is case where we are internally rendering a 404/500, so we need to bypass checks that were done already + const isReroute = response.headers.get(REROUTE_DIRECTIVE_HEADER); + if (isReroute === 'no' && typeof i18n.fallback === 'undefined') { + return response; + } // If the route we're processing is not a page, then we ignore it if (type !== 'page' && type !== 'fallback') { return response; diff --git a/packages/astro/src/vite-plugin-astro-server/request.ts b/packages/astro/src/vite-plugin-astro-server/request.ts index b231bfde35c6..d45cf8b55c2e 100644 --- a/packages/astro/src/vite-plugin-astro-server/request.ts +++ b/packages/astro/src/vite-plugin-astro-server/request.ts @@ -63,7 +63,6 @@ export async function handleRequest({ url, pathname: resolvedPathname, body, - origin, pipeline, manifestData, incomingRequest: incomingRequest, diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 8de52d158bb0..5e888ceaf38f 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -127,7 +127,6 @@ type HandleRoute = { url: URL; pathname: string; body: ArrayBuffer | undefined; - origin: string; manifestData: ManifestData; incomingRequest: http.IncomingMessage; incomingResponse: http.ServerResponse; @@ -139,7 +138,6 @@ export async function handleRoute({ url, pathname, body, - origin, pipeline, manifestData, incomingRequest, @@ -156,12 +154,10 @@ export async function handleRoute({ let request: Request; let renderContext: RenderContext; let mod: ComponentInstance | undefined = undefined; - let options: SSROptions | undefined = undefined; let route: RouteData; const middleware = (await loadMiddleware(loader)).onRequest; const locals = Reflect.get(incomingRequest, clientLocalsSymbol); - const filePath: URL | undefined = matchedRoute.filePath; const { preloadedComponent } = matchedRoute; route = matchedRoute.route; // Allows adapters to pass in locals in dev mode. @@ -181,15 +177,6 @@ export async function handleRoute({ if (value) incomingResponse.setHeader(name, value); } - options = { - pipeline, - filePath, - preload: preloadedComponent, - pathname, - request, - route, - }; - mod = preloadedComponent; renderContext = await RenderContext.create({ @@ -248,18 +235,17 @@ export async function handleRoute({ if (statusCode === 404 && response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no') { const fourOhFourRoute = await matchRoute('/404', manifestData, pipeline); - if (options && options.route !== fourOhFourRoute?.route) - return handleRoute({ - ...options, - matchedRoute: fourOhFourRoute, - url: new URL(pathname, url), - body, - origin, + if (fourOhFourRoute) { + renderContext = await RenderContext.create({ + locals, pipeline, - manifestData, - incomingRequest, - incomingResponse, + pathname, + middleware: isDefaultPrerendered404(fourOhFourRoute.route) ? undefined : middleware, + request, + routeData: fourOhFourRoute.route, }); + response = await renderContext.render(fourOhFourRoute.preloadedComponent); + } } // We remove the internally-used header before we send the response to the user agent. diff --git a/packages/astro/test/fixtures/i18n-routing/src/pages/404.astro b/packages/astro/test/fixtures/i18n-routing/src/pages/404.astro index fce4a30b836c..bfde753739fe 100644 --- a/packages/astro/test/fixtures/i18n-routing/src/pages/404.astro +++ b/packages/astro/test/fixtures/i18n-routing/src/pages/404.astro @@ -7,6 +7,7 @@ const currentLocale = Astro.currentLocale;

404 - Not Found

+

Custom 404

Current Locale: {currentLocale ? currentLocale : "none"}

diff --git a/packages/astro/test/i18n-routing.test.js b/packages/astro/test/i18n-routing.test.js index 8e6c672bee28..b557038277ff 100644 --- a/packages/astro/test/i18n-routing.test.js +++ b/packages/astro/test/i18n-routing.test.js @@ -83,6 +83,12 @@ describe('[DEV] i18n routing', () => { assert.equal((await response.text()).includes('Endurance'), true); }); + it('should render the 404.astro file', async () => { + const response = await fixture.fetch('/do-not-exist'); + assert.equal(response.status, 404); + assert.match(await response.text(), /Custom 404/); + }); + it('should return the correct locale on 404 page for non existing default locale page', async () => { const response = await fixture.fetch('/es/nonexistent-page'); assert.equal(response.status, 404);