search

withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
45.84k stars 2.41k forks source link

fix(astro): don't run middleware in dev for prerendered 404 routes #11273

Closed ascorbic closed 3 months ago

ascorbic commented 3 months ago

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 ensure404Route means 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?

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: 851b0faa40e0c716f87f149be6ff2b808cf8d002

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

matthewp commented 3 months ago

Do we not run middleware for output: 'static'? I assume that we did. If so then I think we should probably do so for prerendered routes. I definitely understand how this can be confusing, however. I'm curious if people are using middleware for static pages already, and if that will break some expectations they have.

ascorbic commented 3 months ago

@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.