feat: Fail prerendering for pages with mutative endpoints - fixes #3410…

… and #4252 (#4812) * feat: Failing test * feat: Prerendered pages with mutative endpoints fail at runtime * feat: Changeset * fix: clarify with variable * determine prerenderability based on config, not state * rename state.prerender to state.prerendering, remove state.prerendering.default * fix type Co-authored-by: Rich Harris <hello@rich-harris.dev>
sveltejs · May 23, 2022 · a72123a · a72123a
1 parent 77a1a73
commit a72123a
Show file tree

Hide file tree

Showing 13 changed files with 73 additions and 28 deletions.
diff --git a/.changeset/brave-jobs-occur.md b/.changeset/brave-jobs-occur.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+feat: Pages marked for prerendering fail during ssr at runtime
diff --git a/packages/kit/src/core/build/build_server.js b/packages/kit/src/core/build/build_server.js
@@ -70,7 +70,10 @@ export class Server {
 			method_override: ${s(config.kit.methodOverride)},
 			paths: { base, assets },
 			prefix: assets + '/${config.kit.appDir}/',
-			prerender: ${config.kit.prerender.enabled},
+			prerender: {
+				default: ${config.kit.prerender.default},
+				enabled: ${config.kit.prerender.enabled}
+			},
 			read,
 			root,
 			service_worker: ${has_service_worker ? "base + '/service-worker.js'" : 'null'},

diff --git a/packages/kit/src/core/build/prerender/prerender.js b/packages/kit/src/core/build/prerender/prerender.js
@@ -131,8 +131,7 @@ export async function prerender({ config, entries, files, log }) {
 
 		const response = await server.respond(new Request(`http://sveltekit-prerender${encoded}`), {
 			getClientAddress,
-			prerender: {
-				default: config.kit.prerender.default,
+			prerendering: {
 				dependencies
 			}
 		});
@@ -272,9 +271,8 @@ export async function prerender({ config, entries, files, log }) {
 
 	const rendered = await server.respond(new Request('http://sveltekit-prerender/[fallback]'), {
 		getClientAddress,
-		prerender: {
+		prerendering: {
 			fallback: true,
-			default: false,
 			dependencies: new Map()
 		}
 	});

diff --git a/packages/kit/src/core/dev/plugin.js b/packages/kit/src/core/dev/plugin.js
@@ -315,7 +315,10 @@ export async function create_plugin(config, cwd) {
 									assets
 								},
 								prefix: '',
-								prerender: config.kit.prerender.enabled,
+								prerender: {
+									default: config.kit.prerender.default,
+									enabled: config.kit.prerender.enabled
+								},
 								read: (file) => fs.readFileSync(path.join(config.kit.files.assets, file)),
 								root,
 								router: config.kit.browser.router,

diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
@@ -49,7 +49,7 @@ export async function respond(request, options, state) {
 	/** @type {Record<string, string>} */
 	let params = {};
 
-	if (options.paths.base && !state.prerender?.fallback) {
+	if (options.paths.base && !state.prerendering?.fallback) {
 		if (!decoded.startsWith(options.paths.base)) {
 			return new Response(undefined, { status: 404 });
 		}
@@ -63,7 +63,7 @@ export async function respond(request, options, state) {
 		url = new URL(url.origin + url.pathname.slice(0, -DATA_SUFFIX.length) + url.search);
 	}
 
-	if (!state.prerender || !state.prerender.fallback) {
+	if (!state.prerendering?.fallback) {
 		const matchers = await options.manifest._.matchers();
 
 		for (const candidate of options.manifest._.routes) {
@@ -82,7 +82,7 @@ export async function respond(request, options, state) {
 	if (route?.type === 'page') {
 		const normalized = normalize_path(url.pathname, options.trailing_slash);
 
-		if (normalized !== url.pathname && !state.prerender?.fallback) {
+		if (normalized !== url.pathname && !state.prerendering?.fallback) {
 			return new Response(undefined, {
 				status: 301,
 				headers: {
@@ -173,7 +173,7 @@ export async function respond(request, options, state) {
 					};
 				}
 
-				if (state.prerender && state.prerender.fallback) {
+				if (state.prerendering?.fallback) {
 					return await render_response({
 						event,
 						options,
@@ -275,7 +275,7 @@ export async function respond(request, options, state) {
 					});
 				}
 
-				if (state.prerender) {
+				if (state.prerendering) {
 					return new Response('not found', { status: 404 });
 				}
 

diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js
@@ -52,13 +52,15 @@ export async function load_node({
 	/** @type {import('types').LoadOutput} */
 	let loaded;
 
+	const should_prerender = node.module.prerender ?? options.prerender.default;
+
 	/** @type {import('types').ShadowData} */
 	const shadow = is_leaf
 		? await load_shadow_data(
 				/** @type {import('types').SSRPage} */ (route),
 				event,
 				options,
-				!!state.prerender
+				should_prerender
 		  )
 		: {};
 
@@ -81,7 +83,7 @@ export async function load_node({
 	} else if (module.load) {
 		/** @type {import('types').LoadInput} */
 		const load_input = {
-			url: state.prerender ? create_prerendering_url_proxy(event.url) : event.url,
+			url: state.prerendering ? create_prerendering_url_proxy(event.url) : event.url,
 			params: event.params,
 			props: shadow.body || {},
 			routeId: event.routeId,
@@ -217,9 +219,9 @@ export async function load_node({
 						}
 					);
 
-					if (state.prerender) {
+					if (state.prerendering) {
 						dependency = { response, body: null };
-						state.prerender.dependencies.set(resolved, dependency);
+						state.prerendering.dependencies.set(resolved, dependency);
 					}
 				} else {
 					// external
@@ -364,15 +366,15 @@ export async function load_node({
 	}
 
 	// generate __data.json files when prerendering
-	if (shadow.body && state.prerender) {
+	if (shadow.body && state.prerendering) {
 		const pathname = `${event.url.pathname.replace(/\/$/, '')}/__data.json`;
 
 		const dependency = {
 			response: new Response(undefined),
 			body: JSON.stringify(shadow.body)
 		};
 
-		state.prerender.dependencies.set(pathname, dependency);
+		state.prerendering.dependencies.set(pathname, dependency);
 	}
 
 	return {

diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js
@@ -40,7 +40,7 @@ export async function render_response({
 	resolve_opts,
 	stuff
 }) {
-	if (state.prerender) {
+	if (state.prerendering) {
 		if (options.csp.mode === 'nonce') {
 			throw new Error('Cannot use prerendering if config.kit.csp.mode === "nonce"');
 		}
@@ -108,7 +108,7 @@ export async function render_response({
 				routeId: event.routeId,
 				status,
 				stuff,
-				url: state.prerender ? create_prerendering_url_proxy(event.url) : event.url
+				url: state.prerendering ? create_prerendering_url_proxy(event.url) : event.url
 			},
 			components: branch.map(({ node }) => node.module.default)
 		};
@@ -148,7 +148,7 @@ export async function render_response({
 	await csp_ready;
 	const csp = new Csp(options.csp, {
 		dev: options.dev,
-		prerender: !!state.prerender,
+		prerender: !!state.prerendering,
 		needs_nonce: options.template_contains_nonce
 	});
 
@@ -257,7 +257,7 @@ export async function render_response({
 			<script${csp.script_needs_nonce ? ` nonce="${csp.nonce}"` : ''}>${init_service_worker}</script>`;
 	}
 
-	if (state.prerender) {
+	if (state.prerendering) {
 		const http_equiv = [];
 
 		const csp_headers = csp.get_meta();
@@ -295,7 +295,7 @@ export async function render_response({
 		headers.set('permissions-policy', 'interest-cohort=()');
 	}
 
-	if (!state.prerender) {
+	if (!state.prerendering) {
 		const csp_header = csp.get_header();
 		if (csp_header) {
 			headers.set('content-security-policy', csp_header);

diff --git a/packages/kit/src/runtime/server/page/respond.js b/packages/kit/src/runtime/server/page/respond.js
@@ -68,11 +68,11 @@ export async function respond(opts) {
 
 	let page_config = get_page_config(leaf, options);
 
-	if (state.prerender) {
+	if (state.prerendering) {
 		// if the page isn't marked as prerenderable (or is explicitly
 		// marked NOT prerenderable, if `prerender.default` is `true`),
 		// then bail out at this point
-		const should_prerender = leaf.prerender ?? state.prerender.default;
+		const should_prerender = leaf.prerender ?? options.prerender.default;
 		if (!should_prerender) {
 			return new Response(undefined, {
 				status: 204

diff --git a/packages/kit/test/apps/basics/src/routes/prerendering/__error.svelte b/packages/kit/test/apps/basics/src/routes/prerendering/__error.svelte
@@ -0,0 +1,15 @@
+<script context="module">
+	export const load = ({ error, status }) => {
+		return {
+			props: {
+				title: `${status}: ${error.message}`
+			}
+		};
+	};
+</script>
+
+<script>
+	export let title = 'Unknown error!';
+</script>
+
+<h1>{title}</h1>
diff --git a/packages/kit/test/apps/basics/src/routes/prerendering/mutative-endpoint.svelte b/packages/kit/test/apps/basics/src/routes/prerendering/mutative-endpoint.svelte
@@ -0,0 +1,5 @@
+<script context="module">
+	export const prerender = true;
+</script>
+
+<h1>I have a mutative endpoint!</h1>
diff --git a/packages/kit/test/apps/basics/src/routes/prerendering/mutative-endpoint.ts b/packages/kit/test/apps/basics/src/routes/prerendering/mutative-endpoint.ts
@@ -0,0 +1,3 @@
+export const post = () => {
+	return {};
+};
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
@@ -1105,6 +1105,15 @@ test.describe.parallel('Errors', () => {
 		expect(response.status()).toBe(500);
 		expect(await response.text()).toMatch('thisvariableisnotdefined is not defined');
 	});
+
+	test('prerendering a page with a mutative page endpoint results in a catchable error', async ({
+		page
+	}) => {
+		await page.goto('/prerendering/mutative-endpoint');
+		expect(await page.textContent('h1')).toBe(
+			'500: Cannot prerender pages that have endpoints with mutative methods'
+		);
+	});
 });
 
 test.describe.parallel('ETags', () => {

diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts
@@ -94,7 +94,7 @@ export class InternalServer extends Server {
 	respond(
 		request: Request,
 		options: RequestOptions & {
-			prerender?: PrerenderOptions;
+			prerendering?: PrerenderOptions;
 		}
 	): Promise<Response>;
 }
@@ -147,7 +147,6 @@ export interface PrerenderDependency {
 
 export interface PrerenderOptions {
 	fallback?: boolean;
-	default: boolean;
 	dependencies: Map<string, PrerenderDependency>;
 }
 
@@ -254,7 +253,10 @@ export interface SSROptions {
 		assets: string;
 	};
 	prefix: string;
-	prerender: boolean;
+	prerender: {
+		default: boolean;
+		enabled: boolean;
+	};
 	read(file: string): Buffer;
 	root: SSRComponent['default'];
 	router: boolean;
@@ -308,7 +310,7 @@ export interface SSRState {
 	getClientAddress: () => string;
 	initiator?: SSRPage | null;
 	platform?: any;
-	prerender?: PrerenderOptions;
+	prerendering?: PrerenderOptions;
 }
 
 export type StrictBody = string | Uint8Array;