chore: avoid loading page nodes three times (#13540)

Author: benmccann

packages/kit/src/runtime/server/page/index.js

@@ -15,7 +15,6 @@ import { render_response } from './render.js';
 import { respond_with_error } from './respond_with_error.js';
 import { get_option } from '../../../utils/options.js';
 import { get_data_json } from '../data/index.js';
-import { load_page_nodes } from './load_page_nodes.js';
 import { DEV } from 'esm-env';
 
 /**
@@ -29,10 +28,11 @@ const MAX_DEPTH = 10;
  * @param {import('types').SSROptions} options
  * @param {import('@sveltejs/kit').SSRManifest} manifest
  * @param {import('types').SSRState} state
+ * @param {Array<import('types').SSRNode | undefined>} nodes
  * @param {import('types').RequiredResolveOptions} resolve_opts
  * @returns {Promise<Response>}
  */
-export async function render_page(event, page, options, manifest, state, resolve_opts) {
+export async function render_page(event, page, options, manifest, state, nodes, resolve_opts) {
 	if (state.depth > MAX_DEPTH) {
 		// infinite request cycle detected
 		return text(`Not found: ${event.url.pathname}`, {
@@ -46,8 +46,6 @@ export async function render_page(event, page, options, manifest, state, resolve
 	}
 
 	try {
-		const nodes = await load_page_nodes(page, manifest);
-
 		const leaf_node = /** @type {import('types').SSRNode} */ (nodes.at(-1));
 
 		let status = 200;

packages/kit/src/runtime/server/page/load_page_nodes.js

@@ -24,7 +24,6 @@ import { json, text } from '../../exports/index.js';
 import { action_json_redirect, is_action_json_request } from './page/actions.js';
 import { INVALIDATED_PARAM, TRAILING_SLASH_PARAM } from '../shared.js';
 import { get_public_env } from './env_module.js';
-import { load_page_nodes } from './page/load_page_nodes.js';
 import { get_page_config } from '../../utils/route_config.js';
 import { resolve_route } from './page/server_routing.js';
 import { validateHeaders } from './validate-headers.js';
@@ -237,18 +236,19 @@ export async function respond(request, options, manifest, state) {
 	};
 
 	try {
+		/** @type {Array<import('types').SSRNode | undefined> | undefined} */
+		const page_nodes = route?.page ? await load_page_nodes(route.page, manifest) : undefined;
+
 		// determine whether we need to redirect to add/remove a trailing slash
 		if (route) {
 			// if `paths.base === '/a/b/c`, then the root route is `/a/b/c/`,
 			// regardless of the `trailingSlash` route option
 			if (url.pathname === base || url.pathname === base + '/') {
 				trailing_slash = 'always';
-			} else if (route.page) {
-				const nodes = await load_page_nodes(route.page, manifest);
-
+			} else if (page_nodes) {
 				if (DEV) {
-					const layouts = nodes.slice(0, -1);
-					const page = nodes.at(-1);
+					const layouts = page_nodes.slice(0, -1);
+					const page = page_nodes.at(-1);
 
 					for (const layout of layouts) {
 						if (layout) {
@@ -269,7 +269,7 @@ export async function respond(request, options, manifest, state) {
 					}
 				}
 
-				trailing_slash = get_option(nodes, 'trailingSlash');
+				trailing_slash = get_option(page_nodes, 'trailingSlash');
 			} else if (route.endpoint) {
 				const node = await route.endpoint();
 				trailing_slash = node.trailingSlash;
@@ -306,10 +306,9 @@ export async function respond(request, options, manifest, state) {
 					const node = await route.endpoint();
 					config = node.config ?? config;
 					prerender = node.prerender ?? prerender;
-				} else if (route.page) {
-					const nodes = await load_page_nodes(route.page, manifest);
-					config = get_page_config(nodes) ?? config;
-					prerender = get_option(nodes, 'prerender') ?? false;
+				} else if (page_nodes) {
+					config = get_page_config(page_nodes) ?? config;
+					prerender = get_option(page_nodes, 'prerender') ?? false;
 				}
 
 				if (state.before_handle) {
@@ -349,7 +348,7 @@ export async function respond(request, options, manifest, state) {
 		const response = await options.hooks.handle({
 			event,
 			resolve: (event, opts) =>
-				resolve(event, opts).then((response) => {
+				resolve(event, page_nodes, opts).then((response) => {
 					// add headers/cookies here, rather than inside `resolve`, so that we
 					// can do it once for all responses instead of once per `return`
 					for (const key in headers) {
@@ -426,9 +425,10 @@ export async function respond(request, options, manifest, state) {
 
 	/**
 	 * @param {import('@sveltejs/kit').RequestEvent} event
+	 * @param {Array<import('types').SSRNode | undefined> | undefined} page_nodes
 	 * @param {import('@sveltejs/kit').ResolveOptions} [opts]
 	 */
-	async function resolve(event, opts) {
+	async function resolve(event, page_nodes, opts) {
 		try {
 			if (opts) {
 				resolve_opts = {
@@ -472,8 +472,18 @@ export async function respond(request, options, manifest, state) {
 				} else if (route.endpoint && (!route.page || is_endpoint_request(event))) {
 					response = await render_endpoint(event, await route.endpoint(), state);
 				} else if (route.page) {
-					if (page_methods.has(method)) {
-						response = await render_page(event, route.page, options, manifest, state, resolve_opts);
+					if (!page_nodes) {
+						throw new Error('page_nodes not found. This should never happen');
+					} else if (page_methods.has(method)) {
+						response = await render_page(
+							event,
+							route.page,
+							options,
+							manifest,
+							state,
+							page_nodes,
+							resolve_opts
+						);
 					} else {
 						const allowed_methods = new Set(allowed_page_methods);
 						const node = await manifest._.nodes[route.page.leaf]();
@@ -499,9 +509,8 @@ export async function respond(request, options, manifest, state) {
 						}
 					}
 				} else {
-					// a route will always have a page or an endpoint, but TypeScript
-					// doesn't know that
-					throw new Error('This should never happen');
+					// a route will always have a page or an endpoint, but TypeScript doesn't know that
+					throw new Error('Route is neither page nor endpoint. This should never happen');
 				}
 
 				// If the route contains a page and an endpoint, we need to add a
@@ -578,3 +587,15 @@ export async function respond(request, options, manifest, state) {
 		}
 	}
 }
+
+/**
+ * @param {import('types').PageNodeIndexes} page
+ * @param {import('@sveltejs/kit').SSRManifest} manifest
+ */
+export function load_page_nodes(page, manifest) {
+	return Promise.all([
+		// we use == here rather than === because [undefined] serializes as "[null]"
+		...page.layouts.map((n) => (n == undefined ? n : manifest._.nodes[n]())),
+		manifest._.nodes[page.leaf]()
+	]);
+}