Skip to content

fix: allow reroute to point to prerendered route #13575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 12, 2025
Merged
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/nine-glasses-build.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: allow reroute to point to prerendered route
6 changes: 2 additions & 4 deletions packages/kit/src/runtime/server/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as set_cookie_parser from 'set-cookie-parser';
import { respond } from './respond.js';
import * as paths from '__sveltekit/paths';
import { read_implementation } from '__sveltekit/server';
import { has_prerendered_path } from './utils.js';

/**
* @param {{
Expand Down Expand Up @@ -112,10 +113,7 @@ export function create_fetch({ event, options, manifest, state, get_cookie_heade
return await fetch(request);
}

if (
manifest._.prerendered_routes.has(decoded) ||
(decoded.at(-1) === '/' && manifest._.prerendered_routes.has(decoded.slice(0, -1)))
) {
if (has_prerendered_path(manifest, paths.base + decoded)) {
// The path of something prerendered could match a different route
// that is still in the manifest, leading to the wrong route being loaded.
// We therefore bail early here. The prerendered logic is different for
Expand Down
37 changes: 36 additions & 1 deletion packages/kit/src/runtime/server/respond.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ import { render_page } from './page/index.js';
import { render_response } from './page/render.js';
import { respond_with_error } from './page/respond_with_error.js';
import { is_form_content_type } from '../../utils/http.js';
import { handle_fatal_error, method_not_allowed, redirect_response } from './utils.js';
import {
handle_fatal_error,
has_prerendered_path,
method_not_allowed,
redirect_response
} from './utils.js';
import { decode_pathname, decode_params, disable_search, normalize_path } from '../../utils/url.js';
import { exec } from '../../utils/routing.js';
import { redirect_json_response, render_data } from './data/index.js';
Expand All @@ -21,6 +26,8 @@ import { get_public_env } from './env_module.js';
import { resolve_route } from './page/server_routing.js';
import { validateHeaders } from './validate-headers.js';
import {
add_data_suffix,
add_resolution_suffix,
has_data_suffix,
has_resolution_suffix,
strip_data_suffix,
Expand Down Expand Up @@ -191,6 +198,34 @@ export async function respond(request, options, manifest, state) {
return text('Malformed URI', { status: 400 });
}

if (
resolved_path !== url.pathname &&
!state.prerendering?.fallback &&
has_prerendered_path(manifest, resolved_path)
) {
const url = new URL(request.url);
url.pathname = is_data_request
? add_data_suffix(resolved_path)
: is_route_resolution_request
? add_resolution_suffix(resolved_path)
: resolved_path;

// `fetch` automatically decodes the body, so we need to delete the related headers to not break the response
// Also see https://github.com/sveltejs/kit/issues/12197 for more info (we should fix this more generally at some point)
const response = await fetch(url, request);
const headers = new Headers(response.headers);
if (headers.has('content-encoding')) {
headers.delete('content-encoding');
headers.delete('content-length');
}

return new Response(response.body, {
headers,
status: response.status,
statusText: response.statusText
});
}

/** @type {import('types').SSRRoute | null} */
let route = null;

Expand Down
12 changes: 12 additions & 0 deletions packages/kit/src/runtime/server/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,15 @@ export function stringify_uses(node) {

return `"uses":{${uses.join(',')}}`;
}

/**
* Returns `true` if the given path was prerendered
* @param {import('@sveltejs/kit').SSRManifest} manifest
* @param {string} pathname Should include the base and be decoded
*/
export function has_prerendered_path(manifest, pathname) {
return (
manifest._.prerendered_routes.has(pathname) ||
(pathname.at(-1) === '/' && manifest._.prerendered_routes.has(pathname.slice(0, -1)))
);
}
4 changes: 4 additions & 0 deletions packages/kit/test/apps/basics/src/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ export const reroute = ({ url, fetch }) => {
return fetch('/reroute/api').then((r) => r.text());
}

if (url.pathname === '/reroute/prerendered/to-destination') {
return '/reroute/prerendered/destination';
}

if (url.pathname in mapping) {
return mapping[url.pathname];
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<a href="/reroute/prerendered/to-destination">to prerendered page</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>reroute that points to prerendered page works</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const prerender = true;
6 changes: 6 additions & 0 deletions packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,7 @@
expect(page.locator('p.loadingfail')).toBeHidden();
});

test('Catches fetch errors from server load functions (direct hit)', async ({ page }) => {

Check warning on line 1268 in packages/kit/test/apps/basics/test/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit (22, ubuntu-latest, chromium)

retries: 2
page.goto('/streaming/server-error');
await expect(page.locator('p.eager')).toHaveText('eager');
await expect(page.locator('p.fail')).toHaveText('fail');
Expand Down Expand Up @@ -1461,6 +1461,12 @@
);
});

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']");
expect(await page.textContent('h1')).toContain('reroute that points to prerendered page works');
});

test('Apply reroute after client-only redirects', async ({ page }) => {
await page.goto('/reroute/client-only-redirect');
expect(await page.textContent('h1')).toContain('Successfully rewritten');
Expand Down
5 changes: 5 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,11 @@ test.describe('reroute', () => {
);
});

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');
});

test('Returns a 500 response if reroute throws an error on the server', async ({ page }) => {
const response = await page.goto('/reroute/error-handling/server-error');
expect(response?.status()).toBe(500);
Expand Down
Loading