Refactor getParamsAndProps code (#7537)

packages/astro/src/core/render/context.ts

@@ -8,8 +8,8 @@ import type {
 	SSRResult,
 } from '../../@types/astro';
 import { AstroError, AstroErrorData } from '../errors/index.js';
-import { getParamsAndPropsOrThrow } from './core.js';
 import type { Environment } from './environment';
+import { getParamsAndProps } from './params-and-props.js';
 
 const clientLocalsSymbol = Symbol.for('astro.locals');
 
@@ -47,7 +47,7 @@ export async function createRenderContext(
 	const url = new URL(request.url);
 	const origin = options.origin ?? url.origin;
 	const pathname = options.pathname ?? url.pathname;
-	const [params, props] = await getParamsAndPropsOrThrow({
+	const [params, props] = await getParamsAndProps({
 		mod: options.mod as any,
 		route: options.route,
 		routeCache: options.env.routeCache,

packages/astro/src/core/render/core.ts

@@ -1,108 +1,10 @@
-import type { AstroCookies, ComponentInstance, Params, Props, RouteData } from '../../@types/astro';
+import type { AstroCookies, ComponentInstance } from '../../@types/astro';
 import { renderPage as runtimeRenderPage } from '../../runtime/server/index.js';
 import { attachToResponse } from '../cookies/index.js';
-import { AstroError, AstroErrorData } from '../errors/index.js';
-import type { LogOptions } from '../logger/core.js';
 import { redirectRouteGenerate, redirectRouteStatus, routeIsRedirect } from '../redirects/index.js';
-import { getParams } from '../routing/params.js';
 import type { RenderContext } from './context.js';
 import type { Environment } from './environment.js';
 import { createResult } from './result.js';
-import { callGetStaticPaths, findPathItemByKey, RouteCache } from './route-cache.js';
-
-interface GetParamsAndPropsOptions {
-	mod: ComponentInstance;
-	route?: RouteData | undefined;
-	routeCache: RouteCache;
-	pathname: string;
-	logging: LogOptions;
-	ssr: boolean;
-}
-
-export const enum GetParamsAndPropsError {
-	NoMatchingStaticPath,
-}
-
-/**
- * It retrieves `Params` and `Props`, or throws an error
- * if they are not correctly retrieved.
- */
-export async function getParamsAndPropsOrThrow(
-	options: GetParamsAndPropsOptions
-): Promise<[Params, Props]> {
-	let paramsAndPropsResp = await getParamsAndProps(options);
-	if (paramsAndPropsResp === GetParamsAndPropsError.NoMatchingStaticPath) {
-		throw new AstroError({
-			...AstroErrorData.NoMatchingStaticPathFound,
-			message: AstroErrorData.NoMatchingStaticPathFound.message(options.pathname),
-			hint: options.route?.component
-				? AstroErrorData.NoMatchingStaticPathFound.hint([options.route?.component])
-				: '',
-		});
-	}
-	return paramsAndPropsResp;
-}
-
-export async function getParamsAndProps(
-	opts: GetParamsAndPropsOptions
-): Promise<[Params, Props] | GetParamsAndPropsError> {
-	const { logging, mod, route, routeCache, pathname, ssr } = opts;
-	// Handle dynamic routes
-	let params: Params = {};
-	let pageProps: Props;
-	if (route && !route.pathname) {
-		if (route.params.length) {
-			// The RegExp pattern expects a decoded string, but the pathname is encoded
-			// when the URL contains non-English characters.
-			const paramsMatch = route.pattern.exec(decodeURIComponent(pathname));
-			if (paramsMatch) {
-				params = getParams(route.params)(paramsMatch);
-
-				// If we have an endpoint at `src/pages/api/[slug].ts` that's prerendered, and the `slug`
-				// is `undefined`, throw an error as we can't generate the `/api` file and `/api` directory
-				// at the same time. Using something like `[slug].json.ts` instead will work.
-				if (route.type === 'endpoint' && mod.getStaticPaths) {
-					const lastSegment = route.segments[route.segments.length - 1];
-					const paramValues = Object.values(params);
-					const lastParam = paramValues[paramValues.length - 1];
-					// Check last segment is solely `[slug]` or `[...slug]` case (dynamic). Make sure it's not
-					// `foo[slug].js` by checking segment length === 1. Also check here if that param is undefined.
-					if (lastSegment.length === 1 && lastSegment[0].dynamic && lastParam === undefined) {
-						throw new AstroError({
-							...AstroErrorData.PrerenderDynamicEndpointPathCollide,
-							message: AstroErrorData.PrerenderDynamicEndpointPathCollide.message(route.route),
-							hint: AstroErrorData.PrerenderDynamicEndpointPathCollide.hint(route.component),
-							location: {
-								file: route.component,
-							},
-						});
-					}
-				}
-			}
-		}
-		let routeCacheEntry = routeCache.get(route);
-		// During build, the route cache should already be populated.
-		// During development, the route cache is filled on-demand and may be empty.
-		// TODO(fks): Can we refactor getParamsAndProps() to receive routeCacheEntry
-		// as a prop, and not do a live lookup/populate inside this lower function call.
-		if (!routeCacheEntry) {
-			routeCacheEntry = await callGetStaticPaths({ mod, route, isValidate: true, logging, ssr });
-			routeCache.set(route, routeCacheEntry);
-		}
-		const matchedStaticPath = findPathItemByKey(routeCacheEntry.staticPaths, params, route);
-		if (!matchedStaticPath && (ssr ? route.prerender : true)) {
-			return GetParamsAndPropsError.NoMatchingStaticPath;
-		}
-		// Note: considered using Object.create(...) for performance
-		// Since this doesn't inherit an object's properties, this caused some odd user-facing behavior.
-		// Ex. console.log(Astro.props) -> {}, but console.log(Astro.props.property) -> 'expected value'
-		// Replaced with a simple spread as a compromise
-		pageProps = matchedStaticPath?.props ? { ...matchedStaticPath.props } : {};
-	} else {
-		pageProps = {};
-	}
-	return [params, pageProps];
-}
 
 export type RenderPage = {
 	mod: ComponentInstance;

packages/astro/src/core/render/environment.ts

@@ -10,9 +10,15 @@ import { RouteCache } from './route-cache.js';
  * Thus they can be created once and passed through to renderPage on each request.
  */
 export interface Environment {
+	/**
+	 * Used to provide better error messages for `Astro.clientAddress`
+	 */
 	adapterName?: string;
 	/** logging options */
 	logging: LogOptions;
+	/**
+	 * Used to support `Astro.__renderMarkdown` for legacy `<Markdown />` component
+	 */
 	markdown: MarkdownRenderingOptions;
 	/** "development" or "production" */
 	mode: RuntimeMode;
@@ -20,7 +26,13 @@ export interface Environment {
 	clientDirectives: Map<string, string>;
 	resolve: (s: string) => Promise<string>;
 	routeCache: RouteCache;
+	/**
+	 * Used for `Astro.site`
+	 */
 	site?: string;
+	/**
+	 * Value of Astro config's `output` option, true if "server" or "hybrid"
+	 */
 	ssr: boolean;
 	streaming: boolean;
 }

packages/astro/src/core/render/index.ts

@@ -1,11 +1,7 @@
 export { createRenderContext } from './context.js';
 export type { RenderContext } from './context.js';
-export {
+export { renderPage } from './core.js';
-	getParamsAndProps,
-	GetParamsAndPropsError,
-	getParamsAndPropsOrThrow,
-	renderPage,
-} from './core.js';
 export type { Environment } from './environment';
 export { createBasicEnvironment, createEnvironment } from './environment.js';
+export { getParamsAndProps } from './params-and-props.js';
 export { loadRenderer, loadRenderers } from './renderer.js';

packages/astro/src/core/render/params-and-props.ts

@@ -0,0 +1,92 @@
+import type { ComponentInstance, Params, Props, RouteData } from '../../@types/astro';
+import { AstroError, AstroErrorData } from '../errors/index.js';
+import type { LogOptions } from '../logger/core.js';
+import { getParams } from '../routing/params.js';
+import { callGetStaticPaths, findPathItemByKey, RouteCache } from './route-cache.js';
+
+interface GetParamsAndPropsOptions {
+	mod: ComponentInstance;
+	route?: RouteData | undefined;
+	routeCache: RouteCache;
+	pathname: string;
+	logging: LogOptions;
+	ssr: boolean;
+}
+
+export async function getParamsAndProps(opts: GetParamsAndPropsOptions): Promise<[Params, Props]> {
+	const { logging, mod, route, routeCache, pathname, ssr } = opts;
+
+	// If there's no route, or if there's a pathname (e.g. a static `src/pages/normal.astro` file),
+	// then we know for sure they don't have params and props, return a fallback value.
+	if (!route || route.pathname) {
+		return [{}, {}];
+	}
+
+	// This is a dynamic route, start getting the params
+	const params = getRouteParams(route, pathname) ?? {};
+
+	validatePrerenderEndpointCollision(route, mod, params);
+
+	let routeCacheEntry = routeCache.get(route);
+	// During build, the route cache should already be populated.
+	// During development, the route cache is filled on-demand and may be empty.
+	// TODO(fks): Can we refactor getParamsAndProps() to receive routeCacheEntry
+	// as a prop, and not do a live lookup/populate inside this lower function call.
+	if (!routeCacheEntry) {
+		routeCacheEntry = await callGetStaticPaths({ mod, route, isValidate: true, logging, ssr });
+		routeCache.set(route, routeCacheEntry);
+	}
+
+	const matchedStaticPath = findPathItemByKey(routeCacheEntry.staticPaths, params, route);
+	if (!matchedStaticPath && (ssr ? route.prerender : true)) {
+		throw new AstroError({
+			...AstroErrorData.NoMatchingStaticPathFound,
+			message: AstroErrorData.NoMatchingStaticPathFound.message(pathname),
+			hint: AstroErrorData.NoMatchingStaticPathFound.hint([route.component]),
+		});
+	}
+
+	const props: Props = matchedStaticPath?.props ? { ...matchedStaticPath.props } : {};
+
+	return [params, props];
+}
+
+function getRouteParams(route: RouteData, pathname: string): Params | undefined {
+	if (route.params.length) {
+		// The RegExp pattern expects a decoded string, but the pathname is encoded
+		// when the URL contains non-English characters.
+		const paramsMatch = route.pattern.exec(decodeURIComponent(pathname));
+		if (paramsMatch) {
+			return getParams(route.params)(paramsMatch);
+		}
+	}
+}
+
+/**
+ * If we have an endpoint at `src/pages/api/[slug].ts` that's prerendered, and the `slug`
+ * is `undefined`, throw an error as we can't generate the `/api` file and `/api` directory
+ * at the same time. Using something like `[slug].json.ts` instead will work.
+ */
+function validatePrerenderEndpointCollision(
+	route: RouteData,
+	mod: ComponentInstance,
+	params: Params
+) {
+	if (route.type === 'endpoint' && mod.getStaticPaths) {
+		const lastSegment = route.segments[route.segments.length - 1];
+		const paramValues = Object.values(params);
+		const lastParam = paramValues[paramValues.length - 1];
+		// Check last segment is solely `[slug]` or `[...slug]` case (dynamic). Make sure it's not
+		// `foo[slug].js` by checking segment length === 1. Also check here if that param is undefined.
+		if (lastSegment.length === 1 && lastSegment[0].dynamic && lastParam === undefined) {
+			throw new AstroError({
+				...AstroErrorData.PrerenderDynamicEndpointPathCollide,
+				message: AstroErrorData.PrerenderDynamicEndpointPathCollide.message(route.route),
+				hint: AstroErrorData.PrerenderDynamicEndpointPathCollide.hint(route.component),
+				location: {
+					file: route.component,
+				},
+			});
+		}
+	}
+}

packages/astro/src/core/render/result.ts

@@ -24,10 +24,19 @@ const clientAddressSymbol = Symbol.for('astro.clientAddress');
 const responseSentSymbol = Symbol.for('astro.responseSent');
 
 export interface CreateResultArgs {
+	/**
+	 * Used to provide better error messages for `Astro.clientAddress`
+	 */
 	adapterName: string | undefined;
+	/**
+	 * Value of Astro config's `output` option, true if "server" or "hybrid"
+	 */
 	ssr: boolean;
 	logging: LogOptions;
 	origin: string;
+	/**
+	 * Used to support `Astro.__renderMarkdown` for legacy `<Markdown />` component
+	 */
 	markdown: MarkdownRenderingOptions;
 	mode: RuntimeMode;
 	params: Params;
@@ -36,6 +45,9 @@ export interface CreateResultArgs {
 	renderers: SSRLoadedRenderer[];
 	clientDirectives: Map<string, string>;
 	resolve: (s: string) => Promise<string>;
+	/**
+	 * Used for `Astro.site`
+	 */
 	site: string | undefined;
 	links?: Set<SSRElement>;
 	scripts?: Set<SSRElement>;

packages/astro/src/vite-plugin-astro-server/route.ts

@@ -4,12 +4,12 @@ import type { ComponentInstance, ManifestData, RouteData } from '../@types/astro
 import { attachToResponse } from '../core/cookies/index.js';
 import { call as callEndpoint } from '../core/endpoint/dev/index.js';
 import { throwIfRedirectNotAllowed } from '../core/endpoint/index.js';
-import { AstroErrorData } from '../core/errors/index.js';
+import { AstroErrorData, isAstroError } from '../core/errors/index.js';
 import { warn } from '../core/logger/core.js';
 import { loadMiddleware } from '../core/middleware/loadMiddleware.js';
 import type { DevelopmentEnvironment, SSROptions } from '../core/render/dev/index';
 import { preload, renderPage } from '../core/render/dev/index.js';
-import { getParamsAndProps, GetParamsAndPropsError } from '../core/render/index.js';
+import { getParamsAndProps } from '../core/render/index.js';
 import { createRequest } from '../core/request.js';
 import { matchAllRoutes } from '../core/routing/index.js';
 import { getSortedPreloadedMatches } from '../prerender/routing.js';
@@ -50,16 +50,15 @@ export async function matchRoute(
 	for await (const { preloadedComponent, route: maybeRoute, filePath } of preloadedMatches) {
 		// attempt to get static paths
 		// if this fails, we have a bad URL match!
-		const paramsAndPropsRes = await getParamsAndProps({
+		try {
-			mod: preloadedComponent,
+			await getParamsAndProps({
-			route: maybeRoute,
+				mod: preloadedComponent,
-			routeCache,
+				route: maybeRoute,
-			pathname: pathname,
+				routeCache,
-			logging,
+				pathname: pathname,
-			ssr: isServerLikeOutput(settings.config),
+				logging,
-		});
+				ssr: isServerLikeOutput(settings.config),
-
+			});
-		if (paramsAndPropsRes !== GetParamsAndPropsError.NoMatchingStaticPath) {
 			return {
 				route: maybeRoute,
 				filePath,
@@ -67,6 +66,12 @@ export async function matchRoute(
 				preloadedComponent,
 				mod: preloadedComponent,
 			};
+		} catch (e) {
+			// Ignore error for no matching static paths
+			if (isAstroError(e) && e.title === AstroErrorData.NoMatchingStaticPathFound.title) {
+				continue;
+			}
+			throw e;
 		}
 	}