Skip to content

Commit

Permalink
fix: skip calling respond for server-side fetch on prerendered pa…
Browse files Browse the repository at this point in the history
…ges (#13377)

When using server-side fetch for internal requests, if a server route
is matched from the server `manifest.js`, it gets called without making
a real HTTP request.

However, prerendered routes are not included on this list! This is fine
when routes are prerendered with `export const prerender = true;`, but
will cause issues with a non-prerendered route also matches the same
URL as any prerendered routes.

This commit adds `prerendered_routes: Set<string>` to the `manifest.js`,
which skips calling the non-prerendered route.

Fixes #12778
Fixes #12739
  • Loading branch information
aloisklink authored Jan 30, 2025
1 parent f30352f commit d62ed39
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/nine-llamas-fetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: skip hooks for server fetch to prerendered routes
5 changes: 5 additions & 0 deletions .changeset/slow-penguins-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: default server fetch to use prerendered paths
2 changes: 2 additions & 0 deletions packages/kit/src/core/adapt/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export function create_builder({
generateManifest: ({ relativePath }) =>
generate_manifest({
build_data,
prerendered: [],
relative_path: relativePath,
routes: Array.from(filtered)
})
Expand Down Expand Up @@ -185,6 +186,7 @@ export function create_builder({
generateManifest({ relativePath, routes: subset }) {
return generate_manifest({
build_data,
prerendered: prerendered.paths,
relative_path: relativePath,
routes: subset
? subset.map((route) => /** @type {import('types').RouteData} */ (lookup.get(route)))
Expand Down
4 changes: 3 additions & 1 deletion packages/kit/src/core/generate_manifest/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import { find_server_assets } from './find_server_assets.js';
* build process, to power routing, etc.
* @param {{
* build_data: import('types').BuildData;
* prerendered: string[];
* relative_path: string;
* routes: import('types').RouteData[];
* }} opts
*/
export function generate_manifest({ build_data, relative_path, routes }) {
export function generate_manifest({ build_data, prerendered, relative_path, routes }) {
/**
* @type {Map<any, number>} The new index of each node in the filtered nodes array
*/
Expand Down Expand Up @@ -113,6 +114,7 @@ export function generate_manifest({ build_data, relative_path, routes }) {
`;
}).filter(Boolean).join(',\n')}
],
prerendered_routes: new Set(${s(prerendered)}),
matchers: async () => {
${Array.from(
matchers,
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1297,6 +1297,7 @@ export interface SSRManifest {
client: NonNullable<BuildData['client']>;
nodes: SSRNodeLoader[];
routes: SSRRoute[];
prerendered_routes: Set<string>;
matchers: () => Promise<Record<string, ParamMatcher>>;
/** A `[file]: size` map of all assets imported by server code */
server_assets: Record<string, number>;
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/exports/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ export async function dev(vite, vite_config, svelte_config) {
return result;
};
}),
prerendered_routes: new Set(),
routes: compact(
manifest_data.routes.map((route) => {
if (!route.page && !route.endpoint) return null;
Expand Down
3 changes: 3 additions & 0 deletions packages/kit/src/exports/vite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,7 @@ Tips:
manifest_path,
`export const manifest = ${generate_manifest({
build_data,
prerendered: [],
relative_path: '.',
routes: manifest_data.routes
})};\n`
Expand Down Expand Up @@ -917,6 +918,7 @@ Tips:
manifest_path,
`export const manifest = ${generate_manifest({
build_data,
prerendered: [],
relative_path: '.',
routes: manifest_data.routes
})};\n`
Expand Down Expand Up @@ -948,6 +950,7 @@ Tips:
`${out}/server/manifest.js`,
`export const manifest = ${generate_manifest({
build_data,
prerendered: prerendered.paths,
relative_path: '.',
routes: manifest_data.routes.filter((route) => prerender_map.get(route.id) !== true)
})};\n`
Expand Down
12 changes: 12 additions & 0 deletions packages/kit/src/runtime/server/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,18 @@ 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)))
) {
// 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
// each adapter, (except maybe for prerendered redirects)
// so we need to make an actual HTTP request.
return await fetch(request);
}

if (credentials !== 'omit') {
const cookie = get_cookie_header(url, request.headers.get('cookie'));
if (cookie) {
Expand Down
10 changes: 10 additions & 0 deletions packages/kit/test/apps/basics/src/hooks.server.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { building, dev } from '$app/environment';
import { error, isHttpError, redirect } from '@sveltejs/kit';
import { sequence } from '@sveltejs/kit/hooks';
import fs from 'node:fs';
Expand Down Expand Up @@ -136,6 +137,15 @@ export const handle = sequence(

return resolve(event);
},
async ({ event, resolve }) => {
if (!dev && !building && event.url.pathname === '/prerendering/prerendered-endpoint/api') {
error(
500,
`Server hooks should not be called for prerendered endpoints: isSubRequest=${event.isSubRequest}`
);
}
return resolve(event);
},
async ({ event, resolve }) => {
if (['/non-existent-route', '/non-existent-route-loop'].includes(event.url.pathname)) {
event.locals.url = new URL(event.request.url);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { building, dev } from '$app/environment';
import { error, json } from '@sveltejs/kit';

export const prerender = 'auto';

export function entries() {
return [
{
option: 'prerendered'
}
];
}

export async function GET({ params: { option } }) {
if ((await entries()).find((entry) => entry.option === option)) {
if (dev || building) {
return json({ message: 'Im prerendered and called from a non-prerendered +page.server.js' });
} else {
error(500, 'I should not be called at runtime because I am prerendered');
}
}
return json({ message: 'Im not prerendered' });
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
export async function GET({ fetch }) {
export async function GET({ fetch, url }) {
if (url.searchParams.get('api-with-param-option') === 'prerendered') {
return await fetch('/prerendering/prerendered-endpoint/api-with-param/prerendered');
}

return await fetch('/prerendering/prerendered-endpoint/api');
}
21 changes: 21 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,27 @@ test.describe('Endpoints', () => {
});
});

test('Partially Prerendered +server.js called from a non-prerendered +server.js works', async ({
baseURL
}) => {
for (const [description, url] of [
['direct', `${baseURL}/prerendering/prerendered-endpoint/api-with-param/prerendered`],
[
'proxied',
`${baseURL}/prerendering/prerendered-endpoint/proxy?api-with-param-option=prerendered`
]
]) {
await test.step(description, async () => {
const res = await fetch(url);

expect(res.status).toBe(200);
expect(await res.json()).toStrictEqual({
message: 'Im prerendered and called from a non-prerendered +page.server.js'
});
});
}
});

test('invalid request method returns allow header', async ({ request }) => {
const response = await request.post('/endpoint-output/body');

Expand Down
1 change: 1 addition & 0 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,7 @@ declare module '@sveltejs/kit' {
client: NonNullable<BuildData['client']>;
nodes: SSRNodeLoader[];
routes: SSRRoute[];
prerendered_routes: Set<string>;
matchers: () => Promise<Record<string, ParamMatcher>>;
/** A `[file]: size` map of all assets imported by server code */
server_assets: Record<string, number>;
Expand Down

0 comments on commit d62ed39

Please sign in to comment.