breaking: throw redirects from remote functions

.changeset/orange-peas-see.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': minor
+---
+
+feat: throw redirects from commands

packages/kit/src/exports/internal/shared.js

@@ -22,12 +22,12 @@ export class HttpError {
 	}
 }
 
-export class Redirect {
+export class Redirect extends Error {
 	/**
 	 * @param {300 | 301 | 302 | 303 | 304 | 305 | 306 | 307 | 308} status
 	 * @param {string} location
 	 */
-	constructor(status, location) {
+	constructor(status, location, refresh = false) {
 		try {
 			new Headers({ location });
 		} catch {
@@ -37,8 +37,31 @@ export class Redirect {
 			);
 		}
 
+		const message = `
+			A redirect was thrown outside render. To navigate, catch the error and use \`goto\`:
+
+			import { isRedirect } from '@sveltejs/kit';
+			import { goto } from '$app/navigation';
+
+			try {
+				...
+			} catch (e) {
+				if (isRedirect(e)) {
+					goto(e.location);
+				} else {
+					throw e;
+				}
+			}
+		`;
+
+		super(message.replace(/^\t{3}/gm, '').trim());
+
 		this.status = status;
 		this.location = location;
+
+		// TODO this is only needed for `form`, so that we add `refreshAll: true` to
+		// the `goto` call in the `catch` clause. ideally it wouldn't be exposed
+		this.refresh = refresh;
 	}
 }
 

packages/kit/src/runtime/client/client.js

@@ -680,6 +680,9 @@ async function _preload_code(url) {
 	}
 }
 
+/** @type {WeakSet<App.Error>} */
+const transformed_errors = new WeakSet();
+
 /**
  * @param {import('./types.js').NavigationFinished} result
  * @param {HTMLElement} target
@@ -717,10 +720,31 @@ async function initialize(result, target, hydrate) {
 		// Svelte 5 specific: asynchronously instantiate the component, i.e. don't call flushSync
 		sync: false,
 		transformError: /** @param {unknown} e */ async (e) => {
+			if (typeof e === 'object' && transformed_errors.has(/** @type {App.Error} */ (e))) {
+				return e;
+			}
+
+			if (e instanceof Redirect) {
+				await _goto(
+					e.location,
+					{
+						refreshAll: e.refresh
+					},
+					0
+				);
+
+				await new Promise((f) => setTimeout(f, 1000));
+
+				transformed_errors.add(e);
+				return e;
+			}
+
 			const error = await handle_error(e, current.nav);
 			rendering_error = { error, status: error.status };
 			page.error = error;
 			page.status = rendering_error.status;
+
+			transformed_errors.add(error);
 			return error;
 		}
 	});
@@ -2229,6 +2253,10 @@ export async function handle_error(error, event) {
 		return error.body;
 	}
 
+	if (error instanceof Redirect) {
+		return error;
+	}
+
 	if (DEV) {
 		errored = true;
 		console.warn('The next HMR update will cause the page to reload');

packages/kit/src/runtime/client/remote-functions/command.svelte.js

@@ -54,12 +54,6 @@ export function command(id) {
 					headers
 				});
 
-				if (response.redirect) {
-					throw new Error(
-						'Redirects are not allowed in commands. Return a result instead and use goto on the client'
-					);
-				}
-
 				return response._;
 			} finally {
 				overrides?.forEach((fn) => fn());

packages/kit/src/runtime/client/remote-functions/form.svelte.js

@@ -3,7 +3,7 @@
 /** @import { InternalRemoteFormIssue } from 'types' */
 import { app_dir, base } from '$app/paths/internal/client';
 import { DEV } from 'esm-env';
-import { HttpError } from '@sveltejs/kit/internal';
+import { HttpError, Redirect } from '@sveltejs/kit/internal';
 import {
 	query_responses,
 	_goto,
@@ -206,7 +206,7 @@ export function form(id) {
 					}
 
 					const { blob } = serialize_binary_form(convert(form_data), {
-						remote_refreshes: Array.from(refreshes ?? [])
+						remote_refreshes: refreshes ? Array.from(refreshes) : undefined
 					});
 
 					const response = await remote_request(
@@ -225,26 +225,12 @@ export function form(id) {
 
 					({ issues: raw_issues = [], result } = response._ ?? {});
 
-					// if the developer took control of updates via `.updates(...)` (even with
-					// no arguments), or the server performed explicit refreshes, don't invalidateAll
-					const should_refresh = refreshes === null && !response.r;
-
-					if (response.redirect) {
-						// Use internal version to allow redirects to external URLs
-						void _goto(
-							response.redirect,
-							{
-								refreshAll: should_refresh
-							},
-							0
-						);
-						return true;
-					}
-
 					const succeeded = raw_issues.length === 0;
 
 					if (succeeded) {
-						if (should_refresh) {
+						// if the developer took control of updates via `.updates(...)` (even with
+						// no arguments), or the server performed explicit refreshes, don't refreshAll
+						if (!response.r) {
 							void refreshAll();
 						}
 					} else {
@@ -255,6 +241,17 @@ export function form(id) {
 
 					return succeeded;
 				} catch (e) {
+					if (e instanceof Redirect) {
+						// Use internal version to allow redirects to external URLs
+						void _goto(
+							e.location,
+							{
+								refreshAll: e.refresh
+							},
+							0
+						);
+					}
+
 					result = undefined;
 					raw_issues = [];
 					throw e;

packages/kit/src/runtime/client/remote-functions/prerender.svelte.js

@@ -98,12 +98,6 @@ export function prerender(id) {
 
 				const result = await remote_request(url, { headers });
 
-				if (result.redirect) {
-					// Use internal version to allow redirects to external URLs
-					void _goto(result.redirect, {}, 0);
-					return;
-				}
-
 				const data = result._;
 
 				// For successful prerender requests, save to cache

packages/kit/src/runtime/client/remote-functions/query-batch.svelte.js

@@ -49,21 +49,6 @@ export function query_batch(id) {
 							headers
 						});
 
-						if (response.redirect) {
-							// Use internal version to allow redirects to external URLs
-							await _goto(response.redirect, {}, 0);
-
-							// settle all batched promises (with `undefined`, like a redirect
-							// from a non-batched query) so that callers don't hang forever
-							for (const resolvers of batched.values()) {
-								for (const { resolve } of resolvers) {
-									resolve(undefined);
-								}
-							}
-
-							return;
-						}
-
 						const results = response._;
 						let i = 0;
 

packages/kit/src/runtime/client/remote-functions/query/index.js

@@ -26,12 +26,7 @@ export function query(id) {
 		return new QueryProxy(id, arg, async (payload) => {
 			const url = `${base}/${app_dir}/remote/${id}${payload ? `?payload=${payload}` : ''}`;
 
-			const result = await remote_request(url, { headers: get_remote_request_headers() });
-
-			if (result.redirect) {
-				// Use internal version to allow redirects to external URLs
-				await _goto(result.redirect, {}, 0);
-			}
+			await remote_request(url, { headers: get_remote_request_headers() });
 		});
 	};
 

packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js

@@ -1,5 +1,5 @@
 import { query_responses, handle_error } from '../../client.js';
-import { HttpError } from '@sveltejs/kit/internal';
+import { HttpError, Redirect } from '@sveltejs/kit/internal';
 import { QUERY_OVERRIDE_KEY } from '../shared.svelte.js';
 import { noop } from '../../../../utils/functions.js';
 import { tick, untrack } from 'svelte';
@@ -126,33 +126,20 @@
 
 				resolve(undefined);
 			})
-			.catch(async (e) => {
+			.catch((e) => {
 				// TODO: Our behavior here could be better:
-				// - We should not reject on redirects, but should hook into the router
-				//   to ensure the query is properly refreshed before the navigation completes
 				// - Instead of failing on transport-level errors, we should probably do what
 				//   LiveQuery does and preserve the last known good value and retry the connection
-				if (this.#latest.indexOf(resolve) === -1) return;
-
-				const error = await handle_error(e, {
-					params: {},
-					route: { id: null },
-					url: new URL(location.href)
-				});
-
-				// Re-check after the async `handle_error` gap: a later request may have
-				// resolved/rejected while we were awaiting and superseded this one, so
-				// recompute the index and bail out if this request is no longer current
 				const idx = this.#latest.indexOf(resolve);
 				if (idx === -1) return;
 
 				untrack(() => {
 					this.#latest.splice(0, idx).forEach((r) => r(undefined));
-					this.#error = error;
+					this.#error = e;
 					this.#loading = false;
 				});
 
-				reject(new HttpError(error.status, error)); // so that transformError doesn't transform it again
+				reject(e);
 			});
 
 		return promise;

packages/kit/src/runtime/client/remote-functions/shared.svelte.js

@@ -183,6 +183,10 @@ export async function remote_request(url, init) {
 		}
 	}
 
+	if (result.type === 'redirect') {
+		throw new Redirect(/** @type {300} */ (result.status), result.location, !data.r);
+	}
+
 	return data;
 }
 

packages/kit/src/runtime/server/remote.js

@@ -243,6 +243,10 @@ async function handle_remote_call_internal(event, state, options, manifest, id)
 				const { data: input, meta, form_data } = await deserialize_binary_form(event.request);
 				state.remote.requested = create_requested_map(meta.remote_refreshes);
 
+				if (meta.remote_refreshes) {
+					data.r = true;
+				}
+
 				// If this is a keyed form instance (created via form.for(key)), add the key to the form data (unless already set)
 				// Note that additional_args will only be set if the form is not enhanced, as enhanced forms transfer the key inside `data`.
 				if (additional_args && !('id' in input)) {
@@ -316,11 +320,13 @@ async function handle_remote_call_internal(event, state, options, manifest, id)
 		);
 	} catch (error) {
 		if (error instanceof Redirect) {
-			const data = await collect_remote_data({ redirect: error.location }, event, state, options);
+			const data = await collect_remote_data({}, event, state, options);
 
 			return json(
 				/** @type {RemoteFunctionResponse} */ ({
-					type: 'result',
+					type: 'redirect',
+					status: error.status,
+					location: error.location,
 					data: stringify(data, transport)
 				}),
 				{ headers }

packages/kit/src/types/internal.d.ts

@@ -326,8 +326,6 @@ export type RemoteFunctionData = {
 	f?: Record<string, RemoteFunctionDataNode>;
 	/** Whether there were any refreshes/reconnects during the request */
 	r?: true;
-	/** The redirect location, if any */
-	redirect?: string;
 };
 
 export type RemoteFunctionResponse =

packages/kit/test/apps/async/src/routes/remote/batch-redirect/+page.svelte

@@ -1,4 +1,5 @@
 <script>
+	import { isRedirect } from '@sveltejs/kit';
 	import { batch_redirect } from './data.remote.js';
 
 	let status = $state('idle');
@@ -9,8 +10,8 @@
 		try {
 			await batch_redirect('a');
 			status = 'resolved';
-		} catch {
-			status = 'rejected';
+		} catch (e) {
+			status = isRedirect(e) ? 'redirected' : 'rejected';
 		}
 	}
 </script>

packages/kit/test/apps/async/src/routes/remote/redirect-external/+page.svelte

@@ -1,19 +1,12 @@
 <script>
 	import { redirectOutsideApp } from './redirect.remote.js';
 
-	let status = $state('idle');
+	let condition = $state(false);
 
 	async function run() {
-		status = 'pending';
-
-		try {
-			await redirectOutsideApp();
-			status = 'resolved';
-		} catch {
-			status = 'rejected';
-		}
+		condition = true;
 	}
 </script>
 
 <button id="trigger" onclick={run}>trigger</button>
-<p id="status">{status}</p>
+<p id="status">{await redirectOutsideApp(condition)}</p>

packages/kit/test/apps/async/src/routes/remote/redirect-external/redirect.remote.js

@@ -4,7 +4,11 @@ import { redirect } from '@sveltejs/kit';
 // Regression for https://github.com/sveltejs/kit/issues/14285: a server-issued
 // redirect to a same-origin URL that is not a client route must still navigate,
 // rather than being rejected by the stricter `goto` behaviour.
-export const redirectOutsideApp = query(() => {
-	// `/robots.txt` is a real static asset but not a SvelteKit route
-	redirect(307, '/robots.txt');
+export const redirectOutsideApp = query('unchecked', (condition) => {
+	if (condition) {
+		// `/robots.txt` is a real static asset but not a SvelteKit route
+		redirect(307, '/robots.txt');
+	}
+
+	return 'idle';
 });

packages/kit/test/apps/async/test/client.test.js

@@ -543,8 +543,10 @@ test.describe('remote function mutations', () => {
 		await page.click('#trigger');
 
 		// the redirect must both navigate and settle the awaited query
-		await expect(page.locator('#status')).toHaveText('resolved');
-		expect(page.url()).toContain('#redirected');
+		await expect(page.locator('#status')).toHaveText('redirected');
+
+		// the query ran outside render, so should not cause a redirect
+		expect(page.url()).not.toContain('#redirected');
 	});
 
 	test('non-exported remote functions are never serialized into responses', async ({ page }) => {