breaking: stricter route validation, more correct config resolution (#…

…11256) * fix get_config * oops * fix, changeset * always validate server exports --------- Co-authored-by: Rich Harris <[email protected]>
sveltejs · Dec 11, 2023 · aedb572 · aedb572
1 parent c59375c
commit aedb572
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 72 deletions.
diff --git a/.changeset/eight-pens-help.md b/.changeset/eight-pens-help.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': major
+---
+
+breaking: fail if route with +page and +server is marked prerenderable
diff --git a/.changeset/loud-plants-move.md b/.changeset/loud-plants-move.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': major
+---
+
+breaking: fail if +page and +server have mismatched config
diff --git a/.changeset/mean-trees-notice.md b/.changeset/mean-trees-notice.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: resolve route config correctly
diff --git a/packages/kit/src/core/postbuild/analyse.js b/packages/kit/src/core/postbuild/analyse.js
@@ -52,97 +52,54 @@ async function analyse({ manifest_path, env }) {
 		routes: new Map()
 	};
 
-	// analyse nodes
-	for (const loader of manifest._.nodes) {
-		const node = await loader();
+	const nodes = await Promise.all(manifest._.nodes.map((loader) => loader()));
 
+	// analyse nodes
+	for (const node of nodes) {
 		metadata.nodes[node.index] = {
 			has_server_load: node.server?.load !== undefined || node.server?.trailingSlash !== undefined
 		};
 	}
 
 	// analyse routes
 	for (const route of manifest._.routes) {
-		/** @type {Array<'GET' | 'POST'>} */
-		const page_methods = [];
-
-		/** @type {(import('types').HttpMethod | '*')[]} */
-		const api_methods = [];
+		const page =
+			route.page &&
+			analyse_page(
+				route.page.layouts.map((n) => (n === undefined ? n : nodes[n])),
+				nodes[route.page.leaf]
+			);
 
-		/** @type {import('types').PrerenderOption | undefined} */
-		let prerender = undefined;
-		/** @type {any} */
-		let config = undefined;
-		/** @type {import('types').PrerenderEntryGenerator | undefined} */
-		let entries = undefined;
+		const endpoint = route.endpoint && analyse_endpoint(route, await route.endpoint());
 
-		if (route.endpoint) {
-			const mod = await route.endpoint();
-			if (mod.prerender !== undefined) {
-				validate_server_exports(mod, route.id);
+		if (page?.prerender && endpoint?.prerender) {
+			throw new Error(`Cannot prerender a route with both +page and +server files (${route.id})`);
+		}
 
-				if (mod.prerender && (mod.POST || mod.PATCH || mod.PUT || mod.DELETE)) {
+		if (page?.config && endpoint?.config) {
+			for (const key in { ...page.config, ...endpoint.config }) {
+				if (JSON.stringify(page.config[key]) !== JSON.stringify(endpoint.config[key])) {
 					throw new Error(
-						`Cannot prerender a +server file with POST, PATCH, PUT, or DELETE (${route.id})`
+						`Mismatched route config for ${route.id} — the +page and +server files must export the same config, if any`
 					);
 				}
-
-				prerender = mod.prerender;
 			}
-
-			Object.values(mod).forEach((/** @type {import('types').HttpMethod} */ method) => {
-				if (mod[method] && ENDPOINT_METHODS.has(method)) {
-					api_methods.push(method);
-				} else if (mod.fallback) {
-					api_methods.push('*');
-				}
-			});
-
-			config = mod.config;
-			entries = mod.entries;
 		}
 
-		if (route.page) {
-			const nodes = await Promise.all(
-				[...route.page.layouts, route.page.leaf].map((n) => {
-					if (n !== undefined) return manifest._.nodes[n]();
-				})
-			);
-
-			const layouts = nodes.slice(0, -1);
-			const page = nodes.at(-1);
-
-			for (const layout of layouts) {
-				if (layout) {
-					validate_layout_server_exports(layout.server, layout.server_id);
-					validate_layout_exports(layout.universal, layout.universal_id);
-				}
-			}
-
-			if (page) {
-				page_methods.push('GET');
-				if (page.server?.actions) page_methods.push('POST');
-
-				validate_page_server_exports(page.server, page.server_id);
-				validate_page_exports(page.universal, page.universal_id);
-			}
-
-			prerender = get_option(nodes, 'prerender') ?? false;
-
-			config = get_config(nodes);
-			entries ??= get_option(nodes, 'entries');
-		}
+		const page_methods = page?.methods ?? [];
+		const api_methods = endpoint?.methods ?? [];
+		const entries = page?.entries ?? endpoint?.entries;
 
 		metadata.routes.set(route.id, {
-			config,
+			config: page?.config ?? endpoint?.config,
 			methods: Array.from(new Set([...page_methods, ...api_methods])),
 			page: {
 				methods: page_methods
 			},
 			api: {
 				methods: api_methods
 			},
-			prerender,
+			prerender: page?.prerender ?? endpoint?.prerender,
 			entries:
 				entries && (await entries()).map((entry_object) => resolvePath(route.id, entry_object))
 		});
@@ -151,20 +108,81 @@ async function analyse({ manifest_path, env }) {
 	return metadata;
 }
 
+/**
+ * @param {import('types').SSRRoute} route
+ * @param {import('types').SSREndpoint} mod
+ */
+function analyse_endpoint(route, mod) {
+	validate_server_exports(mod, route.id);
+
+	if (mod.prerender && (mod.POST || mod.PATCH || mod.PUT || mod.DELETE)) {
+		throw new Error(
+			`Cannot prerender a +server file with POST, PATCH, PUT, or DELETE (${route.id})`
+		);
+	}
+
+	/** @type {Array<import('types').HttpMethod | '*'>} */
+	const methods = [];
+
+	Object.values(mod).forEach((/** @type {import('types').HttpMethod} */ method) => {
+		if (mod[method] && ENDPOINT_METHODS.has(method)) {
+			methods.push(method);
+		} else if (mod.fallback) {
+			methods.push('*');
+		}
+	});
+
+	return {
+		config: mod.config,
+		entries: mod.entries,
+		methods,
+		prerender: mod.prerender ?? false
+	};
+}
+
+/**
+ * @param {Array<import('types').SSRNode | undefined>} layouts
+ * @param {import('types').SSRNode} leaf
+ */
+function analyse_page(layouts, leaf) {
+	for (const layout of layouts) {
+		if (layout) {
+			validate_layout_server_exports(layout.server, layout.server_id);
+			validate_layout_exports(layout.universal, layout.universal_id);
+		}
+	}
+
+	/** @type {Array<'GET' | 'POST'>} */
+	const methods = ['GET'];
+	if (leaf.server?.actions) methods.push('POST');
+
+	validate_page_server_exports(leaf.server, leaf.server_id);
+	validate_page_exports(leaf.universal, leaf.universal_id);
+
+	return {
+		config: get_config([...layouts, leaf]),
+		entries: leaf.universal?.entries ?? leaf.server?.entries,
+		methods,
+		prerender: get_option([...layouts, leaf], 'prerender') ?? false
+	};
+}
+
 /**
  * Do a shallow merge (first level) of the config object
  * @param {Array<import('types').SSRNode | undefined>} nodes
  */
 function get_config(nodes) {
+	/** @type {any} */
 	let current = {};
+
 	for (const node of nodes) {
-		const config = node?.universal?.config ?? node?.server?.config;
-		if (config) {
-			current = {
-				...current,
-				...config
-			};
-		}
+		if (!node?.universal?.config && !node?.server?.config) continue;
+
+		current = {
+			...current,
+			...node?.universal?.config,
+			...node?.server?.config
+		};
 	}
 
 	return Object.keys(current).length ? current : undefined;