From cb4d07819f0dbdfd94bc4f084edf7720ada01323 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Tue, 18 Jun 2024 16:04:59 +0100 Subject: [PATCH] fix(astro): don't run middlewarein dev for prerendered 404 (#11273) --- .changeset/silly-pens-fetch.md | 7 + .../src/vite-plugin-astro-server/route.ts | 152 ++++++------------ .../i18n-routing-manual/src/middleware.js | 2 +- .../i18n-routing-manual/src/pages/404.astro | 12 ++ .../src/pages/integration-post.astro | 13 ++ .../src/pages/integration-pre.astro | 13 ++ .../astro/test/i18n-routing-manual.test.js | 5 + 7 files changed, 97 insertions(+), 107 deletions(-) create mode 100644 .changeset/silly-pens-fetch.md create mode 100644 packages/astro/test/fixtures/i18n-routing-manual/src/pages/404.astro create mode 100644 packages/astro/test/fixtures/middleware space/src/pages/integration-post.astro create mode 100644 packages/astro/test/fixtures/middleware space/src/pages/integration-pre.astro diff --git a/.changeset/silly-pens-fetch.md b/.changeset/silly-pens-fetch.md new file mode 100644 index 0000000000..ff336a7990 --- /dev/null +++ b/.changeset/silly-pens-fetch.md @@ -0,0 +1,7 @@ +--- +'astro': patch +--- + +Corrects an inconsistency in dev where middleware would run for prerendered 404 routes. +Middleware is not run for prerendered 404 routes in production, so this was incorrect. + diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index b054123143..0d5f5d0d76 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -162,11 +162,10 @@ export async function handleRoute({ }: HandleRoute): Promise { const timeStart = performance.now(); const { config, loader, logger } = pipeline; - if (!matchedRoute && !config.i18n) { - if (isLoggedRequest(pathname)) { - logger.info(null, req({ url: pathname, method: incomingRequest.method, statusCode: 404 })); - } - return handle404Response(origin, incomingRequest, incomingResponse); + + if (!matchedRoute) { + // This should never happen, because ensure404Route will add a 404 route if none exists. + throw new Error('No route matched, and default 404 route was not found.'); } let request: Request; @@ -177,107 +176,48 @@ export async function handleRoute({ const middleware = (await loadMiddleware(loader)).onRequest; const locals = Reflect.get(incomingRequest, clientLocalsSymbol); - if (!matchedRoute) { - if (config.i18n) { - const locales = config.i18n.locales; - const pathNameHasLocale = pathname - .split('/') - .filter(Boolean) - .some((segment) => { - let found = false; - for (const locale of locales) { - if (typeof locale === 'string') { - if (normalizeTheLocale(locale) === normalizeTheLocale(segment)) { - found = true; - break; - } - } else { - if (locale.path === segment) { - found = true; - break; - } - } - } - return found; - }); - // Even when we have `config.base`, the pathname is still `/` because it gets stripped before - if (!pathNameHasLocale && pathname !== '/') { - return handle404Response(origin, incomingRequest, incomingResponse); - } - } - request = createRequest({ - base: config.base, - url, - headers: incomingRequest.headers, - logger, - // no route found, so we assume the default for rendering the 404 page - staticLike: config.output === 'static' || config.output === 'hybrid', - }); - route = { - component: '', - generate(_data: any): string { - return ''; - }, - params: [], - // Disable eslint as we only want to generate an empty RegExp - // eslint-disable-next-line prefer-regex-literals - pattern: new RegExp(''), - prerender: false, - segments: [], - type: 'fallback', - route: '', - fallbackRoutes: [], - isIndex: false, - }; + const filePath: URL | undefined = matchedRoute.filePath; + const { preloadedComponent } = matchedRoute; + route = matchedRoute.route; + // Allows adapters to pass in locals in dev mode. + request = createRequest({ + base: config.base, + url, + headers: incomingRequest.headers, + method: incomingRequest.method, + body, + logger, + clientAddress: incomingRequest.socket.remoteAddress, + staticLike: config.output === 'static' || route.prerender, + }); - renderContext = RenderContext.create({ - pipeline: pipeline, - pathname, - middleware, - request, - routeData: route, - }); - } else { - const filePath: URL | undefined = matchedRoute.filePath; - const { preloadedComponent } = matchedRoute; - route = matchedRoute.route; - // Allows adapters to pass in locals in dev mode. - request = createRequest({ - base: config.base, - url, - headers: incomingRequest.headers, - method: incomingRequest.method, - body, - logger, - clientAddress: incomingRequest.socket.remoteAddress, - staticLike: config.output === 'static' || route.prerender, - }); - - // Set user specified headers to response object. - for (const [name, value] of Object.entries(config.server.headers ?? {})) { - if (value) incomingResponse.setHeader(name, value); - } - - options = { - pipeline, - filePath, - preload: preloadedComponent, - pathname, - request, - route, - }; - - mod = preloadedComponent; - renderContext = RenderContext.create({ - locals, - pipeline, - pathname, - middleware, - request, - routeData: route, - }); + // Set user specified headers to response object. + for (const [name, value] of Object.entries(config.server.headers ?? {})) { + if (value) incomingResponse.setHeader(name, value); } + options = { + pipeline, + filePath, + preload: preloadedComponent, + pathname, + request, + route, + }; + + mod = preloadedComponent; + + const isPrerendered404 = matchedRoute.route.route === '/404' && matchedRoute.route.prerender; + + renderContext = RenderContext.create({ + locals, + pipeline, + pathname, + middleware: isPrerendered404 ? undefined : middleware, + request, + routeData: route, + }); + let response; try { response = await renderContext.render(mod); @@ -288,9 +228,9 @@ export async function handleRoute({ } // Log useful information that the custom 500 page may not display unlike the default error overlay logger.error('router', err.stack || err.message); - const filePath = new URL(`./${custom500.component}`, config.root); - const preloadedComponent = await pipeline.preload(custom500, filePath); - response = await renderContext.render(preloadedComponent); + const filePath500 = new URL(`./${custom500.component}`, config.root); + const preloaded500Component = await pipeline.preload(custom500, filePath500); + response = await renderContext.render(preloaded500Component); status = 500; } diff --git a/packages/astro/test/fixtures/i18n-routing-manual/src/middleware.js b/packages/astro/test/fixtures/i18n-routing-manual/src/middleware.js index 33b110b638..fc926e8bfc 100644 --- a/packages/astro/test/fixtures/i18n-routing-manual/src/middleware.js +++ b/packages/astro/test/fixtures/i18n-routing-manual/src/middleware.js @@ -11,7 +11,7 @@ export const onRequest = defineMiddleware(async (context, next) => { return await next(); } - if (context.url.pathname === '/') { + if (context.url.pathname === '/' || context.url.pathname === '/redirect-me') { return redirectToDefaultLocale(context); } return new Response(null, { diff --git a/packages/astro/test/fixtures/i18n-routing-manual/src/pages/404.astro b/packages/astro/test/fixtures/i18n-routing-manual/src/pages/404.astro new file mode 100644 index 0000000000..b0a1d22960 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-manual/src/pages/404.astro @@ -0,0 +1,12 @@ +--- +export const prerender = true +--- + + + + Astro + + +404 + + diff --git a/packages/astro/test/fixtures/middleware space/src/pages/integration-post.astro b/packages/astro/test/fixtures/middleware space/src/pages/integration-post.astro new file mode 100644 index 0000000000..c6edf9cd75 --- /dev/null +++ b/packages/astro/test/fixtures/middleware space/src/pages/integration-post.astro @@ -0,0 +1,13 @@ +--- +const data = Astro.locals; +--- + + + + Testing + + + +

{data?.name}

+ + diff --git a/packages/astro/test/fixtures/middleware space/src/pages/integration-pre.astro b/packages/astro/test/fixtures/middleware space/src/pages/integration-pre.astro new file mode 100644 index 0000000000..c6edf9cd75 --- /dev/null +++ b/packages/astro/test/fixtures/middleware space/src/pages/integration-pre.astro @@ -0,0 +1,13 @@ +--- +const data = Astro.locals; +--- + + + + Testing + + + +

{data?.name}

+ + diff --git a/packages/astro/test/i18n-routing-manual.test.js b/packages/astro/test/i18n-routing-manual.test.js index 1feaf96334..d0de75fe8d 100644 --- a/packages/astro/test/i18n-routing-manual.test.js +++ b/packages/astro/test/i18n-routing-manual.test.js @@ -52,6 +52,11 @@ describe('Dev server manual routing', () => { text = await response.text(); assert.equal(text.includes('Hola.'), true); }); + + it('should not redirect prerendered 404 routes in dev', async () => { + const response = await fixture.fetch('/redirect-me'); + assert.equal(response.status, 404); + }); }); // SSG