Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/orange-mice-bet.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: don't redirect in forks
5 changes: 4 additions & 1 deletion packages/kit/src/core/sync/write_root.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,12 @@ export function write_root(manifest_data, output) {
${
isSvelte5Plus()
? dedent`
let { stores, page, constructors, components = [], form, ${levels
let { stores, page, constructors, components = [], fork, form, ${levels
.map((l) => `data_${l} = null`)
.join(', ')} } = $props();
if (browser) {
setContext('__sveltekit_fork', () => fork);
}
`
: dedent`
export let stores;
Expand Down
32 changes: 27 additions & 5 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,9 @@
export let pending_invalidate;

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

Expand Down Expand Up @@ -540,10 +541,21 @@
// resolve, bail rather than creating an orphan fork
if (lc === load_cache && result.type === 'loaded') {
try {
return svelte.fork(() => {
root.$set(result.props);
update(result.props.page);
});
let fork = svelte.fork(() => {});

Check failure on line 544 in packages/kit/src/runtime/client/client.js

View workflow job for this annotation

GitHub Actions / lint-all

'fork' is never reassigned. Use 'const' instead
if (!fork.run) {
// backwards compatibility for Svelte versions that don't have `fork.run`
fork.discard();
return svelte.fork(() => {
root.$set({ ...result.props, fork });
update(result.props.page);
});
} else {
fork.run(() => {
root.$set({ ...result.props, fork });
update(result.props.page);
});
return fork;
}
} catch {
// if it errors, it's because the experimental flag isn't enabled
}
Expand Down Expand Up @@ -1855,6 +1867,16 @@
});
}

/**
* @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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ export function query(id) {
}
}

return create_remote_function(id, (cache_key, payload) => {
return create_remote_function(id, (cache_key, payload, forks) => {
return new Query(cache_key, async () => {
if (Object.hasOwn(remote_responses, cache_key)) {
return remote_responses[cache_key];
}

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

const result = await remote_request(url);
const result = await remote_request(url, forks);
return devalue.parse(result, app.decoders);
});
});
Expand Down
56 changes: 49 additions & 7 deletions packages/kit/src/runtime/client/remote-functions/shared.svelte.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@
/** @import { RemoteFunctionResponse } from 'types' */
/** @import { Query } from './query.svelte.js' */
import * as devalue from 'devalue';
import { app, goto, query_map, remote_responses } from '../client.js';
import { app, goto, redirect_fork, query_map, remote_responses } from '../client.js';
import { HttpError, Redirect } from '@sveltejs/kit/internal';
import { tick } from 'svelte';
import { getContext, tick } from 'svelte';
import { create_remote_key, stringify_remote_arg } from '../../shared.js';

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

if (result.type === 'redirect') {
await goto(result.location);
if (
forks.size === 0 ||
Array.from(forks.keys()).some(
(fork) =>
!fork ||
!fork.isDiscarded || // isDiscarded et al was introduced later, do this for backwards compatibility
fork.isCommitted()
)
) {
// If this query is used in at least one non-forked context,
// it means it's part of the current world, therefore perform a regular redirect
await goto(result.location);
} else {
for (const fork of /** @type {MapIterator<import('svelte').Fork>} */ (forks.keys())) {
if (!fork.isDiscarded()) {
redirect_fork(fork, result.location);

Check failure on line 48 in packages/kit/src/runtime/client/remote-functions/shared.svelte.js

View workflow job for this annotation

GitHub Actions / lint-all

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
break; // there can only be one current fork
}
}
}

throw new Redirect(307, result.location);
}

Expand All @@ -43,22 +64,41 @@
/**
* Client-version of the `query`/`prerender`/`cache` function from `$app/server`.
* @param {string} id
* @param {(key: string, args: string) => any} create
* @param {(key: string, args: string, forks: Map<import('svelte').Fork | undefined, number>) => any} create
*/
export function create_remote_function(id, create) {
return (/** @type {any} */ arg) => {
/** @type {import('svelte').Fork | undefined} */
let fork = undefined;
try {
fork = getContext('__sveltekit_fork')?.();
} catch {
// not called in a reactive context
}

const payload = stringify_remote_arg(arg, app.hooks.transport);
const cache_key = create_remote_key(id, payload);
let entry = query_map.get(cache_key);

let tracking = true;
try {
$effect.pre(() => {
if (entry) entry.count++;
if (entry) {
entry.count++;
entry.forks.set(fork, (entry.forks.get(fork) ?? 0) + 1);
}
return () => {
const entry = query_map.get(cache_key);
if (entry) {
entry.count--;

const fork_count = /** @type {number} */ (entry.forks.get(fork)) - 1;
if (fork_count === 0) {
entry.forks.delete(fork);
} else {
entry.forks.set(fork, fork_count);
}

void tick().then(() => {
if (!entry.count && entry === query_map.get(cache_key)) {
query_map.delete(cache_key);
Expand All @@ -74,7 +114,8 @@

let resource = entry?.resource;
if (!resource) {
resource = create(cache_key, payload);
const forks = new Map([[fork, 1]]);
resource = create(cache_key, payload, forks);

Object.defineProperty(resource, '_key', {
value: cache_key
Expand All @@ -84,6 +125,7 @@
cache_key,
(entry = {
count: tracking ? 1 : 0,
forks,
resource
})
);
Expand Down
Loading