Skip to content

feat: Fail prerendering for pages with mutative endpoints - fixes #3410 and #4252 #4812

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
5 changes: 5 additions & 0 deletions .changeset/brave-jobs-occur.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

feat: Pages marked for prerendering fail during ssr at runtime
5 changes: 4 additions & 1 deletion packages/kit/src/core/build/build_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ export class Server {
method_override: ${s(config.kit.methodOverride)},
paths: { base, assets },
prefix: assets + '/${config.kit.appDir}/',
prerender: ${config.kit.prerender.enabled},
prerender: {
default: ${config.kit.prerender.default},
enabled: ${config.kit.prerender.enabled}
},
read,
root,
service_worker: ${has_service_worker ? "base + '/service-worker.js'" : 'null'},
Expand Down
6 changes: 2 additions & 4 deletions packages/kit/src/core/build/prerender/prerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ export async function prerender({ config, entries, files, log }) {

const response = await server.respond(new Request(`http://sveltekit-prerender${encoded}`), {
getClientAddress,
prerender: {
default: config.kit.prerender.default,
prerendering: {
dependencies
}
});
Expand Down Expand Up @@ -272,9 +271,8 @@ export async function prerender({ config, entries, files, log }) {

const rendered = await server.respond(new Request('http://sveltekit-prerender/[fallback]'), {
getClientAddress,
prerender: {
prerendering: {
fallback: true,
default: false,
dependencies: new Map()
}
});
Expand Down
5 changes: 4 additions & 1 deletion packages/kit/src/core/dev/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,10 @@ export async function create_plugin(config, cwd) {
assets
},
prefix: '',
prerender: config.kit.prerender.enabled,
prerender: {
default: config.kit.prerender.default,
enabled: config.kit.prerender.enabled
},
read: (file) => fs.readFileSync(path.join(config.kit.files.assets, file)),
root,
router: config.kit.browser.router,
Expand Down
10 changes: 5 additions & 5 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export async function respond(request, options, state) {
/** @type {Record<string, string>} */
let params = {};

if (options.paths.base && !state.prerender?.fallback) {
if (options.paths.base && !state.prerendering?.fallback) {
if (!decoded.startsWith(options.paths.base)) {
return new Response(undefined, { status: 404 });
}
Expand All @@ -63,7 +63,7 @@ export async function respond(request, options, state) {
url = new URL(url.origin + url.pathname.slice(0, -DATA_SUFFIX.length) + url.search);
}

if (!state.prerender || !state.prerender.fallback) {
if (!state.prerendering?.fallback) {
const matchers = await options.manifest._.matchers();

for (const candidate of options.manifest._.routes) {
Expand All @@ -82,7 +82,7 @@ export async function respond(request, options, state) {
if (route?.type === 'page') {
const normalized = normalize_path(url.pathname, options.trailing_slash);

if (normalized !== url.pathname && !state.prerender?.fallback) {
if (normalized !== url.pathname && !state.prerendering?.fallback) {
return new Response(undefined, {
status: 301,
headers: {
Expand Down Expand Up @@ -173,7 +173,7 @@ export async function respond(request, options, state) {
};
}

if (state.prerender && state.prerender.fallback) {
if (state.prerendering?.fallback) {
return await render_response({
event,
options,
Expand Down Expand Up @@ -275,7 +275,7 @@ export async function respond(request, options, state) {
});
}

if (state.prerender) {
if (state.prerendering) {
return new Response('not found', { status: 404 });
}

Expand Down
14 changes: 8 additions & 6 deletions packages/kit/src/runtime/server/page/load_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,15 @@ export async function load_node({
/** @type {import('types').LoadOutput} */
let loaded;

const should_prerender = node.module.prerender ?? options.prerender.default;

/** @type {import('types').ShadowData} */
const shadow = is_leaf
? await load_shadow_data(
/** @type {import('types').SSRPage} */ (route),
event,
options,
!!state.prerender
should_prerender
)
: {};

Expand All @@ -81,7 +83,7 @@ export async function load_node({
} else if (module.load) {
/** @type {import('types').LoadInput} */
const load_input = {
url: state.prerender ? create_prerendering_url_proxy(event.url) : event.url,
url: state.prerendering ? create_prerendering_url_proxy(event.url) : event.url,
params: event.params,
props: shadow.body || {},
routeId: event.routeId,
Expand Down Expand Up @@ -217,9 +219,9 @@ export async function load_node({
}
);

if (state.prerender) {
if (state.prerendering) {
dependency = { response, body: null };
state.prerender.dependencies.set(resolved, dependency);
state.prerendering.dependencies.set(resolved, dependency);
}
} else {
// external
Expand Down Expand Up @@ -364,15 +366,15 @@ export async function load_node({
}

// generate __data.json files when prerendering
if (shadow.body && state.prerender) {
if (shadow.body && state.prerendering) {
const pathname = `${event.url.pathname.replace(/\/$/, '')}/__data.json`;

const dependency = {
response: new Response(undefined),
body: JSON.stringify(shadow.body)
};

state.prerender.dependencies.set(pathname, dependency);
state.prerendering.dependencies.set(pathname, dependency);
}

return {
Expand Down
10 changes: 5 additions & 5 deletions packages/kit/src/runtime/server/page/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export async function render_response({
resolve_opts,
stuff
}) {
if (state.prerender) {
if (state.prerendering) {
if (options.csp.mode === 'nonce') {
throw new Error('Cannot use prerendering if config.kit.csp.mode === "nonce"');
}
Expand Down Expand Up @@ -108,7 +108,7 @@ export async function render_response({
routeId: event.routeId,
status,
stuff,
url: state.prerender ? create_prerendering_url_proxy(event.url) : event.url
url: state.prerendering ? create_prerendering_url_proxy(event.url) : event.url
},
components: branch.map(({ node }) => node.module.default)
};
Expand Down Expand Up @@ -148,7 +148,7 @@ export async function render_response({
await csp_ready;
const csp = new Csp(options.csp, {
dev: options.dev,
prerender: !!state.prerender,
prerender: !!state.prerendering,
needs_nonce: options.template_contains_nonce
});

Expand Down Expand Up @@ -257,7 +257,7 @@ export async function render_response({
<script${csp.script_needs_nonce ? ` nonce="${csp.nonce}"` : ''}>${init_service_worker}</script>`;
}

if (state.prerender) {
if (state.prerendering) {
const http_equiv = [];

const csp_headers = csp.get_meta();
Expand Down Expand Up @@ -295,7 +295,7 @@ export async function render_response({
headers.set('permissions-policy', 'interest-cohort=()');
}

if (!state.prerender) {
if (!state.prerendering) {
const csp_header = csp.get_header();
if (csp_header) {
headers.set('content-security-policy', csp_header);
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/src/runtime/server/page/respond.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ export async function respond(opts) {

let page_config = get_page_config(leaf, options);

if (state.prerender) {
if (state.prerendering) {
// if the page isn't marked as prerenderable (or is explicitly
// marked NOT prerenderable, if `prerender.default` is `true`),
// then bail out at this point
const should_prerender = leaf.prerender ?? state.prerender.default;
const should_prerender = leaf.prerender ?? options.prerender.default;
if (!should_prerender) {
return new Response(undefined, {
status: 204
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<script context="module">
export const load = ({ error, status }) => {
return {
props: {
title: `${status}: ${error.message}`
}
};
};
</script>

<script>
export let title = 'Unknown error!';
</script>

<h1>{title}</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script context="module">
export const prerender = true;
</script>

<h1>I have a mutative endpoint!</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const post = () => {
return {};
};
9 changes: 9 additions & 0 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,15 @@ test.describe.parallel('Errors', () => {
expect(response.status()).toBe(500);
expect(await response.text()).toMatch('thisvariableisnotdefined is not defined');
});

test('prerendering a page with a mutative page endpoint results in a catchable error', async ({
page
}) => {
await page.goto('/prerendering/mutative-endpoint');
expect(await page.textContent('h1')).toBe(
'500: Cannot prerender pages that have endpoints with mutative methods'
);
});
});

test.describe.parallel('ETags', () => {
Expand Down
10 changes: 6 additions & 4 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export class InternalServer extends Server {
respond(
request: Request,
options: RequestOptions & {
prerender?: PrerenderOptions;
prerendering?: PrerenderOptions;
}
): Promise<Response>;
}
Expand Down Expand Up @@ -147,7 +147,6 @@ export interface PrerenderDependency {

export interface PrerenderOptions {
fallback?: boolean;
default: boolean;
dependencies: Map<string, PrerenderDependency>;
}

Expand Down Expand Up @@ -254,7 +253,10 @@ export interface SSROptions {
assets: string;
};
prefix: string;
prerender: boolean;
prerender: {
default: boolean;
enabled: boolean;
};
read(file: string): Buffer;
root: SSRComponent['default'];
router: boolean;
Expand Down Expand Up @@ -308,7 +310,7 @@ export interface SSRState {
getClientAddress: () => string;
initiator?: SSRPage | null;
platform?: any;
prerender?: PrerenderOptions;
prerendering?: PrerenderOptions;
}

export type StrictBody = string | Uint8Array;
Expand Down