Address review comments

`environment.ts` is now lives directly in `src/core`, rather than `src/core/render`.

`environment.createPipeline` is removed. `Pipeline.create` is used instead.

Constructors with positional arguments are replaced by `Environment.create` with named arguments.

Clarifies the use of `HiddenPipeline`.

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

@@ -1,3 +1,7 @@
-import { Environment } from "../render/environment.js";
+import { Environment } from "../environment.js";
 
-export class AppEnvironment extends Environment {}
+export class AppEnvironment extends Environment {
+    static create({ logger, manifest, mode, renderers, resolve, serverLike, streaming }: Pick<AppEnvironment, 'logger' | 'manifest' | 'mode' | 'renderers' | 'resolve' | 'serverLike' | 'streaming'>) {
+        return new AppEnvironment(logger, manifest, mode, renderers, resolve, serverLike, streaming);
+    }
+}

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

@@ -29,6 +29,7 @@ import { matchRoute } from '../routing/match.js';
 import { AppEnvironment } from './environment.js';
 import type { RouteInfo } from './types.js';
 import { normalizeTheLocale } from '../../i18n/index.js';
+import { Pipeline } from '../pipeline.js';
 export { deserializeManifest } from './common.js';
 
 const localsSymbol = Symbol.for('astro.locals');
@@ -124,12 +125,12 @@ export class App {
 	 * @private
 	 */
 	#createEnvironment(streaming = false) {
-		return new AppEnvironment(
-			this.#logger,
-			this.#manifest,
-			'production',
-			this.#manifest.renderers,
-			async (specifier: string) => {
+		return AppEnvironment.create({
+			logger: this.#logger,
+			manifest: this.#manifest,
+			mode: 'production',
+			renderers: this.#manifest.renderers,
+			resolve: async (specifier: string) => {
 				if (!(specifier in this.#manifest.entryModules)) {
 					throw new Error(`Unable to resolve [${specifier}]`);
 				}
@@ -144,10 +145,9 @@ export class App {
 					}
 				}
 			},
-			true,
+			serverLike: true,
 			streaming,
-			new RouteCache(this.#logger),
-		);
+		})
 	}
 
 	set setManifestData(newManifestData: ManifestData) {
@@ -323,7 +323,7 @@ export class App {
 		);
 		let response;
 		try {
-			const pipeline = this.#environment.createPipeline({ pathname, renderContext });
+			const pipeline = Pipeline.create({ environment: this.#environment, pathname, renderContext })
 			response = await pipeline.renderRoute(pageModule);
 		} catch (err: any) {
 			this.#logger.error(null, err.stack || err.message || String(err));
@@ -481,11 +481,12 @@ export class App {
 					mod,
 					status
 				);
-				const pipeline = this.#environment.createPipeline({
+				const pipeline = Pipeline.create({
+					environment: this.#environment,
+					middleware: skipMiddleware ? (_, next) => next() : undefined,
 					pathname: this.#getPathnameFromRequest(request),
 					renderContext: newRenderContext,
-					middleware: skipMiddleware ? (_, next) => next() : undefined
-				});
+				})
 				const response = await pipeline.renderRoute(await mod.page());
 				return this.#mergeResponses(response, originalResponse);
 			} catch {

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

@@ -1,3 +1,4 @@
+import type { B } from 'shikiji-core/dist/chunk-types.mjs';
 import type { SSRLoadedRenderer } from '../../@types/astro.js';
 import { getOutputDirectory, isServerLikeOutput } from '../../prerender/utils.js';
 import { BEFORE_HYDRATION_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
@@ -19,10 +20,10 @@ import { i18nHasFallback } from './util.js';
  * This build environment is responsible to gather the files emitted by the SSR build and generate the pages by executing these files.
  */
 export class BuildEnvironment extends Environment {
-	constructor(
-		readonly options: StaticBuildOptions,
+	private constructor(
 		readonly internals: BuildInternals,
 		readonly manifest: SSRManifest,
+		readonly options: StaticBuildOptions,
 		readonly config = options.settings.config,
 		readonly settings = options.settings
 	) {
@@ -47,8 +48,13 @@ export class BuildEnvironment extends Environment {
 		}
 		const serverLike = isServerLikeOutput(config);
 		const streaming = true;
-		super(options.logger, manifest, options.mode, manifest.renderers, resolve, serverLike, streaming, options.routeCache)
+		super(options.logger, manifest, options.mode, manifest.renderers, resolve, serverLike, streaming)
 	}
+
+	static create({ internals, manifest, options }: Pick<BuildEnvironment, 'internals' | 'manifest' | 'options'>) {
+		return new BuildEnvironment(internals, manifest, options);
+	}
+
 	/**
 	 * The SSR build emits two important files:
 	 * - dist/server/manifest.mjs

packages/astro/src/core/build/generate.ts

@@ -66,6 +66,7 @@ import type {
 } from './types.js';
 import { getTimeStat, shouldAppendForwardSlash } from './util.js';
 import { NoPrerenderedRoutesWithDomains } from '../errors/errors-data.js';
+import { Pipeline } from '../pipeline.js';
 
 function createEntryURL(filePath: string, outFolder: URL) {
 	return new URL('./' + filePath + `?time=${Date.now()}`, outFolder);
@@ -136,14 +137,14 @@ export function chunkIsPage(
 	return false;
 }
 
-export async function generatePages(opts: StaticBuildOptions, internals: BuildInternals) {
+export async function generatePages(options: StaticBuildOptions, internals: BuildInternals) {
 	const generatePagesTimer = performance.now();
-	const ssr = isServerLikeOutput(opts.settings.config);
+	const ssr = isServerLikeOutput(options.settings.config);
 	let manifest: SSRManifest;
 	if (ssr) {
-		manifest = await BuildEnvironment.retrieveManifest(opts, internals);
+		manifest = await BuildEnvironment.retrieveManifest(options, internals);
 	} else {
-		const baseDirectory = getOutputDirectory(opts.settings.config);
+		const baseDirectory = getOutputDirectory(options.settings.config);
 		const renderersEntryUrl = new URL('renderers.mjs', baseDirectory);
 		const renderers = await import(renderersEntryUrl.toString());
 		let middleware: MiddlewareHandler = (_, next) => next();
@@ -155,18 +156,18 @@ export async function generatePages(opts: StaticBuildOptions, internals: BuildIn
 			);
 		} catch {}
 		manifest = createBuildManifest(
-			opts.settings,
+			options.settings,
 			internals,
 			renderers.renderers as SSRLoadedRenderer[],
 			middleware
 		);
 	}
-	const environment = new BuildEnvironment(opts, internals, manifest);
+	const environment = BuildEnvironment.create({ internals, manifest, options });
 	const { config, logger } = environment;
 
 	const outFolder = ssr
-		? opts.settings.config.build.server
-		: getOutDirWithinCwd(opts.settings.config.outDir);
+		? options.settings.config.build.server
+		: getOutDirWithinCwd(options.settings.config.outDir);
 
 	// HACK! `astro:assets` relies on a global to know if its running in dev, prod, ssr, ssg, full moon
 	// If we don't delete it here, it's technically not impossible (albeit improbable) for it to leak
@@ -192,7 +193,7 @@ export async function generatePages(opts: StaticBuildOptions, internals: BuildIn
 
 				const ssrEntryURLPage = createEntryURL(filePath, outFolder);
 				const ssrEntryPage = await import(ssrEntryURLPage.toString());
-				if (opts.settings.adapter?.adapterFeatures?.functionPerRoute) {
+				if (options.settings.adapter?.adapterFeatures?.functionPerRoute) {
 					// forcing to use undefined, so we fail in an expected way if the module is not even there.
 					const ssrEntry = ssrEntryPage?.pageModule;
 					if (ssrEntry) {
@@ -252,10 +253,7 @@ export async function generatePages(opts: StaticBuildOptions, internals: BuildIn
 		delete globalThis?.astroAsset?.addStaticImage;
 	}
 
-	await runHookBuildGenerated({
-		config: opts.settings.config,
-		logger: environment.logger,
-	});
+	await runHookBuildGenerated({ config, logger });
 }
 
 async function generatePage(
@@ -331,7 +329,7 @@ async function getPathsForRoute(
 	environment: BuildEnvironment,
 	builtPaths: Set<string>
 ): Promise<Array<string>> {
-	const { logger, options, serverLike } = environment;
+	const { logger, options, routeCache, serverLike } = environment;
 	let paths: Array<string> = [];
 	if (route.pathname) {
 		paths.push(route.pathname);
@@ -346,7 +344,7 @@ async function getPathsForRoute(
 		const staticPaths = await callGetStaticPaths({
 			mod,
 			route,
-			routeCache: options.routeCache,
+			routeCache,
 			logger,
 			ssr: serverLike,
 		}).catch((err) => {
@@ -555,10 +553,7 @@ async function generatePath(
 		routing: i18n?.routing,
 		defaultLocale: i18n?.defaultLocale,
 	});
-	const pipeline = environment.createPipeline({
-		pathname,
-		renderContext,
-	})
+	const pipeline = Pipeline.create({ environment, pathname, renderContext })
 
 	let body: string | Uint8Array;
 

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

@@ -98,7 +98,6 @@ class AstroBuilder {
 	private logger: Logger;
 	private mode: RuntimeMode = 'production';
 	private origin: string;
-	private routeCache: RouteCache;
 	private manifest: ManifestData;
 	private timer: Record<string, number>;
 	private teardownCompiler: boolean;
@@ -110,7 +109,6 @@ class AstroBuilder {
 		this.settings = settings;
 		this.logger = options.logger;
 		this.teardownCompiler = options.teardownCompiler ?? true;
-		this.routeCache = new RouteCache(this.logger);
 		this.origin = settings.config.site
 			? new URL(settings.config.site).origin
 			: `http://localhost:${settings.config.server.port}`;
@@ -195,7 +193,6 @@ class AstroBuilder {
 			mode: this.mode,
 			origin: this.origin,
 			pageNames,
-			routeCache: this.routeCache,
 			teardownCompiler: this.teardownCompiler,
 			viteConfig,
 		};

packages/astro/src/core/build/types.ts

@@ -43,7 +43,6 @@ export interface StaticBuildOptions {
 	mode: RuntimeMode;
 	origin: string;
 	pageNames: string[];
-	routeCache: RouteCache;
 	viteConfig: InlineConfig;
 	teardownCompiler: boolean;
 }

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

@@ -1,18 +1,18 @@
-import type { MiddlewareHandler, RuntimeMode, SSRLoadedRenderer, SSRManifest } from '../../@types/astro.js';
-import type { Logger } from '../logger/core.js';
-import type { RouteCache } from './route-cache.js';
-import { Pipeline } from '../pipeline.js';
-import type { RenderContext } from './context.js';
-import { createI18nMiddleware } from '../../i18n/middleware.js';
-import { sequence } from '../middleware/index.js';
+import type { MiddlewareHandler, RuntimeMode, SSRLoadedRenderer, SSRManifest } from '../@types/astro.js';
+import type { Logger } from './logger/core.js';
+import { RouteCache } from './render/route-cache.js';
+import { Pipeline } from './pipeline.js';
+import type { RenderContext } from './render/context.js';
+import { createI18nMiddleware } from '../i18n/middleware.js';
+import { sequence } from './middleware/index.js';
 
 /**
  * The environment represents the static parts of rendering that do not change between requests.
  * These are mostly known when the server first starts up and do not change.
  * Thus, an environment is created once at process start and then used by every pipeline.
  */
 export abstract class Environment {
-	readonly internalMiddleware: MiddlewareHandler;
+	readonly internalMiddleware: MiddlewareHandler[];
 
 	constructor(
 		readonly logger: Logger,
@@ -21,14 +21,13 @@ export abstract class Environment {
 		 * "development" or "production"
 		 */
 		readonly mode: RuntimeMode,
-		public renderers: SSRLoadedRenderer[],
+		readonly renderers: SSRLoadedRenderer[],
 		readonly resolve: (s: string) => Promise<string>,
 		/**
 		 * Based on Astro config's `output` option, `true` if "server" or "hybrid".
 		 */
 		readonly serverLike: boolean,
 		readonly streaming: boolean,
-		readonly routeCache: RouteCache,
 		/**
 		 * Used to provide better error messages for `Astro.clientAddress`
 		 */
@@ -37,15 +36,12 @@ export abstract class Environment {
 		readonly compressHTML = manifest.compressHTML,
 		readonly i18n = manifest.i18n,
 		readonly middleware = manifest.middleware,
+		readonly routeCache = new RouteCache(logger, mode),
 		/**
 		 * Used for `Astro.site`.
 		 */
 		readonly site = manifest.site,
 	) {
-		this.internalMiddleware = createI18nMiddleware(i18n, manifest.base, manifest.trailingSlash, manifest.buildFormat);
-	}
-
-	createPipeline({ renderContext, pathname, middleware }: { pathname: string; renderContext: RenderContext; middleware?: MiddlewareHandler }) {
-		return new Pipeline(this, renderContext.locals ?? {}, sequence(this.internalMiddleware, middleware ?? this.middleware), pathname, renderContext);
+		this.internalMiddleware = [ createI18nMiddleware(i18n, manifest.base, manifest.trailingSlash, manifest.buildFormat) ];
 	}
 }

packages/astro/src/core/pipeline.ts

@@ -3,6 +3,7 @@ import { renderEndpoint } from '../runtime/server/endpoint.js';
 import { attachCookiesToResponse } from './cookies/response.js';
 import { createAPIContext } from './endpoint/index.js';
 import { callMiddleware } from './middleware/callMiddleware.js';
+import { sequence } from './middleware/index.js';
 import { renderPage } from './render/core.js';
 import type { Environment, RenderContext } from './render/index.js';
 
@@ -12,7 +13,7 @@ import type { Environment, RenderContext } from './render/index.js';
  * Check the {@link ./README.md|README} for more information about the pipeline.
  */
 export class Pipeline {
-	constructor(
+	private constructor(
 		readonly environment: Environment,
 		readonly locals: App.Locals,
 		readonly middleware: MiddlewareHandler,
@@ -21,6 +22,10 @@ export class Pipeline {
 		readonly request = renderContext.request,
 	) {}
 
+	static create({ environment, locals, middleware, pathname, renderContext }: Pick<Pipeline, 'environment' | 'pathname' | 'renderContext'> & Partial<Pick<Pipeline, 'locals' | 'middleware'>>) {
+		return new Pipeline(environment, locals ?? {}, sequence(...environment.internalMiddleware, middleware ?? environment.middleware), pathname, renderContext)
+	}
+
 	/**
 	 * The main function of the pipeline. Use this function to render any route known to Astro;
 	 * It attempts to render a route. A route can be a:
@@ -40,10 +45,10 @@ export class Pipeline {
 		const { adapterName, logger, site, serverLike } = environment;
 		const apiContext = createAPIContext({ adapterName, defaultLocale, locales, params, props, request, route, routingStrategy, site });
 		HiddenPipeline.set(request, this);
-		const terminalNext = renderContext.route.type === 'endpoint'
+		const lastNext = renderContext.route.type === 'endpoint'
 			? () => renderEndpoint(componentInstance as any as EndpointHandler, apiContext, serverLike, logger)
 			: () => renderPage({ mod: componentInstance, renderContext, env: environment, cookies: apiContext.cookies });
-		const response = await callMiddleware(this.middleware, apiContext, terminalNext);
+		const response = await callMiddleware(this.middleware, apiContext, lastNext);
 		attachCookiesToResponse(response, apiContext.cookies);
 		return response;
 	}
@@ -53,10 +58,23 @@ export class Pipeline {
 	}
 }
 
+/** 
+ * The constructor of this class returns the passed object, which allows private fields to be set by a subclass.
+ */
+class AllowPrivateFields {
+	constructor(request: Request) {
+		return request
+	}
+}
+
 /**
  * Allows internal middleware to read the pipeline associated with the current request.
+ * 
+ * It works by setting a private field on the request object called #pipeline.
+ * This prevents user code from having access to routeData and other internals,
+ * as only the HiddenPipeline class and its static methods can see the #pipeline field.
  */
-class HiddenPipeline extends class { constructor(request: Request) { return request } } {
+class HiddenPipeline extends AllowPrivateFields {
 	#pipeline!: Pipeline
 	static get(request: Request) {
 		if (#pipeline in request) return request.#pipeline 

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

@@ -9,7 +9,7 @@ import type {
 } from '../../@types/astro.js';
 import { normalizeTheLocale, toCodes } from '../../i18n/index.js';
 import { AstroError, AstroErrorData } from '../errors/index.js';
-import type { Environment } from './environment.js';
+import type { Environment } from '../environment.js';
 import { getParamsAndProps } from './params-and-props.js';
 import type { RoutingStrategies } from '../config/schema.js';
 

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

@@ -6,7 +6,7 @@ import { AstroError } from '../errors/index.js';
 import { routeIsFallback } from '../redirects/helpers.js';
 import { redirectRouteGenerate, redirectRouteStatus, routeIsRedirect } from '../redirects/index.js';
 import type { RenderContext } from './context.js';
-import type { Environment } from './environment.js';
+import type { Environment } from '../environment.js';
 import { createResult } from './result.js';
 
 export type RenderPage = {

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

@@ -1,8 +1,8 @@
 import type { AstroMiddlewareInstance, ComponentInstance, RouteData } from '../../@types/astro.js';
-import type { Environment } from './environment.js';
+import type { Environment } from '../environment.js';
 export { computePreferredLocale, createRenderContext } from './context.js';
 export type { RenderContext } from './context.js';
-export { Environment } from './environment.js';
+export { Environment } from '../environment.js';
 export { getParamsAndProps } from './params-and-props.js';
 export { loadRenderer } from './renderer.js';