From a276be1e36d431c709d25d94d943c83fcde42988 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 3 Apr 2026 22:13:44 +0200 Subject: [PATCH 1/8] fix: don't redirect in forks This prevents a redirect that a remote function could do from doing a navigation in a fork. It's implemented by putting the context into fork, pulling it out on remote function invocation to check if we're in a fork, put that into a map, and pull it out of there when a redirect occurs to check if the remote function is only called in context of that fork, and if so instead of redirecting we tell the SvelteKit router that this new route will redirect elsewhere. In the future we could follow that redirect and run it in the fork, but this is good enough for now and ties nicely into the current SvelteKit router. Fixes #14935 --- packages/kit/src/core/sync/write_root.js | 5 +- packages/kit/src/runtime/client/client.js | 39 +++++++- .../remote-functions/prerender.svelte.js | 17 +++- .../client/remote-functions/query.svelte.js | 37 +++---- .../client/remote-functions/shared.svelte.js | 97 ++++++++++++++++++- .../src/routes/remote/prerender/+page.svelte | 1 + .../remote/prerender/redirect/+layout.svelte | 12 +++ .../remote/prerender/redirect/+page.svelte | 1 + .../prerender/redirect/redirect.remote.js | 10 ++ .../redirect/redirected/+page.svelte | 1 + .../routes/remote/query-redirect/+page.svelte | 6 +- .../kit/test/apps/async/test/client.test.js | 32 ++++++ 12 files changed, 228 insertions(+), 30 deletions(-) create mode 100644 packages/kit/test/apps/async/src/routes/remote/prerender/redirect/+layout.svelte create mode 100644 packages/kit/test/apps/async/src/routes/remote/prerender/redirect/+page.svelte create mode 100644 packages/kit/test/apps/async/src/routes/remote/prerender/redirect/redirect.remote.js create mode 100644 packages/kit/test/apps/async/src/routes/remote/prerender/redirect/redirected/+page.svelte diff --git a/packages/kit/src/core/sync/write_root.js b/packages/kit/src/core/sync/write_root.js index 445c17e45dbc..dd091d447614 100644 --- a/packages/kit/src/core/sync/write_root.js +++ b/packages/kit/src/core/sync/write_root.js @@ -106,10 +106,13 @@ export function write_root(manifest_data, config, output) { ${ isSvelte5Plus() ? dedent` - let { stores, page, constructors, components = [], form, ${use_boundaries ? 'errors = [], error, ' : ''}${levels + let { stores, page, constructors, components = [], form, fork, ${use_boundaries ? 'errors = [], error, ' : ''}${levels .map((l) => `data_${l} = null`) .join(', ')} } = $props(); ${use_boundaries ? `let data = $derived({${levels.map((l) => `'${l}': data_${l}`).join(', ')}})` : ''} + if (browser) { + setContext('__sveltekit_fork', () => fork); + } ` : dedent` export let stores; diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 6d207bcc9ff5..9f88ebc2a58e 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -560,10 +560,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} */ + let fork = { + // TODO have this in fork API? + get committed() { + return committed; + }, + get discarded() { + return discarded; + } + }; + let 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 } @@ -627,6 +651,7 @@ async function initialize(result, target, hydrate) { // @ts-ignore Svelte 5 specific: transformError allows to transform errors before they are passed to boundaries transformError: __SVELTEKIT_EXPERIMENTAL_USE_TRANSFORM_ERROR__ ? /** @param {unknown} e */ async (e) => { + debugger; const error = await handle_error(e, current.nav); rendering_error = { error, status: get_status(e) }; page.error = error; @@ -1956,6 +1981,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 */ function setup_preload() { diff --git a/packages/kit/src/runtime/client/remote-functions/prerender.svelte.js b/packages/kit/src/runtime/client/remote-functions/prerender.svelte.js index 3ab177c090b9..709f7c257377 100644 --- a/packages/kit/src/runtime/client/remote-functions/prerender.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/prerender.svelte.js @@ -4,7 +4,12 @@ import { version } from '__sveltekit/environment'; import * as devalue from 'devalue'; import { DEV } from 'esm-env'; import { app, prerender_responses } from '../client.js'; -import { get_remote_request_headers, remote_request } from './shared.svelte.js'; +import { + get_remote_request_headers, + is_in_effect, + register_fork, + remote_request +} from './shared.svelte.js'; import { create_remote_key, stringify_remote_arg } from '../../shared.js'; // Initialize Cache API for prerender functions @@ -59,6 +64,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) { @@ -94,7 +107,7 @@ export function prerender(id) { } } - const encoded = await remote_request(url, headers); + const encoded = await remote_request(url, headers, cache_key); // For successful prerender requests, save to cache if (prerender_cache) { diff --git a/packages/kit/src/runtime/client/remote-functions/query.svelte.js b/packages/kit/src/runtime/client/remote-functions/query.svelte.js index e95b64cff5e9..74d5df654c0a 100644 --- a/packages/kit/src/runtime/client/remote-functions/query.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/query.svelte.js @@ -1,8 +1,14 @@ /** @import { RemoteQueryFunction } from '@sveltejs/kit' */ /** @import { RemoteFunctionResponse } from 'types' */ import { app_dir, base } from '$app/paths/internal/client'; -import { app, goto, query_map, query_responses } from '../client.js'; -import { get_remote_request_headers, remote_request } from './shared.svelte.js'; +import { app, query_map, query_responses } from '../client.js'; +import { + get_remote_request_headers, + handle_remote_redirect, + is_in_effect, + register_fork, + remote_request +} from './shared.svelte.js'; import * as devalue from 'devalue'; import { HttpError, Redirect } from '@sveltejs/kit/internal'; import { DEV } from 'esm-env'; @@ -19,18 +25,6 @@ import { create_remote_key, stringify_remote_arg, unfriendly_hydratable } from ' * }} RemoteQueryCacheEntry */ -/** - * @returns {boolean} Returns `true` if we are in an effect - */ -function is_in_effect() { - try { - $effect.pre(() => {}); - return true; - } catch { - return false; - } -} - /** * @param {string} id * @returns {RemoteQueryFunction} @@ -51,7 +45,7 @@ export function query(id) { const url = `${base}/${app_dir}/remote/${id}${payload ? `?payload=${payload}` : ''}`; const serialized = await unfriendly_hydratable(key, () => - remote_request(url, get_remote_request_headers()) + remote_request(url, get_remote_request_headers(), key) ); return devalue.parse(serialized, app.decoders); @@ -115,8 +109,7 @@ export function query_batch(id) { } if (result.type === 'redirect') { - await goto(result.location); - throw new Redirect(307, result.location); + return handle_remote_redirect(key, result.location); } const results = devalue.parse(result.result, app.decoders); @@ -261,6 +254,8 @@ export class Query { this.#loading = false; }); + if (e instanceof Redirect) this.#promise = null; // allow retries after redirects + reject(e); }); @@ -374,6 +369,8 @@ class QueryProxy { #payload; #fn; #active = true; + /** @type {(() => void) | null} */ + #release_fork = null; /** * Whether this proxy was created in a tracking context. * @readonly @@ -395,9 +392,15 @@ class QueryProxy { return; } + // A bit duplicative with #get_or_create_cache_entry but this way we can reuse + // register_fork in the remote prerender function, too. + this.#release_fork = register_fork(this._key); + const entry = this.#get_or_create_cache_entry(); $effect.pre(() => () => { + /** @type {() => void} */ (this.#release_fork)(); + const die = this.#release(entry); void tick().then(die); }); diff --git a/packages/kit/src/runtime/client/remote-functions/shared.svelte.js b/packages/kit/src/runtime/client/remote-functions/shared.svelte.js index eff2cfe00400..d4ed2413439b 100644 --- a/packages/kit/src/runtime/client/remote-functions/shared.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/shared.svelte.js @@ -2,11 +2,86 @@ /** @import { RemoteFunctionResponse } from 'types' */ /** @import { Query } from './query.svelte.js' */ import * as devalue from 'devalue'; -import { app, goto, query_map } from '../client.js'; +import { app, goto, query_map, redirect_fork } from '../client.js'; import { HttpError, Redirect } from '@sveltejs/kit/internal'; -import { untrack } from 'svelte'; +import { getContext, untrack } from 'svelte'; import { navigating, page } from '../state.svelte.js'; +/** @typedef {import('svelte').Fork & { discarded: boolean; committed: boolean }} SvelteKitFork */ + +/** @type {Map>} */ +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); + 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); + throw new Redirect(307, location); +} + /** * @returns {{ 'x-sveltekit-pathname': string, 'x-sveltekit-search': string }} */ @@ -27,8 +102,9 @@ export function get_remote_request_headers() { /** * @param {string} url * @param {HeadersInit} headers + * @param {string} key */ -export async function remote_request(url, headers) { +export async function remote_request(url, headers, key) { const response = await fetch(url, { headers: { 'Content-Type': 'application/json', @@ -43,8 +119,7 @@ export async function remote_request(url, headers) { const result = /** @type {RemoteFunctionResponse} */ (await response.json()); if (result.type === 'redirect') { - await goto(result.location); - throw new Redirect(307, result.location); + return handle_remote_redirect(key, result.location); } if (result.type === 'error') { @@ -84,3 +159,15 @@ export function refresh_queries(stringified_refreshes, updates = []) { entry?.resource.set(value); } } + +/** + * @returns {boolean} Returns `true` if we are in an effect + */ +export function is_in_effect() { + try { + $effect.pre(() => {}); + return true; + } catch { + return false; + } +} diff --git a/packages/kit/test/apps/async/src/routes/remote/prerender/+page.svelte b/packages/kit/test/apps/async/src/routes/remote/prerender/+page.svelte index 0497af9b6749..82e9e4c2cb83 100644 --- a/packages/kit/test/apps/async/src/routes/remote/prerender/+page.svelte +++ b/packages/kit/test/apps/async/src/routes/remote/prerender/+page.svelte @@ -8,6 +8,7 @@ whole-page functions-only +redirect