diff --git a/.changeset/reduce-ssr-url-parsing.md b/.changeset/reduce-ssr-url-parsing.md new file mode 100644 index 000000000000..c8acfd601ba6 --- /dev/null +++ b/.changeset/reduce-ssr-url-parsing.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Reduces redundant per-request URL parsing and allocations in the core SSR render and match paths. The domain-based i18n `Host`-header probe now runs only when a `domains-*` strategy is configured, the trailing-slash handler reuses the URL already parsed by the render pipeline instead of re-parsing it, `getParams` avoids building intermediate arrays, the memory cache provider reuses the request URL it already parsed, and domain i18n memoizes its parsed lookup table. Internal performance change with no behavior difference. diff --git a/packages/astro/src/core/app/base.ts b/packages/astro/src/core/app/base.ts index 2bf4695bd954..c72ffc831057 100644 --- a/packages/astro/src/core/app/base.ts +++ b/packages/astro/src/core/app/base.ts @@ -4,7 +4,7 @@ import { removeTrailingForwardSlash, } from '@astrojs/internal-helpers/path'; import { matchPattern } from '@astrojs/internal-helpers/remote'; -import { computePathnameFromDomain } from '../i18n/domain.js'; +import { computePathnameFromDomain, isDomainI18nStrategy } from '../i18n/domain.js'; import { isLocalizedErrorRoute } from '../../i18n/error-routes.js'; import type { RoutesList } from '../../types/astro.js'; import type { RemotePattern, RouteData } from '../../types/public/index.js'; @@ -152,6 +152,14 @@ export abstract class BaseApp

{ */ #featureCheckDone = false; + /** + * True when the configured i18n strategy resolves the locale from the + * request's domain (`domains-*`). `render()` reads this to decide whether + * a request needs the Host-header-based pathname lookup; routing for every + * other app is driven by the URL pathname alone. + */ + #hasDomainI18nRouting: boolean; + get logger(): AstroLogger { return this.pipeline.logger; } @@ -166,6 +174,10 @@ export abstract class BaseApp

{ constructor(manifest: SSRManifest, streaming = true, ...args: any[]) { this.manifest = manifest; + // The `domains-*` strategies are the ones that derive the locale from + // the request's domain. `isDomainI18nStrategy` is the single source of + // truth, shared with `computePathnameFromDomain`. + this.#hasDomainI18nRouting = isDomainI18nStrategy(manifest.i18n?.strategy); this.baseWithoutTrailingSlash = removeTrailingForwardSlash(manifest.base); this.pipeline = this.createPipeline(streaming, manifest, ...args); // Share the pipeline's manifestData so both BaseApp and the pipeline @@ -301,7 +313,13 @@ export abstract class BaseApp

{ const url = new URL(request.url); // ignore requests matching public assets if (this.manifest.assets.has(url.pathname)) return undefined; - let pathname = this.computePathnameFromDomain(request); + // Only the `domains-*` i18n strategies derive the pathname from the + // request's Host header; every other app routes on the URL pathname, so + // the probe is skipped. + let pathname: string | undefined; + if (this.#hasDomainI18nRouting) { + pathname = this.computePathnameFromDomain(request); + } if (!pathname) { pathname = prependForwardSlash(this.removeBase(url.pathname)); } @@ -394,7 +412,7 @@ export abstract class BaseApp

{ // For domain-based i18n, match against the locale-prefixed pathname // derived from the Host header. FetchState recomputes this pathname // itself for param/locale resolution, so it isn't threaded through here. - if (!routeData) { + if (!routeData && this.#hasDomainI18nRouting) { const domainPathname = this.computePathnameFromDomain(request); if (domainPathname) { routeData = this.pipeline.matchRoute(this.safeDecodeURI(domainPathname)); diff --git a/packages/astro/src/core/cache/memory-provider.ts b/packages/astro/src/core/cache/memory-provider.ts index 7e8af26e71d1..20cde8479006 100644 --- a/packages/astro/src/core/cache/memory-provider.ts +++ b/packages/astro/src/core/cache/memory-provider.ts @@ -404,13 +404,14 @@ const memoryProvider = ((config): CacheProvider => { name: 'memory', async onRequest(context, next) { - const requestUrl = new URL(context.request.url); - // Only cache GET requests. if (context.request.method !== 'GET') { return next(); } + // Reuse the URL the cache handler already parsed instead of re-parsing. + const requestUrl = context.url; + const primaryKey = getCacheKey(requestUrl, queryConfig); // Build the full key including Vary'd header values if we know them diff --git a/packages/astro/src/core/fetch/fetch-state.ts b/packages/astro/src/core/fetch/fetch-state.ts index b0dc5b79163d..9aabb6955906 100644 --- a/packages/astro/src/core/fetch/fetch-state.ts +++ b/packages/astro/src/core/fetch/fetch-state.ts @@ -195,6 +195,15 @@ export class FetchState implements AstroFetchState { } /** Normalized URL for this request. */ url: URL; + /** + * The request URL's pathname and search as originally parsed, before + * `normalizeUrl` collapses duplicate slashes and decodes `pathname` in + * place. `TrailingSlashHandler` matches against these so it sees the raw + * path a redirect would correct (e.g. duplicate or missing trailing + * slashes). `#applyForwardedHeaders` keeps them in sync with `request`. + */ + rawPathname: string; + rawSearch: string; /** Client address for this request. */ clientAddress: string | undefined; /** Whether this is a partial render (container API). */ @@ -267,6 +276,11 @@ export class FetchState implements AstroFetchState { this.slots = undefined; // Parse the URL once and derive both pathname and url from it. const url = new URL(request.url); + // Record the raw pathname/search now, while `url` still holds the + // original path. `normalizeUrl` (below) rewrites `url.pathname` in + // place. TrailingSlashHandler matches redirects against this raw path. + this.rawPathname = url.pathname; + this.rawSearch = url.search; // For domain-based i18n routing, the locale prefix is derived from the // request's Host header rather than its URL. When a locale is detected, // the resulting pathname includes the prefix (e.g. /en/boats/1/foo) that @@ -1003,6 +1017,12 @@ export class FetchState implements AstroFetchState { if (app !== undefined) { Reflect.set(this.request, appSymbol, app); } + + // `this.request` now carries the rebuilt URL (`this.url`, including the + // forwarded host/proto/port), so re-sync the raw pathname/search to it; + // TrailingSlashHandler then matches against the request actually served. + this.rawPathname = this.url.pathname; + this.rawSearch = this.url.search; } /** diff --git a/packages/astro/src/core/i18n/domain.ts b/packages/astro/src/core/i18n/domain.ts index 4e871ec73133..6437e1638234 100644 --- a/packages/astro/src/core/i18n/domain.ts +++ b/packages/astro/src/core/i18n/domain.ts @@ -9,6 +9,48 @@ import { normalizeTheLocale } from '../../i18n/path.js'; import type { SSRManifest } from '../app/types.js'; import type { AstroLogger } from '../logger/core.js'; +/** + * The i18n strategies that derive the request's locale from its domain (the + * `Host` header) rather than the URL pathname. `BaseApp` gates its + * Host-header pathname probe on this predicate, and `computePathnameFromDomain` + * only acts for these strategies. Keeping both driven by this one function + * stops the two lists from drifting apart. + */ +export function isDomainI18nStrategy(strategy: string | undefined): boolean { + return ( + strategy === 'domains-prefix-always' || + strategy === 'domains-prefix-other-locales' || + strategy === 'domains-prefix-always-no-redirect' + ); +} + +/** + * Per-config cache of parsed `domainLookupTable` entries. The table comes from + * the (immutable) manifest, so its domain keys are parsed once and reused on + * every request instead of being re-parsed per request. Keyed by the table + * object itself so distinct apps don't share entries. + */ +const domainEntriesCache = new WeakMap< + object, + Array<{ host: string; protocol: string; locale: string }> +>(); + +function getDomainEntries( + domainLookupTable: Record, +): Array<{ host: string; protocol: string; locale: string }> { + let entries = domainEntriesCache.get(domainLookupTable); + if (!entries) { + entries = Object.entries(domainLookupTable).map(([domainKey, locale]) => { + // Safe because the protocol is forced via zod in the config; a throw + // here would mean a tampered manifest (caught by the caller). + const url = new URL(domainKey); + return { host: url.host, protocol: url.protocol, locale }; + }); + domainEntriesCache.set(domainLookupTable, entries); + } + return entries; +} + /** * For domain-based i18n routing strategies, derives the locale-prefixed * pathname from the request's `Host` header rather than its URL. For example, @@ -28,12 +70,7 @@ export function computePathnameFromDomain( ): string | undefined { let pathname: string | undefined = undefined; - if ( - i18n && - (i18n.strategy === 'domains-prefix-always' || - i18n.strategy === 'domains-prefix-other-locales' || - i18n.strategy === 'domains-prefix-always-no-redirect') - ) { + if (i18n && isDomainI18nStrategy(i18n.strategy)) { // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host let host = request.headers.get('X-Forwarded-Host'); // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto @@ -56,16 +93,9 @@ export function computePathnameFromDomain( try { let locale; const hostAsUrl = new URL(`${protocol}//${host}`); - for (const [domainKey, localeValue] of Object.entries(i18n.domainLookupTable)) { - // This operation should be safe because we force the protocol via zod inside the configuration - // If not, then it means that the manifest was tampered - const domainKeyAsUrl = new URL(domainKey); - - if ( - hostAsUrl.host === domainKeyAsUrl.host && - hostAsUrl.protocol === domainKeyAsUrl.protocol - ) { - locale = localeValue; + for (const entry of getDomainEntries(i18n.domainLookupTable)) { + if (hostAsUrl.host === entry.host && hostAsUrl.protocol === entry.protocol) { + locale = entry.locale; break; } } diff --git a/packages/astro/src/core/render/params-and-props.ts b/packages/astro/src/core/render/params-and-props.ts index 6c995083c497..0bd0ec444534 100644 --- a/packages/astro/src/core/render/params-and-props.ts +++ b/packages/astro/src/core/render/params-and-props.ts @@ -98,16 +98,28 @@ export function getParams(route: RouteData, pathname: string): Params { ? pathname.slice(0, -5) : pathname; - const allPatterns = [route, ...route.fallbackRoutes].map((r) => r.pattern); - const paramsMatch = allPatterns.map((pattern) => pattern.exec(path)).find((x) => x); + // Match the path against the route's own pattern, then each fallback route, + // taking the first that matches. `route.params` indexes into the capture + // groups of this match below. + let paramsMatch = route.pattern.exec(path); + if (!paramsMatch) { + for (const fallbackRoute of route.fallbackRoutes) { + paramsMatch = fallbackRoute.pattern.exec(path); + if (paramsMatch) break; + } + } if (!paramsMatch) return {}; + // Inside `forEach`'s callback TypeScript treats `paramsMatch` as possibly + // null again (it doesn't narrow `let` across a closure), so read the match + // through a `const`, which stays typed as non-null. + const matched = paramsMatch; const params: Params = {}; route.params.forEach((key, i) => { if (key.startsWith('...')) { - params[key.slice(3)] = paramsMatch[i + 1] ? paramsMatch[i + 1] : undefined; + params[key.slice(3)] = matched[i + 1] ? matched[i + 1] : undefined; } else { - params[key] = paramsMatch[i + 1]; + params[key] = matched[i + 1]; } }); return params; diff --git a/packages/astro/src/core/routing/trailing-slash-handler.ts b/packages/astro/src/core/routing/trailing-slash-handler.ts index f641c3ce931a..07039b7a408e 100644 --- a/packages/astro/src/core/routing/trailing-slash-handler.ts +++ b/packages/astro/src/core/routing/trailing-slash-handler.ts @@ -29,15 +29,16 @@ export class TrailingSlashHandler { * normalization, or `undefined` if no redirect is required. */ handle(state: FetchState): Response | undefined { - // Use a fresh URL parse from the raw request so we see the - // un-normalized pathname (e.g. duplicate slashes like `///`). - // state.url has already been normalized by the FetchState - // constructor, which would hide the redirect targets. - const url = new URL(state.request.url); - const redirect = this.#redirectTrailingSlash(url.pathname); + // Use the raw pathname/search captured by the FetchState constructor + // (before normalization) so we see the un-normalized pathname (e.g. + // duplicate slashes like `///`). state.url has already been normalized, + // which would hide the redirect targets. Reusing the captured values + // avoids re-parsing the request URL on every request. + const pathname = state.rawPathname; + const redirect = this.#redirectTrailingSlash(pathname); // Not a redirect. - if (redirect === url.pathname) { + if (redirect === pathname) { return undefined; } @@ -46,14 +47,14 @@ export class TrailingSlashHandler { const response = new Response( redirectTemplate({ status, - relativeLocation: url.pathname, + relativeLocation: pathname, absoluteLocation: redirect, from: state.request.url, }), { status, headers: { - location: redirect + url.search, + location: redirect + state.rawSearch, }, }, );