Skip to content

Commit 20d783c

Browse files
committed
fix: don't redirect in forks
This prevents a redirect that a remote function could do from doing a navigation in a fork. Leverages Svelte's new forking methods from sveltejs/svelte#17217 It's implemented by putting the context into fork, pulling it out on remote function invocation to check if we're in a fork (TODO what if you have it invoked elsewhere and you just await it in a fork context?), 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
1 parent ab37849 commit 20d783c

File tree

5 files changed

+87
-15
lines changed

5 files changed

+87
-15
lines changed

.changeset/orange-mice-bet.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
fix: don't redirect in forks

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,12 @@ export function write_root(manifest_data, output) {
7272
${
7373
isSvelte5Plus()
7474
? dedent`
75-
let { stores, page, constructors, components = [], form, ${levels
75+
let { stores, page, constructors, components = [], fork, form, ${levels
7676
.map((l) => `data_${l} = null`)
7777
.join(', ')} } = $props();
78+
if (browser) {
79+
setContext('__sveltekit_fork', () => fork);
80+
}
7881
`
7982
: dedent`
8083
export let stores;

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

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,9 @@ const preload_tokens = new Set();
282282
export let pending_invalidate;
283283

284284
/**
285-
* @type {Map<string, {count: number, resource: any}>}
285+
* @type {Map<string, {count: number, resource: any, forks: Map<import('svelte').Fork | undefined, number>}>}
286286
* A map of id -> query info with all queries that currently exist in the app.
287+
* `forks` is a Map of every fork (or undefined when in the current world) and how often they're used in that context.
287288
*/
288289
export const query_map = new Map();
289290

@@ -540,10 +541,21 @@ async function _preload_data(intent) {
540541
// resolve, bail rather than creating an orphan fork
541542
if (lc === load_cache && result.type === 'loaded') {
542543
try {
543-
return svelte.fork(() => {
544-
root.$set(result.props);
545-
update(result.props.page);
546-
});
544+
let fork = svelte.fork(() => {});
545+
if (!fork.run) {
546+
// backwards compatibility for Svelte versions that don't have `fork.run`
547+
fork.discard();
548+
return svelte.fork(() => {
549+
root.$set({ ...result.props, fork });
550+
update(result.props.page);
551+
});
552+
} else {
553+
fork.run(() => {
554+
root.$set({ ...result.props, fork });
555+
update(result.props.page);
556+
});
557+
return fork;
558+
}
547559
} catch {
548560
// if it errors, it's because the experimental flag isn't enabled
549561
}
@@ -1855,6 +1867,16 @@ if (import.meta.hot) {
18551867
});
18561868
}
18571869

1870+
/**
1871+
* @param {import('svelte').Fork} fork
1872+
* @param {string} location
1873+
*/
1874+
export async function redirect_fork(fork, location) {
1875+
if ((await load_cache?.fork) === fork && load_cache) {
1876+
load_cache.promise = Promise.resolve({ type: 'redirect', location });
1877+
}
1878+
}
1879+
18581880
/** @typedef {(typeof PRELOAD_PRIORITIES)['hover'] | (typeof PRELOAD_PRIORITIES)['tap']} PreloadDataPriority */
18591881

18601882
function setup_preload() {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@ export function query(id) {
2323
}
2424
}
2525

26-
return create_remote_function(id, (cache_key, payload) => {
26+
return create_remote_function(id, (cache_key, payload, forks) => {
2727
return new Query(cache_key, async () => {
2828
if (Object.hasOwn(remote_responses, cache_key)) {
2929
return remote_responses[cache_key];
3030
}
3131

3232
const url = `${base}/${app_dir}/remote/${id}${payload ? `?payload=${payload}` : ''}`;
3333

34-
const result = await remote_request(url);
34+
const result = await remote_request(url, forks);
3535
return devalue.parse(result, app.decoders);
3636
});
3737
});

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

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,17 @@
22
/** @import { RemoteFunctionResponse } from 'types' */
33
/** @import { Query } from './query.svelte.js' */
44
import * as devalue from 'devalue';
5-
import { app, goto, query_map, remote_responses } from '../client.js';
5+
import { app, goto, redirect_fork, query_map, remote_responses } from '../client.js';
66
import { HttpError, Redirect } from '@sveltejs/kit/internal';
7-
import { tick } from 'svelte';
7+
import { getContext, tick } from 'svelte';
88
import { create_remote_key, stringify_remote_arg } from '../../shared.js';
99

1010
/**
1111
*
1212
* @param {string} url
13+
* @param {Map<import('svelte').Fork | undefined, number>} forks
1314
*/
14-
export async function remote_request(url) {
15+
export async function remote_request(url, forks = new Map()) {
1516
const response = await fetch(url, {
1617
headers: {
1718
// TODO in future, when we support forking, we will likely need
@@ -29,7 +30,27 @@ export async function remote_request(url) {
2930
const result = /** @type {RemoteFunctionResponse} */ (await response.json());
3031

3132
if (result.type === 'redirect') {
32-
await goto(result.location);
33+
if (
34+
forks.size === 0 ||
35+
Array.from(forks.keys()).some(
36+
(fork) =>
37+
!fork ||
38+
!fork.isDiscarded || // isDiscarded et al was introduced later, do this for backwards compatibility
39+
fork.isCommitted()
40+
)
41+
) {
42+
// If this query is used in at least one non-forked context,
43+
// it means it's part of the current world, therefore perform a regular redirect
44+
await goto(result.location);
45+
} else {
46+
for (const fork of /** @type {MapIterator<import('svelte').Fork>} */ (forks.keys())) {
47+
if (!fork.isDiscarded()) {
48+
redirect_fork(fork, result.location);
49+
break; // there can only be one current fork
50+
}
51+
}
52+
}
53+
3354
throw new Redirect(307, result.location);
3455
}
3556

@@ -43,22 +64,41 @@ export async function remote_request(url) {
4364
/**
4465
* Client-version of the `query`/`prerender`/`cache` function from `$app/server`.
4566
* @param {string} id
46-
* @param {(key: string, args: string) => any} create
67+
* @param {(key: string, args: string, forks: Map<import('svelte').Fork | undefined, number>) => any} create
4768
*/
4869
export function create_remote_function(id, create) {
4970
return (/** @type {any} */ arg) => {
71+
/** @type {import('svelte').Fork | undefined} */
72+
let fork = undefined;
73+
try {
74+
fork = getContext('__sveltekit_fork')?.();
75+
} catch {
76+
// not called in a reactive context
77+
}
78+
5079
const payload = stringify_remote_arg(arg, app.hooks.transport);
5180
const cache_key = create_remote_key(id, payload);
5281
let entry = query_map.get(cache_key);
5382

5483
let tracking = true;
5584
try {
5685
$effect.pre(() => {
57-
if (entry) entry.count++;
86+
if (entry) {
87+
entry.count++;
88+
entry.forks.set(fork, (entry.forks.get(fork) ?? 0) + 1);
89+
}
5890
return () => {
5991
const entry = query_map.get(cache_key);
6092
if (entry) {
6193
entry.count--;
94+
95+
const fork_count = /** @type {number} */ (entry.forks.get(fork)) - 1;
96+
if (fork_count === 0) {
97+
entry.forks.delete(fork);
98+
} else {
99+
entry.forks.set(fork, fork_count);
100+
}
101+
62102
void tick().then(() => {
63103
if (!entry.count && entry === query_map.get(cache_key)) {
64104
query_map.delete(cache_key);
@@ -74,7 +114,8 @@ export function create_remote_function(id, create) {
74114

75115
let resource = entry?.resource;
76116
if (!resource) {
77-
resource = create(cache_key, payload);
117+
const forks = new Map([[fork, 1]]);
118+
resource = create(cache_key, payload, forks);
78119

79120
Object.defineProperty(resource, '_key', {
80121
value: cache_key
@@ -84,6 +125,7 @@ export function create_remote_function(id, create) {
84125
cache_key,
85126
(entry = {
86127
count: tracking ? 1 : 0,
128+
forks,
87129
resource
88130
})
89131
);

0 commit comments

Comments
 (0)