diff --git a/.changeset/khaki-buttons-add.md b/.changeset/khaki-buttons-add.md new file mode 100644 index 000000000000..ba543c9fa850 --- /dev/null +++ b/.changeset/khaki-buttons-add.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: allow non-prerendered API endpoint calls during reroute when prerendering diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index 417fa8e9fb95..96f3913c01ed 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -29,7 +29,7 @@ export async function render_endpoint(event, mod, state) { throw new Error('Cannot prerender endpoints that have mutative methods'); } - if (state.prerendering && !prerender) { + if (state.prerendering && !state.prerendering.inside_reroute && !prerender) { if (state.depth > 0) { // if request came from a prerendered page, bail throw new Error(`${event.route.id} is not prerenderable`); @@ -41,7 +41,7 @@ export async function render_endpoint(event, mod, state) { } try { - let response = await with_event(event, () => + const response = await with_event(event, () => handler(/** @type {import('@sveltejs/kit').RequestEvent>} */ (event)) ); @@ -51,15 +51,28 @@ export async function render_endpoint(event, mod, state) { ); } - if (state.prerendering) { - // the returned Response might have immutable Headers - // so we should clone them before trying to mutate them - response = new Response(response.body, { + if (state.prerendering && (!state.prerendering.inside_reroute || prerender)) { + // The returned Response might have immutable Headers + // so we should clone them before trying to mutate them. + // We also need to clone the response body since it may be read twice during prerendering + const cloned = new Response(response.clone().body, { status: response.status, statusText: response.statusText, headers: new Headers(response.headers) }); - response.headers.set('x-sveltekit-prerender', String(prerender)); + cloned.headers.set('x-sveltekit-prerender', String(prerender)); + + if (state.prerendering.inside_reroute && prerender) { + // Without this, the route wouldn't be recorded as prerendered, + // because there's nothing after reroute that would do that. + cloned.headers.set( + 'x-sveltekit-routeid', + encodeURI(/** @type {string} */ (event.route.id)) + ); + state.prerendering.dependencies.set(event.url.pathname, { response: cloned, body: null }); + } else { + return cloned; + } } return response; diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 812ef3df7a69..81b30e0756a5 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -183,7 +183,12 @@ export async function respond(request, options, manifest, state) { let resolved_path; + const prerendering_reroute_state = state.prerendering?.inside_reroute; try { + // For the duration or a reroute, disable the prerendering state as reroute could call API endpoints + // which would end up in the wrong logic path if not disabled. + if (state.prerendering) state.prerendering.inside_reroute = true; + // reroute could alter the given URL, so we pass a copy resolved_path = (await options.hooks.reroute({ url: new URL(url), fetch: event.fetch })) ?? url.pathname; @@ -191,6 +196,8 @@ export async function respond(request, options, manifest, state) { return text('Internal Server Error', { status: 500 }); + } finally { + if (state.prerendering) state.prerendering.inside_reroute = prerendering_reroute_state; } try { @@ -349,7 +356,9 @@ export async function respond(request, options, manifest, state) { set_trailing_slash(trailing_slash); - if (state.prerendering && !state.prerendering.fallback) disable_search(url); + if (state.prerendering && !state.prerendering.fallback && !state.prerendering.inside_reroute) { + disable_search(url); + } const response = await with_event(event, () => options.hooks.handle({ diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts index 6f6c04b3f416..2d54b37ac145 100644 --- a/packages/kit/src/types/internal.d.ts +++ b/packages/kit/src/types/internal.d.ts @@ -216,6 +216,8 @@ export interface PrerenderOptions { cache?: string; // including this here is a bit of a hack, but it makes it easy to add fallback?: boolean; dependencies: Map; + /** True for the duration of a call to the `reroute` hook */ + inside_reroute?: boolean; } export type RecursiveRequired = { diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js index 89b90d39335d..2b21ad1bdc2b 100644 --- a/packages/kit/test/apps/basics/src/hooks.js +++ b/packages/kit/test/apps/basics/src/hooks.js @@ -31,6 +31,10 @@ export const reroute = ({ url, fetch }) => { return fetch('/reroute/api').then((r) => r.text()); } + if (url.pathname === '/reroute/async/c') { + return fetch('/reroute/api/prerendered').then((r) => r.text()); + } + if (url.pathname === '/reroute/prerendered/to-destination') { return '/reroute/prerendered/destination'; } diff --git a/packages/kit/test/apps/basics/src/routes/reroute/api/[prerendered]/+server.ts b/packages/kit/test/apps/basics/src/routes/reroute/api/[prerendered]/+server.ts new file mode 100644 index 000000000000..399f9b37ef88 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/reroute/api/[prerendered]/+server.ts @@ -0,0 +1,9 @@ +import { text } from '@sveltejs/kit'; + +// This is inside a route with a route segment to ensure that the route is marked as prerendered +// as part of reroute resolution even when no `entries` is given. +export const prerender = true; + +export function GET() { + return text('/reroute/async/b'); +} diff --git a/packages/kit/test/apps/basics/src/routes/reroute/async/+page.svelte b/packages/kit/test/apps/basics/src/routes/reroute/async/+page.svelte index a7a995fcde0c..b84f51ae69fa 100644 --- a/packages/kit/test/apps/basics/src/routes/reroute/async/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/reroute/async/+page.svelte @@ -1 +1,4 @@ Go to url that should be rewritten +Go to url that should be rewritten and its reroute api call prerendered diff --git a/packages/kit/test/apps/basics/src/routes/reroute/async/+page.ts b/packages/kit/test/apps/basics/src/routes/reroute/async/+page.ts new file mode 100644 index 000000000000..a2f7163f1f06 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/reroute/async/+page.ts @@ -0,0 +1,3 @@ +// through this we test that the crawler during prerendering does not fail on +// reroute calls that call non-prerendered endpoints. +export const prerender = true; diff --git a/packages/kit/test/apps/basics/svelte.config.js b/packages/kit/test/apps/basics/svelte.config.js index bca05e5376ee..d2193940f0ab 100644 --- a/packages/kit/test/apps/basics/svelte.config.js +++ b/packages/kit/test/apps/basics/svelte.config.js @@ -25,7 +25,13 @@ const config = { '/routing/prerendered/trailing-slash/never', '/routing/prerendered/trailing-slash/ignore' ], - handleHttpError: 'warn' + handleHttpError: ({ path, message }) => { + if (path.includes('/reroute/async')) { + throw new Error('shouldnt error on ' + path); + } + + console.warn(message); + } }, version: { diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index 84683d43ebcd..a4851fd773c2 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -1461,6 +1461,14 @@ test.describe('reroute', () => { ); }); + test('Apply async prerendered reroute during client side navigation', async ({ page }) => { + await page.goto('/reroute/async'); + await page.click("a[href='/reroute/async/c']"); + expect(await page.textContent('h1')).toContain( + 'Successfully rewritten, URL should still show a: /reroute/async/c' + ); + }); + test('Apply reroute to prerendered page during client side navigation', async ({ page }) => { await page.goto('/reroute/prerendered'); await page.click("a[href='/reroute/prerendered/to-destination']"); diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index 905c345a9681..eb8572489fb0 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -683,6 +683,13 @@ test.describe('reroute', () => { ); }); + test('Apply async prerendered reroute when directly accessing a page', async ({ page }) => { + await page.goto('/reroute/async/c'); + expect(await page.textContent('h1')).toContain( + 'Successfully rewritten, URL should still show a: /reroute/async/c' + ); + }); + test('Apply reroute to prerendered page when directly accessing a page', async ({ page }) => { await page.goto('/reroute/prerendered/to-destination'); expect(await page.textContent('h1')).toContain('reroute that points to prerendered page works');