Infrastructure/astro
0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2025-03-24 23:21:57 -05:00
Code Issues Activity

fix(dev): consider pages' route priority when matching request to a page (#10582)

Browse source 
* fix(dev): consider pages' route priority when matching request to a page

* add test

* add changeset
This commit is contained in:
Arsh 2024-03-28 02:07:02 +05:30 • committed by GitHub
parent 7d3b61c56d
commit a05953538f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 137 additions and 108 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/wise-ears-flash.md Normal file
View file

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

2
packages/astro/src/core/routing/astro-designed-error-pages.ts
View file

@ -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: [],

106
packages/astro/src/core/routing/manifest/create.ts
View file

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

105
packages/astro/src/core/routing/priority.ts Normal file
View file

@ -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);
}

5
packages/astro/src/prerender/routing.ts
View file

@ -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 = {

1
packages/astro/src/vite-plugin-astro-server/route.ts
View file

@ -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);

3
packages/astro/test/fixtures/custom-404-loop-case-5/astro.config.mjs vendored Normal file
View file

@ -0,0 +1,3 @@
export default {
output: "server"
}

8
packages/astro/test/fixtures/custom-404-loop-case-5/package.json vendored Normal file
View file

@ -0,0 +1,8 @@
{
"name": "@test/custom-404-loop-case-5",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}

4
packages/astro/test/fixtures/custom-404-loop-case-5/src/pages/[x].astro vendored Normal file
View file

@ -0,0 +1,4 @@
---
Astro.response.status = 404
---
<p>four oh four route</p>

6
pnpm-lock.yaml generated
View file

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