Skip to content
Closed
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/brown-eggs-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: prerender pages that share the same route as a `+server.js` file even if it does not export a `GET` method
7 changes: 4 additions & 3 deletions packages/kit/src/core/postbuild/analyse.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ async function analyse({

const route_config = page?.config ?? endpoint?.config ?? {};
const prerender = page?.prerender ?? endpoint?.prerender;

if (prerender !== true) {
for (const feature of list_features(
route,
Expand All @@ -142,10 +141,12 @@ async function analyse({
config: route_config,
methods: Array.from(new Set([...page_methods, ...api_methods])),
page: {
methods: page_methods
methods: page_methods,
prerender: page?.prerender
},
api: {
methods: api_methods
methods: api_methods,
prerender: endpoint?.prerender
},
prerender,
entries:
Expand Down
80 changes: 50 additions & 30 deletions packages/kit/src/core/postbuild/prerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,55 +214,65 @@ async function prerender({ hash, out, manifest_path, metadata, verbose, env }) {
* @param {string} decoded
* @param {string} [encoded]
* @param {string} [generated_from_id]
* @param {boolean} [expect_html]
*/
function enqueue(referrer, decoded, encoded, generated_from_id) {
if (seen.has(decoded)) return;
seen.add(decoded);
function enqueue(referrer, decoded, encoded, generated_from_id, expect_html) {
const key = expect_html ? decoded + '\x00page' : decoded;
if (seen.has(key)) return;
seen.add(key);

const file = decoded.slice(config.paths.base.length + 1);
if (files.has(file)) return;

return q.add(() => visit(decoded, encoded || encodeURI(decoded), referrer, generated_from_id));
return q.add(() =>
visit(decoded, encoded || encodeURI(decoded), referrer, generated_from_id, expect_html)
);
}

/**
* @param {string} decoded
* @param {string} encoded
* @param {string?} referrer
* @param {string} [generated_from_id]
* @param {boolean} [expect_html]
*/
async function visit(decoded, encoded, referrer, generated_from_id) {
async function visit(decoded, encoded, referrer, generated_from_id, expect_html) {
if (!decoded.startsWith(config.paths.base)) {
handle_http_error({ status: 404, path: decoded, referrer, referenceType: 'linked' });
return;
}

const request_headers = expect_html ? { Accept: 'text/html' } : undefined;

/** @type {Map<string, import('types').PrerenderDependency>} */
const dependencies = new Map();

const response = await server.respond(new Request(config.prerender.origin + encoded), {
getClientAddress() {
throw new Error('Cannot read clientAddress during prerendering');
},
prerendering: {
dependencies,
remote_responses
},
read: (file) => {
// stuff we just wrote
const filepath = saved.get(file);
if (filepath) return readFileSync(filepath);

// Static assets emitted during build
if (file.startsWith(config.appDir)) {
return readFileSync(`${out}/server/${file}`);
}
const response = await server.respond(
new Request(config.prerender.origin + encoded, { headers: request_headers }),
{
getClientAddress() {
throw new Error('Cannot read clientAddress during prerendering');
},
prerendering: {
dependencies,
remote_responses
},
read: (file) => {
// stuff we just wrote
const filepath = saved.get(file);
if (filepath) return readFileSync(filepath);

// Static assets emitted during build
if (file.startsWith(config.appDir)) {
return readFileSync(`${out}/server/${file}`);
}

// stuff in `static`
return readFileSync(join(config.files.assets, file));
},
emulator
});
// stuff in `static`
return readFileSync(join(config.files.assets, file));
},
emulator
}
);

const encoded_id = response.headers.get('x-sveltekit-routeid');
const decoded_id = encoded_id && decode_uri(encoded_id);
Expand Down Expand Up @@ -356,7 +366,7 @@ async function prerender({ hash, out, manifest_path, metadata, verbose, env }) {
/** @type {Set<string>} */ (expected_hashlinks.get(key)).add(decoded);
}

void enqueue(decoded, decode_uri(pathname), pathname);
void enqueue(decoded, decode_uri(pathname), pathname, undefined, false);
}
}
}
Expand Down Expand Up @@ -535,7 +545,15 @@ async function prerender({ hash, out, manifest_path, metadata, verbose, env }) {

if (processed_id.includes('[')) continue;
const path = `/${get_route_segments(processed_id).join('/')}`;
void enqueue(null, config.paths.base + path);

const route_data = metadata.routes.get(id);
void enqueue(
null,
config.paths.base + path,
undefined,
undefined,
!!route_data?.page.prerender
);
}
}
} else {
Expand All @@ -544,8 +562,10 @@ async function prerender({ hash, out, manifest_path, metadata, verbose, env }) {
}

for (const { id, entries } of route_level_entries) {
const route_data = metadata.routes.get(id);
const expect_html = !!route_data?.page.prerender;
for (const entry of entries) {
void enqueue(null, config.paths.base + entry, undefined, id);
void enqueue(null, config.paths.base + entry, undefined, id, expect_html);
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/kit/src/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,11 @@ export interface ServerMetadataRoute {
config: any;
api: {
methods: Array<HttpMethod | '*'>;
prerender: PrerenderOption | undefined;
};
page: {
methods: Array<'GET' | 'POST'>;
prerender: PrerenderOption | undefined;
};
methods: Array<HttpMethod | '*'>;
prerender: PrerenderOption | undefined;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>prerendered page with server endpoint</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function GET() {
return new Response(JSON.stringify({ ok: true }), {
headers: { 'content-type': 'application/json' }
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello World...
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export async function POST() {
return new Response('OK', { status: 200 });
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
import { resolve } from '$app/paths';
</script>

<a href={resolve('/linked-api/my-awesome-endpoint.json')}>My Awesome Endpoint</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const prerender = true;

export function GET() {
return new Response(JSON.stringify({ ok: true }), {
headers: { 'content-type': 'application/json' }
});
}
14 changes: 14 additions & 0 deletions packages/kit/test/prerendering/basics/test/tests.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,20 @@ test('does not prerender page with shadow endpoint with non-load handler', () =>
assert.isFalse(fs.existsSync(`${build}/shadowed-post/__data.json`));
});

test('prerenders a page that coexists with a GET endpoint', () => {
assert.isTrue(fs.existsSync(`${build}/duplicate-get.html`));
});

test('prerenders a page that coexists with a POST endpoint', () => {
assert.isTrue(fs.existsSync(`${build}/get-and-post.html`));
});

test('prerendering a page with a linked GET server endpoint processes properly', () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test passes even without the PR's fix. Do we really need it? Even if the endpoint wasn't discovered while crawling the page, the prerenderer would prerender the endpoint because it detects the exported prerender page option during analysis.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean I see no harm done in keeping it, I put it there in part because I knew my code changed the behavior there and I wanted to be sure that there was no regression happening on my behalf. Fully up to yall.

assert.isTrue(fs.existsSync(`${build}/linked-api.html`));
assert.isTrue(fs.existsSync(`${build}/linked-api/my-awesome-endpoint.json`));
assert.isFalse(fs.existsSync(`${build}/linked-api/my-awesome-endpoint.html`));
});

test('decodes paths when writing files', () => {
let content = read('encoding/path with spaces.html');
expect(content).toMatch('<p id="a">path with spaces</p>');
Expand Down