diff options
| -rw-r--r-- | .changeset/wise-ears-flash.md | 5 | ||||
| -rw-r--r-- | packages/astro/src/core/routing/astro-designed-error-pages.ts | 2 | ||||
| -rw-r--r-- | packages/astro/src/core/routing/manifest/create.ts | 106 | ||||
| -rw-r--r-- | packages/astro/src/core/routing/priority.ts | 105 | ||||
| -rw-r--r-- | packages/astro/src/prerender/routing.ts | 5 | ||||
| -rw-r--r-- | packages/astro/src/vite-plugin-astro-server/route.ts | 1 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/custom-404-loop-case-5/astro.config.mjs | 3 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/custom-404-loop-case-5/package.json | 8 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/custom-404-loop-case-5/src/pages/[x].astro | 4 | ||||
| -rw-r--r-- | pnpm-lock.yaml | 6 |
10 files changed, 137 insertions, 108 deletions
diff --git a/.changeset/wise-ears-flash.md b/.changeset/wise-ears-flash.md new file mode 100644 index 000000000..8446fac33 --- /dev/null +++ b/.changeset/wise-ears-flash.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes an issue where the dev server got stuck in a loop while routing responses with a 404 status code to the 404 route. diff --git a/packages/astro/src/core/routing/astro-designed-error-pages.ts b/packages/astro/src/core/routing/astro-designed-error-pages.ts index e3f2fd553..f4996d223 100644 --- a/packages/astro/src/core/routing/astro-designed-error-pages.ts +++ b/packages/astro/src/core/routing/astro-designed-error-pages.ts @@ -9,7 +9,7 @@ export function ensure404Route(manifest: ManifestData) { params: [], pattern: /\/404/, prerender: false, - segments: [], + segments: [[{ content: '404', dynamic: false,spread: false }]], type: 'page', route: '/404', fallbackRoutes: [], diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index e30440fa4..ea1f00e93 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -21,6 +21,7 @@ import { AstroError } from '../../errors/index.js'; import { removeLeadingForwardSlash, slash } from '../../path.js'; import { resolvePages } from '../../util.js'; import { getRouteGenerator } from './generator.js'; +import { routeComparator } from '../priority.js'; const require = createRequire(import.meta.url); interface Item { @@ -174,111 +175,6 @@ function isSemanticallyEqualSegment(segmentA: RoutePart[], segmentB: RoutePart[] return true; } -/** - * Comparator for sorting routes in resolution order. - * - * The routes are sorted in by the following rules in order, following the first rule that - * applies: - * - More specific routes are sorted before less specific routes. Here, "specific" means - * the number of segments in the route, so a parent route is always sorted after its children. - * For example, `/foo/bar` is sorted before `/foo`. - * Index routes, originating from a file named `index.astro`, are considered to have one more - * segment than the URL they represent. - * - Static routes are sorted before dynamic routes. - * For example, `/foo/bar` is sorted before `/foo/[bar]`. - * - Dynamic routes with single parameters are sorted before dynamic routes with rest parameters. - * For example, `/foo/[bar]` is sorted before `/foo/[...bar]`. - * - Prerendered routes are sorted before non-prerendered routes. - * - Endpoints are sorted before pages. - * For example, a file `/foo.ts` is sorted before `/bar.astro`. - * - If both routes are equal regarding all previosu conditions, they are sorted alphabetically. - * For example, `/bar` is sorted before `/foo`. - * The definition of "alphabetically" is dependent on the default locale of the running system. - */ - -function routeComparator(a: RouteData, b: RouteData) { - const commonLength = Math.min(a.segments.length, b.segments.length); - - for (let index = 0; index < commonLength; index++) { - const aSegment = a.segments[index]; - const bSegment = b.segments[index]; - - const aIsStatic = aSegment.every((part) => !part.dynamic && !part.spread); - const bIsStatic = bSegment.every((part) => !part.dynamic && !part.spread); - - if (aIsStatic && bIsStatic) { - // Both segments are static, they are sorted alphabetically if they are different - const aContent = aSegment.map((part) => part.content).join(''); - const bContent = bSegment.map((part) => part.content).join(''); - - if (aContent !== bContent) { - return aContent.localeCompare(bContent); - } - } - - // Sort static routes before dynamic routes - if (aIsStatic !== bIsStatic) { - return aIsStatic ? -1 : 1; - } - - const aAllDynamic = aSegment.every((part) => part.dynamic); - const bAllDynamic = bSegment.every((part) => part.dynamic); - - // Some route might have partial dynamic segments, e.g. game-[title].astro - // These routes should have higher priority against route that have **only** dynamic segments, e.g. [title].astro - if (aAllDynamic !== bAllDynamic) { - return aAllDynamic ? 1 : -1; - } - - const aHasSpread = aSegment.some((part) => part.spread); - const bHasSpread = bSegment.some((part) => part.spread); - - // Sort dynamic routes with rest parameters after dynamic routes with single parameters - // (also after static, but that is already covered by the previous condition) - if (aHasSpread !== bHasSpread) { - return aHasSpread ? 1 : -1; - } - } - - const aLength = a.segments.length; - const bLength = b.segments.length; - - if (aLength !== bLength) { - const aEndsInRest = a.segments.at(-1)?.some((part) => part.spread); - const bEndsInRest = b.segments.at(-1)?.some((part) => part.spread); - - if (aEndsInRest !== bEndsInRest && Math.abs(aLength - bLength) === 1) { - // If only one of the routes ends in a rest parameter - // and the difference in length is exactly 1 - // and the shorter route is the one that ends in a rest parameter - // the shorter route is considered more specific. - // I.e. `/foo` is considered more specific than `/foo/[...bar]` - if (aLength > bLength && aEndsInRest) { - // b: /foo - // a: /foo/[...bar] - return 1; - } - - if (bLength > aLength && bEndsInRest) { - // a: /foo - // b: /foo/[...bar] - return -1; - } - } - - // Sort routes by length - return aLength > bLength ? -1 : 1; - } - - // Sort endpoints before pages - if ((a.type === 'endpoint') !== (b.type === 'endpoint')) { - return a.type === 'endpoint' ? -1 : 1; - } - - // Both routes have segments with the same properties - return a.route.localeCompare(b.route); -} - export interface CreateRouteManifestParams { /** Astro Settings object */ settings: AstroSettings; diff --git a/packages/astro/src/core/routing/priority.ts b/packages/astro/src/core/routing/priority.ts new file mode 100644 index 000000000..e48c40508 --- /dev/null +++ b/packages/astro/src/core/routing/priority.ts @@ -0,0 +1,105 @@ +import type { RouteData } from "../../@types/astro.js"; + +/** + * Comparator for sorting routes in resolution order. + * + * The routes are sorted in by the following rules in order, following the first rule that + * applies: + * - More specific routes are sorted before less specific routes. Here, "specific" means + * the number of segments in the route, so a parent route is always sorted after its children. + * For example, `/foo/bar` is sorted before `/foo`. + * Index routes, originating from a file named `index.astro`, are considered to have one more + * segment than the URL they represent. + * - Static routes are sorted before dynamic routes. + * For example, `/foo/bar` is sorted before `/foo/[bar]`. + * - Dynamic routes with single parameters are sorted before dynamic routes with rest parameters. + * For example, `/foo/[bar]` is sorted before `/foo/[...bar]`. + * - Prerendered routes are sorted before non-prerendered routes. + * - Endpoints are sorted before pages. + * For example, a file `/foo.ts` is sorted before `/bar.astro`. + * - If both routes are equal regarding all previosu conditions, they are sorted alphabetically. + * For example, `/bar` is sorted before `/foo`. + * The definition of "alphabetically" is dependent on the default locale of the running system. + */ +export function routeComparator(a: RouteData, b: RouteData) { + const commonLength = Math.min(a.segments.length, b.segments.length); + + for (let index = 0; index < commonLength; index++) { + const aSegment = a.segments[index]; + const bSegment = b.segments[index]; + + const aIsStatic = aSegment.every((part) => !part.dynamic && !part.spread); + const bIsStatic = bSegment.every((part) => !part.dynamic && !part.spread); + + if (aIsStatic && bIsStatic) { + // Both segments are static, they are sorted alphabetically if they are different + const aContent = aSegment.map((part) => part.content).join(''); + const bContent = bSegment.map((part) => part.content).join(''); + + if (aContent !== bContent) { + return aContent.localeCompare(bContent); + } + } + + // Sort static routes before dynamic routes + if (aIsStatic !== bIsStatic) { + return aIsStatic ? -1 : 1; + } + + const aAllDynamic = aSegment.every((part) => part.dynamic); + const bAllDynamic = bSegment.every((part) => part.dynamic); + + // Some route might have partial dynamic segments, e.g. game-[title].astro + // These routes should have higher priority against route that have **only** dynamic segments, e.g. [title].astro + if (aAllDynamic !== bAllDynamic) { + return aAllDynamic ? 1 : -1; + } + + const aHasSpread = aSegment.some((part) => part.spread); + const bHasSpread = bSegment.some((part) => part.spread); + + // Sort dynamic routes with rest parameters after dynamic routes with single parameters + // (also after static, but that is already covered by the previous condition) + if (aHasSpread !== bHasSpread) { + return aHasSpread ? 1 : -1; + } + } + + const aLength = a.segments.length; + const bLength = b.segments.length; + + if (aLength !== bLength) { + const aEndsInRest = a.segments.at(-1)?.some((part) => part.spread); + const bEndsInRest = b.segments.at(-1)?.some((part) => part.spread); + + if (aEndsInRest !== bEndsInRest && Math.abs(aLength - bLength) === 1) { + // If only one of the routes ends in a rest parameter + // and the difference in length is exactly 1 + // and the shorter route is the one that ends in a rest parameter + // the shorter route is considered more specific. + // I.e. `/foo` is considered more specific than `/foo/[...bar]` + if (aLength > bLength && aEndsInRest) { + // b: /foo + // a: /foo/[...bar] + return 1; + } + + if (bLength > aLength && bEndsInRest) { + // a: /foo + // b: /foo/[...bar] + return -1; + } + } + + // Sort routes by length + return aLength > bLength ? -1 : 1; + } + + // Sort endpoints before pages + if ((a.type === 'endpoint') !== (b.type === 'endpoint')) { + return a.type === 'endpoint' ? -1 : 1; + } + + // Both routes have segments with the same properties + return a.route.localeCompare(b.route); +}
\ No newline at end of file diff --git a/packages/astro/src/prerender/routing.ts b/packages/astro/src/prerender/routing.ts index 8ecf20a5f..c1b1d5e8a 100644 --- a/packages/astro/src/prerender/routing.ts +++ b/packages/astro/src/prerender/routing.ts @@ -1,5 +1,6 @@ import type { AstroSettings, ComponentInstance, RouteData } from '../@types/astro.js'; import { RedirectComponentInstance, routeIsRedirect } from '../core/redirects/index.js'; +import { routeComparator } from '../core/routing/priority.js'; import type { DevPipeline } from '../vite-plugin-astro-server/pipeline.js'; import { getPrerenderStatus } from './metadata.js'; @@ -19,7 +20,9 @@ export async function getSortedPreloadedMatches({ matches, settings, }) - ).sort((a, b) => prioritizePrerenderedMatchesComparator(a.route, b.route)); + ) + .sort((a,b) => routeComparator(a.route, b.route)) + .sort((a, b) => prioritizePrerenderedMatchesComparator(a.route, b.route)) } type PreloadAndSetPrerenderStatusParams = { diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index f10d9e184..66e0b86c0 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -287,7 +287,6 @@ export async function handleRoute({ } if ( response.status === 404 && - has404Route(manifestData) && response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no' ) { const fourOhFourRoute = await matchRoute('/404', manifestData, pipeline); diff --git a/packages/astro/test/fixtures/custom-404-loop-case-5/astro.config.mjs b/packages/astro/test/fixtures/custom-404-loop-case-5/astro.config.mjs new file mode 100644 index 000000000..97e67349d --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-loop-case-5/astro.config.mjs @@ -0,0 +1,3 @@ +export default { + output: "server" +} diff --git a/packages/astro/test/fixtures/custom-404-loop-case-5/package.json b/packages/astro/test/fixtures/custom-404-loop-case-5/package.json new file mode 100644 index 000000000..1c0718c1d --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-loop-case-5/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/custom-404-loop-case-5", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/custom-404-loop-case-5/src/pages/[x].astro b/packages/astro/test/fixtures/custom-404-loop-case-5/src/pages/[x].astro new file mode 100644 index 000000000..ff217453d --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-loop-case-5/src/pages/[x].astro @@ -0,0 +1,4 @@ +--- +Astro.response.status = 404 +--- +<p>four oh four route</p>
\ No newline at end of file diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 57b8ccc4e..e939aac94 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2672,6 +2672,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/custom-404-loop-case-5: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/custom-404-md: dependencies: astro: |