From 4e830762b379570fb161c6cd4d3cb00b8ecf4c1c Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Thu, 10 Oct 2024 11:12:40 -0400 Subject: [PATCH 1/4] fix: standardize 404 and 500 matching in isRoute404 and isRoute500 functions --- packages/astro/src/core/util.ts | 10 ++++++++++ packages/astro/src/i18n/index.ts | 4 +++- packages/astro/src/vite-plugin-astro-server/route.ts | 7 +++---- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index a6a890137cc9..f196ed428af9 100644 --- a/packages/astro/src/core/util.ts +++ b/packages/astro/src/core/util.ts @@ -33,6 +33,16 @@ export function padMultilineString(source: string, n = 2) { return lines.map((l) => ` `.repeat(n) + l).join(`\n`); } +export function isRoute404(route: string) { + const route404Pattern = /^\/404\/?$/; + return route404Pattern.test(route); +} + +export function isRoute500(route: string) { + const route500Pattern = /^\/500\/?$/; + return route500Pattern.test(route); +} + const STATUS_CODE_PAGES = new Set(['/404', '/500']); /** diff --git a/packages/astro/src/i18n/index.ts b/packages/astro/src/i18n/index.ts index 2e8da37c6a3e..b165cced56bc 100644 --- a/packages/astro/src/i18n/index.ts +++ b/packages/astro/src/i18n/index.ts @@ -10,6 +10,7 @@ import { shouldAppendForwardSlash } from '../core/build/util.js'; import { REROUTE_DIRECTIVE_HEADER } from '../core/constants.js'; import { MissingLocale, i18nNoLocaleFoundInPath } from '../core/errors/errors-data.js'; import { AstroError } from '../core/errors/index.js'; +import { isRoute404, isRoute500 } from '../core/util.js'; import { createI18nMiddleware } from './middleware.js'; import type { RoutingStrategies } from './utils.js'; @@ -21,8 +22,9 @@ export function requestHasLocale(locales: Locales) { export function requestIs404Or500(request: Request, base = '') { const url = new URL(request.url); + const pathname = url.pathname.slice(base.length); - return url.pathname.startsWith(`${base}/404`) || url.pathname.startsWith(`${base}/500`); + return isRoute404(pathname) || isRoute500(pathname); } // Checks if the pathname has any locale diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 30a6eda0190e..d18a05181293 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -13,6 +13,7 @@ import { RenderContext } from '../core/render-context.js'; import { type SSROptions, getProps } from '../core/render/index.js'; import { createRequest } from '../core/request.js'; import { matchAllRoutes } from '../core/routing/index.js'; +import { isRoute404, isRoute500 } from '../core/util.js'; import { getSortedPreloadedMatches } from '../prerender/routing.js'; import type { DevPipeline } from './pipeline.js'; import { writeSSRResult, writeWebResponse } from './response.js'; @@ -36,13 +37,11 @@ function isLoggedRequest(url: string) { } function getCustom404Route(manifestData: ManifestData): RouteData | undefined { - const route404 = /^\/404\/?$/; - return manifestData.routes.find((r) => route404.test(r.route)); + return manifestData.routes.find((r) => isRoute404(r.route)); } function getCustom500Route(manifestData: ManifestData): RouteData | undefined { - const route500 = /^\/500\/?$/; - return manifestData.routes.find((r) => route500.test(r.route)); + return manifestData.routes.find((r) => isRoute500(r.route)); } export async function matchRoute( From 1313505a4efa61053dd68dcab3fad0c515dc78ae Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Thu, 10 Oct 2024 11:14:25 -0400 Subject: [PATCH 2/4] chore: changeset --- .changeset/purple-swans-argue.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/purple-swans-argue.md diff --git a/.changeset/purple-swans-argue.md b/.changeset/purple-swans-argue.md new file mode 100644 index 000000000000..fdfde42ef98a --- /dev/null +++ b/.changeset/purple-swans-argue.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Standardize 404 and 500 route matching via `isRoute404` and `isRoute500` functions From 977212ba9b2f7864a6d187f82689fd887d1e5110 Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Thu, 9 Jan 2025 16:53:57 +0100 Subject: [PATCH 3/4] feat: apply feedback --- .changeset/purple-swans-argue.md | 2 +- packages/astro/src/core/routing/match.ts | 12 +++++++++++- packages/astro/src/core/util.ts | 10 ---------- packages/astro/src/i18n/index.ts | 2 +- packages/astro/src/vite-plugin-astro-server/route.ts | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.changeset/purple-swans-argue.md b/.changeset/purple-swans-argue.md index fdfde42ef98a..d96a2fbd8e10 100644 --- a/.changeset/purple-swans-argue.md +++ b/.changeset/purple-swans-argue.md @@ -2,4 +2,4 @@ 'astro': patch --- -Standardize 404 and 500 route matching via `isRoute404` and `isRoute500` functions +Improves matching of 404 and 500 routes diff --git a/packages/astro/src/core/routing/match.ts b/packages/astro/src/core/routing/match.ts index 0e8a9e3387e4..f909233826de 100644 --- a/packages/astro/src/core/routing/match.ts +++ b/packages/astro/src/core/routing/match.ts @@ -17,6 +17,16 @@ export function matchAllRoutes(pathname: string, manifest: ManifestData): RouteD return manifest.routes.filter((route) => route.pattern.test(decodeURI(pathname))); } +export function isRoute404(route: string) { + const route404Pattern = /^\/404\/?$/; + return route404Pattern.test(route); +} + +export function isRoute500(route: string) { + const route500Pattern = /^\/500\/?$/; + return route500Pattern.test(route); +} + /** * Determines if the given route matches a 404 or 500 error page. * @@ -24,5 +34,5 @@ export function matchAllRoutes(pathname: string, manifest: ManifestData): RouteD * @returns {boolean} `true` if the route matches a 404 or 500 error page, otherwise `false`. */ export function isRoute404or500(route: RouteData): boolean { - return route.pattern.test('/404') || route.pattern.test('/500'); + return isRoute404(route.route) || isRoute500(route.route); } diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index d6569a120a53..1bd53d779b1a 100644 --- a/packages/astro/src/core/util.ts +++ b/packages/astro/src/core/util.ts @@ -36,16 +36,6 @@ export function padMultilineString(source: string, n = 2) { return lines.map((l) => ` `.repeat(n) + l).join(`\n`); } -export function isRoute404(route: string) { - const route404Pattern = /^\/404\/?$/; - return route404Pattern.test(route); -} - -export function isRoute500(route: string) { - const route500Pattern = /^\/500\/?$/; - return route500Pattern.test(route); -} - const STATUS_CODE_PAGES = new Set(['/404', '/500']); /** diff --git a/packages/astro/src/i18n/index.ts b/packages/astro/src/i18n/index.ts index 1731f64caa82..59da491a1d1f 100644 --- a/packages/astro/src/i18n/index.ts +++ b/packages/astro/src/i18n/index.ts @@ -4,7 +4,7 @@ import { shouldAppendForwardSlash } from '../core/build/util.js'; import { REROUTE_DIRECTIVE_HEADER } from '../core/constants.js'; import { MissingLocale, i18nNoLocaleFoundInPath } from '../core/errors/errors-data.js'; import { AstroError } from '../core/errors/index.js'; -import { isRoute404, isRoute500 } from '../core/util.js'; +import { isRoute404, isRoute500 } from '../core/routing/match.js'; import type { AstroConfig, Locales, ValidRedirectStatus } from '../types/public/config.js'; import type { APIContext } from '../types/public/context.js'; import { createI18nMiddleware } from './middleware.js'; diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index ec588213b036..51b7537bfc4a 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -15,7 +15,7 @@ import { getProps } from '../core/render/index.js'; import { createRequest } from '../core/request.js'; import { redirectTemplate } from '../core/routing/3xx.js'; import { matchAllRoutes } from '../core/routing/index.js'; -import { isRoute404, isRoute500 } from '../core/util.js'; +import { isRoute404, isRoute500 } from '../core/routing/match.js'; import { PERSIST_SYMBOL } from '../core/session.js'; import { getSortedPreloadedMatches } from '../prerender/routing.js'; import type { ComponentInstance, ManifestData } from '../types/astro.js'; From d7a83a848538806481069fbc5a1edabd3d415276 Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Thu, 9 Jan 2025 16:55:33 +0100 Subject: [PATCH 4/4] feat: extract regex --- packages/astro/src/core/routing/match.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/astro/src/core/routing/match.ts b/packages/astro/src/core/routing/match.ts index f909233826de..d8ead2f0703b 100644 --- a/packages/astro/src/core/routing/match.ts +++ b/packages/astro/src/core/routing/match.ts @@ -17,14 +17,15 @@ export function matchAllRoutes(pathname: string, manifest: ManifestData): RouteD return manifest.routes.filter((route) => route.pattern.test(decodeURI(pathname))); } +const ROUTE404_RE = /^\/404\/?$/; +const ROUTE500_RE = /^\/500\/?$/; + export function isRoute404(route: string) { - const route404Pattern = /^\/404\/?$/; - return route404Pattern.test(route); + return ROUTE404_RE.test(route); } export function isRoute500(route: string) { - const route500Pattern = /^\/500\/?$/; - return route500Pattern.test(route); + return ROUTE500_RE.test(route); } /**