fix(astro): don't run middleware in dev for prerendered 404 routes #11273
Conversation
🦋 Changeset detectedLatest commit: 851b0fa 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 |
github-actions
bot
added
the
pkg: astro
| const isPrerendered404 = matchedRoute.route.route === '/404' && matchedRoute.route.prerender; | ||
|
|
||
| renderContext = RenderContext.create({ | ||
| locals, | ||
| pipeline, | ||
| pathname, | ||
| middleware: isPrerendered404 ? undefined : middleware, | ||
| request, | ||
| routeData: route, | ||
| }); |
There was a problem hiding this comment.
This is the actual changed logic
| @@ -177,106 +176,47 @@ export async function handleRoute({ | |||
| const middleware = (await loadMiddleware(loader)).onRequest; | |||
| const locals = Reflect.get(incomingRequest, clientLocalsSymbol); | |||
|
|
|||
| if (!matchedRoute) { | |||
There was a problem hiding this comment.
matchedRoute is always truthy because of ensure404Route, so this branch is never hit
| 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. | ||
|
|
There was a problem hiding this comment.
| 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. | |
| Corrects an inconsistency in development where middlewares would run for prerendered 404 routes. It doesn't happen anymore to match production's behavior |
|
Do we not run middleware for |
|
@matthewp yeah, I was a bit unclear there. You're of course right that middleware is run for prerendered pages at build time. The stuff I'm talking about is specific to 404 routes. The issue with prerendered 404 pages is that when we prerender it there's no way of knowing what the actual route will be, so there's no way that middleware can be used for things like i18n redirects if it's prerendered. |
Changes
When a 404 page is prerendered, middleware is not run in production. This is because middleware is only run at build time for prerendered pages. Currently this behaves differently in dev, which was running middleware for all 404 requests. This is confusing, causing reports such as #10785
As part of the work making this change it became clear there was a lot of unused logic in the dev 404 handling. Currently it checks if there is no matching route to handle the 404 case. However there is always a matched route in practice, because
ensure404Routemeans a 404 route will always match it, so those code branches are never hit. This PR removes a lot of this unused code.Testing
Added new tests, and fixed bugs in other tests.
Docs
Just fixes a bug, though maybe there could be docs explaining that middleware doesn't run for prerendered 404s?