fix: don't redirect in forks

packages/kit/src/core/sync/write_root.js

@@ -25,13 +25,19 @@ export function write_root(manifest_data, output) {
 			<!-- This file is generated by @sveltejs/kit — do not edit it! -->
 			<svelte:options runes={true} />
 			<script>
+				import { setContext } from 'svelte';
 				import { afterNavigate } from '$app/navigation';
+				import { browser } from '$app/env';
 
-				let { page, constructors, components = [], form, errors = [], error, ${levels
+				let { page, constructors, components = [], form, fork, errors = [], error, ${levels
 					.map((l) => `data_${l} = null`)
 					.join(', ')} } = $props();
 				let data = $derived({${levels.map((l) => `'${l}': data_${l}`).join(', ')}});
 
+				if (browser) {
+					setContext('__sveltekit_fork', () => fork);
+				}
+
 				let mounted = $state(false);
 				let navigated = $state(false);
 				let title = $state(null);

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

@@ -640,10 +640,34 @@ async function _preload_data(intent) {
 				// resolve, bail rather than creating an orphan fork
 				if (lc === load_cache && result.type === 'loaded') {
 					try {
-						return svelte.fork(() => {
-							root.$set(result.props);
+						// We gotta create a fork facade so that we can put something into the props
+						// the moment the fork callback is run, before the fork object is created.
+						let committed = false;
+						let discarded = false;
+						/** @type {any} */
+						const fork = {
+							// TODO have this in fork API?
+							get committed() {
+								return committed;
+							},
+							get discarded() {
+								return discarded;
+							}
+						};
+						const f = svelte.fork(() => {
+							root.$set({ ...result.props, fork });
 							update(result.props.page);
 						});
+						fork.commit = () => {
+							committed = true;
+							return f.commit();
+						};
+						fork.discard = () => {
+							discarded = true;
+							return f.discard();
+						};
+						fork.id = lc.id;
+						return fork;
 					} catch {
 						// if it errors, it's because the experimental flag isn't enabled in Svelte
 					}
@@ -2079,6 +2103,16 @@ if (import.meta.hot) {
 	});
 }
 
+/**
+ * @param {import('svelte').Fork} fork
+ * @param {string} location
+ */
+export async function redirect_fork(fork, location) {
+	if ((await load_cache?.fork) === fork && load_cache) {
+		load_cache.promise = Promise.resolve({ type: 'redirect', location });
+	}
+}
+
 /** @typedef {(typeof PRELOAD_PRIORITIES)['hover'] | (typeof PRELOAD_PRIORITIES)['tap']} PreloadDataPriority */
 
 /**

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

@@ -2,8 +2,15 @@
 import { app_dir, base } from '$app/paths/internal/client';
 import { version } from '$app/env';
 import * as devalue from 'devalue';
-import { app, _goto, handle_error, prerender_responses } from '../client.js';
-import { get_remote_request_headers, remote_request, unwrap_node } from './shared.svelte.js';
+import { app, handle_error, prerender_responses } from '../client.js';
+import {
+	get_remote_request_headers,
+	handle_remote_redirect,
+	is_in_effect,
+	register_fork,
+	remote_request,
+	unwrap_node
+} from './shared.svelte.js';
 import { create_remote_key, stringify_remote_arg } from '../../shared.js';
 import { noop } from '../../../utils/functions.js';
 import { HttpError } from '@sveltejs/kit/internal';
@@ -60,6 +67,14 @@ export function prerender(id) {
 		const payload = stringify_remote_arg(arg, app.hooks.transport);
 		const cache_key = create_remote_key(id, payload);
 
+		if (is_in_effect()) {
+			const release = register_fork(cache_key);
+
+			$effect.pre(() => () => {
+				release();
+			});
+		}
+
 		let resource = prerender_resources.get(cache_key)?.deref();
 
 		if (!resource) {
@@ -99,8 +114,7 @@ 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);
+					await handle_remote_redirect(cache_key, result.redirect);
 					return;
 				}
 

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

@@ -1,9 +1,15 @@
 /** @import { RemoteQueryFunction } from '@sveltejs/kit' */
 import { app_dir, base } from '$app/paths/internal/client';
-import { _goto, query_map } from '../../client.js';
-import { get_remote_request_headers, QUERY_FUNCTION_ID, remote_request } from '../shared.svelte.js';
+import { query_map } from '../../client.js';
+import {
+	get_remote_request_headers,
+	handle_remote_redirect,
+	QUERY_FUNCTION_ID,
+	remote_request
+} from '../shared.svelte.js';
 import { DEV } from 'esm-env';
 import { QueryProxy } from './proxy.js';
+import { create_remote_key } from '../../../shared.js';
 
 /**
  * @param {string} id
@@ -29,8 +35,7 @@ export function query(id) {
 			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 handle_remote_redirect(create_remote_key(id, payload), result.redirect);
 			}
 		});
 	};

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

@@ -1,9 +1,11 @@
 import { app, query_map } from '../../client.js';
 import {
+	is_in_effect,
 	pin_in_effect,
 	pin_while_resolving,
 	QUERY_OVERRIDE_KEY,
-	QUERY_RESOURCE_KEY
+	QUERY_RESOURCE_KEY,
+	register_fork_in_effect
 } from '../shared.svelte.js';
 import { create_remote_key, stringify_remote_arg } from '../../../shared.js';
 import { Query } from './instance.svelte.js';
@@ -45,6 +47,10 @@ export class QueryProxy {
 		);
 
 		cache.ref(this, entry, this.#id, this.#payload);
+
+		if (is_in_effect()) {
+			register_fork_in_effect(this.#key);
+		}
 	}
 
 	#get_cached_query() {

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

@@ -2,9 +2,16 @@
 /** @import { RemoteQueryUpdate } from '@sveltejs/kit' */
 /** @import { CacheEntry } from './cache.svelte.js' */
 import * as devalue from 'devalue';
-import { app, _goto, live_query_map, query_map, query_responses } from '../client.js';
+import {
+	app,
+	_goto,
+	live_query_map,
+	query_map,
+	query_responses,
+	redirect_fork
+} from '../client.js';
 import { HttpError, Redirect } from '@sveltejs/kit/internal';
-import { untrack } from 'svelte';
+import { getContext, untrack } from 'svelte';
 import { create_remote_key, split_remote_key } from '../../shared.js';
 import { navigating, page } from '../state.svelte.js';
 
@@ -15,6 +22,81 @@ export const QUERY_OVERRIDE_KEY = Symbol('sveltekit.query_override_key');
 /** Indicates a query instance */
 export const QUERY_RESOURCE_KEY = Symbol('sveltekit.query_resource_key');
 
+/** @typedef {import('svelte').Fork & { discarded: boolean; committed: boolean }} SvelteKitFork */
+
+/** @type {Map<string, Map<SvelteKitFork | null, number>>} */
+const forks_by_key = new Map();
+
+/**
+ * @returns {() => SvelteKitFork | null}
+ */
+function get_fork_context() {
+	try {
+		return getContext('__sveltekit_fork');
+	} catch {
+		return () => null;
+	}
+}
+
+/**
+ * @param {string} key
+ * @returns {() => void}
+ */
+export function register_fork(key) {
+	const get_fork = get_fork_context()();
+	const instances = forks_by_key.get(key) ?? new Map();
+
+	instances.set(get_fork, (instances.get(get_fork) ?? 0) + 1);
+	forks_by_key.set(key, instances);
+
+	return () => {
+		const current = forks_by_key.get(key);
+		if (!current) return;
+
+		const count = current.get(get_fork);
+		if (count === undefined) return;
+
+		if (count > 1) {
+			current.set(get_fork, count - 1);
+		} else {
+			current.delete(get_fork);
+		}
+
+		if (current.size === 0) {
+			forks_by_key.delete(key);
+		}
+	};
+}
+
+/**
+ * @param {string} key
+ * @param {string} location
+ */
+export async function handle_remote_redirect(key, location) {
+	const forks = forks_by_key.get(key) ?? new Map();
+	let target;
+
+	for (const fork of forks.keys()) {
+		if (!fork || fork.committed) {
+			await _goto(location, {}, 0);
+			throw new Redirect(307, location);
+		} else if (!fork.discarded) {
+			target = fork;
+		}
+	}
+
+	if (target) {
+		await redirect_fork(target, location);
+		// This request happened in a speculative fork and has been routed through the fork loader.
+		// Keep the promise pending to avoid turning the redirect into a render error in the current world.
+		// TODO this is a Svelte bug we need to fix that
+		return new Promise(() => {});
+	}
+
+	await _goto(location, {}, 0);
+	throw new Redirect(307, location);
+}
+
 /**
  * If we're inside a reactive context, pin a cache entry for as long as the
  * surrounding effect is alive. Without this, a transiently-referenced
@@ -48,8 +130,8 @@ export function pin_in_effect(cache_map, cache, id, payload) {
  * cache entry stays pinned for the lifetime of the awaited promise. Without
  * this, a proxy awaited outside any effect (e.g. in an event handler) could
  * be GC'd between the `.then` getter returning the thenable and the
- * underlying promise settling, causing the FinalizationRegistry to evict the
- * cache entry mid-flight and the awaited value to resolve to Svelte's
+ * underlying promise settling, causing the FinalizationRegistry to evict
+ * the cache entry mid-flight and the awaited value to resolve to Svelte's
  * `UNINITIALIZED` sentinel from a torn-down `$derived`.
  *
  * @template TResource
@@ -279,3 +361,30 @@ export function categorize_updates(updates) {
 
 	return { overrides, refreshes };
 }
+
+/**
+ * @returns {boolean} Returns `true` if we are in an effect
+ */
+export function is_in_effect() {
+	try {
+		$effect.pre(() => {});
+		return true;
+	} catch {
+		return false;
+	}
+}
+
+/**
+ * Registers a fork for the given key and ties its cleanup to the surrounding
+ * effect. Must be called from within a reactive context (i.e. when
+ * `is_in_effect()` returns `true`).
+ *
+ * @param {string} key
+ */
+export function register_fork_in_effect(key) {
+	const release = register_fork(key);
+
+	$effect.pre(() => () => {
+		release();
+	});
+}

packages/kit/test/apps/async/src/routes/remote/prerender/+page.svelte

@@ -8,6 +8,7 @@
 
 <a href="/remote/prerender/whole-page">whole-page</a>
 <a href="/remote/prerender/functions-only">functions-only</a>
+<a data-sveltekit-preload-data="hover" href="/remote/prerender/redirect">redirect</a>
 
 <button id="fetch-prerendered" onclick={async () => (prerendered_result = await prerendered())}>
 	{prerendered_result}

packages/kit/test/apps/async/src/routes/remote/prerender/redirect/+layout.svelte

@@ -0,0 +1,12 @@
+<script>
+	import { page } from '$app/state';
+	import { layout_redirect } from './redirect.remote.js';
+
+	let { children } = $props();
+</script>
+
+<p id="layout-prerender-query">
+	on page {await layout_redirect(page.url.pathname)} (== {page.url.pathname})
+</p>
+
+{@render children()}

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

@@ -0,0 +1 @@
+<p>should never see this</p>

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

@@ -0,0 +1,10 @@
+import { prerender } from '$app/server';
+import { redirect } from '@sveltejs/kit';
+
+export const layout_redirect = prerender('unchecked', (path) => {
+	if (path !== '/remote/prerender/redirect/redirected') {
+		redirect(307, '/remote/prerender/redirect/redirected');
+	}
+
+	return path;
+});

packages/kit/test/apps/async/src/routes/remote/prerender/redirect/redirected/+page.svelte

@@ -0,0 +1 @@
+<p id="redirected">redirected</p>

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

@@ -1,5 +1,4 @@
-<!-- TODO can remove this when preloading doesn't eagerly redirect -->
-<a data-sveltekit-preload-data={false} href="/remote/query-redirect/from-page">from page</a>
-<a data-sveltekit-preload-data={false} href="/remote/query-redirect/from-common-layout"
+<a data-sveltekit-preload-data="hover" href="/remote/query-redirect/from-page">from page</a>
+<a data-sveltekit-preload-data="hover" href="/remote/query-redirect/from-common-layout"
 	>from layout</a
 >

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

@@ -22,6 +22,38 @@ test.describe('remote functions', () => {
 		await expect(page.locator('p')).toHaveText('foobaz');
 	});
 
+	test('prerender preload does not eagerly navigate redirects', async ({ page }) => {
+		await page.goto('/remote/prerender');
+		await page.locator('a[href="/remote/prerender/redirect"]').hover();
+		await Promise.all([page.waitForTimeout(100), page.waitForLoadState('networkidle')]);
+
+		await expect(page).toHaveURL('/remote/prerender');
+
+		await page.click('a[href="/remote/prerender/redirect"]');
+		await expect(page.locator('#redirected')).toHaveText('redirected');
+		await expect(page.locator('#layout-prerender-query')).toHaveText(
+			'on page /remote/prerender/redirect/redirected (== /remote/prerender/redirect/redirected)'
+		);
+	});
+
+	test('query preload does not eagerly navigate redirects', async ({ page }) => {
+		await page.goto('/remote/query-redirect');
+
+		await page.locator('a[href="/remote/query-redirect/from-page"]').hover();
+		await Promise.all([page.waitForTimeout(100), page.waitForLoadState('networkidle')]);
+		await expect(page).toHaveURL('/remote/query-redirect');
+
+		await page.locator('a[href="/remote/query-redirect/from-common-layout"]').hover();
+		await Promise.all([page.waitForTimeout(100), page.waitForLoadState('networkidle')]);
+		await expect(page).toHaveURL('/remote/query-redirect');
+
+		await page.click('a[href="/remote/query-redirect/from-common-layout"]');
+		await expect(page.locator('#redirected')).toHaveText('redirected');
+		await expect(page.locator('#layout-query')).toHaveText(
+			'on page /remote/query-redirect/from-common-layout/redirected (== /remote/query-redirect/from-common-layout/redirected)'
+		);
+	});
+
 	test('remote query responses are not cacheable', async ({ page }) => {
 		// the query is kicked off during SSR but fetched by the client after
 		// hydration, so we can observe the response headers on the wire