diff --git a/.changeset/orange-peas-see.md b/.changeset/orange-peas-see.md new file mode 100644 index 000000000000..8cb8fdc2c5fb --- /dev/null +++ b/.changeset/orange-peas-see.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': minor +--- + +feat: throw redirects from commands diff --git a/packages/kit/src/exports/internal/shared.js b/packages/kit/src/exports/internal/shared.js index 1be42191963b..9bfb0dd0bc7d 100644 --- a/packages/kit/src/exports/internal/shared.js +++ b/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; } } diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index dcc3c53aee61..285f129f8ff5 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -680,6 +680,9 @@ async function _preload_code(url) { } } +/** @type {WeakSet} */ +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'); diff --git a/packages/kit/src/runtime/client/remote-functions/command.svelte.js b/packages/kit/src/runtime/client/remote-functions/command.svelte.js index a2bdc81a3cd6..0f5455a9f44f 100644 --- a/packages/kit/src/runtime/client/remote-functions/command.svelte.js +++ b/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()); diff --git a/packages/kit/src/runtime/client/remote-functions/form.svelte.js b/packages/kit/src/runtime/client/remote-functions/form.svelte.js index a83a1e0d1b08..5b8dbbe5caad 100644 --- a/packages/kit/src/runtime/client/remote-functions/form.svelte.js +++ b/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; 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 117740d5ce21..2bf7a7950c96 100644 --- a/packages/kit/src/runtime/client/remote-functions/prerender.svelte.js +++ b/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 diff --git a/packages/kit/src/runtime/client/remote-functions/query-batch.svelte.js b/packages/kit/src/runtime/client/remote-functions/query-batch.svelte.js index 916bfa0d3bbb..b7a5d0b63919 100644 --- a/packages/kit/src/runtime/client/remote-functions/query-batch.svelte.js +++ b/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; diff --git a/packages/kit/src/runtime/client/remote-functions/query/index.js b/packages/kit/src/runtime/client/remote-functions/query/index.js index 11ec99aa4bc2..fb374706123e 100644 --- a/packages/kit/src/runtime/client/remote-functions/query/index.js +++ b/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() }); }); }; diff --git a/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js b/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js index 6a86b5134746..af43499dbb69 100644 --- a/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js +++ b/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 @@ export class Query { 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; 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 bad25b47736c..7fda8a20ffe7 100644 --- a/packages/kit/src/runtime/client/remote-functions/shared.svelte.js +++ b/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; } diff --git a/packages/kit/src/runtime/server/remote.js b/packages/kit/src/runtime/server/remote.js index 787690990db5..eb71b4e7e680 100644 --- a/packages/kit/src/runtime/server/remote.js +++ b/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 } diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts index 096593aede22..c14049587564 100644 --- a/packages/kit/src/types/internal.d.ts +++ b/packages/kit/src/types/internal.d.ts @@ -326,8 +326,6 @@ export type RemoteFunctionData = { f?: Record; /** Whether there were any refreshes/reconnects during the request */ r?: true; - /** The redirect location, if any */ - redirect?: string; }; export type RemoteFunctionResponse = diff --git a/packages/kit/test/apps/async/src/routes/remote/batch-redirect/+page.svelte b/packages/kit/test/apps/async/src/routes/remote/batch-redirect/+page.svelte index 13a7a19ee58f..b0b51113f5ef 100644 --- a/packages/kit/test/apps/async/src/routes/remote/batch-redirect/+page.svelte +++ b/packages/kit/test/apps/async/src/routes/remote/batch-redirect/+page.svelte @@ -1,4 +1,5 @@ diff --git a/packages/kit/test/apps/async/src/routes/remote/redirect-external/+page.svelte b/packages/kit/test/apps/async/src/routes/remote/redirect-external/+page.svelte index c7ac758d27bf..3fb58c5c034b 100644 --- a/packages/kit/test/apps/async/src/routes/remote/redirect-external/+page.svelte +++ b/packages/kit/test/apps/async/src/routes/remote/redirect-external/+page.svelte @@ -1,19 +1,12 @@ -

{status}

+

{await redirectOutsideApp(condition)}

diff --git a/packages/kit/test/apps/async/src/routes/remote/redirect-external/redirect.remote.js b/packages/kit/test/apps/async/src/routes/remote/redirect-external/redirect.remote.js index f2ded88795f5..6e31ec038d58 100644 --- a/packages/kit/test/apps/async/src/routes/remote/redirect-external/redirect.remote.js +++ b/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'; }); diff --git a/packages/kit/test/apps/async/test/client.test.js b/packages/kit/test/apps/async/test/client.test.js index 30474d7d8137..69b71dde51e2 100644 --- a/packages/kit/test/apps/async/test/client.test.js +++ b/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 }) => {