Skip to content

fix: prerender pages adjacent to endpoints - #15621

Closed
aenriii wants to merge 14 commits into
sveltejs:mainfrom
aenriii:main
Closed

fix: prerender pages adjacent to endpoints#15621
aenriii wants to merge 14 commits into
sveltejs:mainfrom
aenriii:main

Conversation

@aenriii

@aenriii aenriii commented Mar 30, 2026

Copy link
Copy Markdown

closes #15620
closes #10735

This PR addresses #15620, and adds tests for any possible regressions as a result of the implemented fix. (edit: seems to also fix #10735)

Changes:

  • modified kit/src/core/postbuild/analyse.js to add additional metadata fields to route metadata, page.prerender and api.prerender, which pass on the page and endpoint prerender fields. this prevents a regression which forces all parts of a route to prerender even if only one part requests it.
  • modified kit/src/core/postbuild/prerender.js to check if page.prerender and api.prerender is true, and enqueue prerendering if so. uses \x00page as a postfix to page keys within the preexisting seen set to differentiate between api endpoints and pages.
  • modified kit/src/core/postbuild/prerender.js, added and documented expect_html parameter in visit() and enqueue() to specify that the Accept: text/html header should be sent to the internal server when prerendering a page.
  • implemented two tests in prerendering/basics, one which tests the fix itself (see prerender-with-server-endpoint) and one which confirms the non-existence of a possible regression (see linked-api). both tests pass with flying colors :D

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

@changeset-bot

changeset-bot Bot commented Mar 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 116467f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@aenriii aenriii changed the title fix: prerender no longer silently fails pertinent to #15620 fix: prerender no longer silently ignores webpages pertinent to #15620 Mar 30, 2026
aenriii added a commit to lvehrt/openrx.info that referenced this pull request Apr 1, 2026
@svelte-docs-bot

Copy link
Copy Markdown

@aenriii

aenriii commented Apr 5, 2026

Copy link
Copy Markdown
Author

regressions fixed, should also close #10735

@teemingc teemingc changed the title fix: prerender no longer silently ignores webpages pertinent to #15620 fix: prerender pages adjacent to endpoints without exported GET May 6, 2026
Comment thread .changeset/brown-eggs-march.md Outdated
Comment thread packages/kit/src/types/internal.d.ts Outdated
Comment thread packages/kit/src/types/internal.d.ts Outdated
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.

Comment thread packages/kit/test/prerendering/basics/test/tests.spec.js Outdated
Comment thread packages/kit/test/prerendering/basics/test/tests.spec.js Outdated
Comment thread packages/kit/src/core/postbuild/prerender.js Outdated
Comment thread packages/kit/src/core/postbuild/prerender.js Outdated
Comment thread packages/kit/src/core/postbuild/prerender.js Outdated
Comment thread packages/kit/src/core/postbuild/prerender.js Outdated

@teemingc teemingc left a comment

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.

Thanks! This is looking good. I just have two concerns which I've outlined in the files changed.

@teemingc teemingc changed the title fix: prerender pages adjacent to endpoints without exported GET fix: prerender pages adjacent to endpoints May 18, 2026
@aenriii
aenriii requested a review from teemingc May 26, 2026 11:38
@aenriii

aenriii commented May 26, 2026

Copy link
Copy Markdown
Author

hit the big "rebase to sveltejs:main" button and nothing broke, supposedly looks good?

theetrain added a commit to theetrain/theetrain-website that referenced this pull request May 30, 2026
Routes with content negotiation cannot be prerendered. This may
potentially be fixed by sveltejs/kit#15621
@Rich-Harris

Copy link
Copy Markdown
Member

This doesn't seem right to me. Surely if a route has both a +page and a +server prerendering just should fail outright?

@teemingc

Copy link
Copy Markdown
Member

Closing in favour of #16075

@teemingc teemingc closed this Jun 18, 2026
Rich-Harris added a commit that referenced this pull request Jun 18, 2026
…erenderable (#16075)

I'm pretty sure this, and not #15621, is the desired behaviour. Closes
#15620

---

### Please don't delete this checklist! Before submitting the PR, please
make sure you do the following:
- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] This message body should clearly illustrate what problems it
solves.
- [ ] Ideally, include a test that fails without this PR but passes with
it.

### Tests
- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint` and `pnpm check`

### Changesets
- [x] If your PR makes a change that should be noted in one or more
packages' changelogs, generate a changeset by running `pnpm changeset`
and following the prompts. Changesets that add features should be
`minor` and those that fix bugs should be `patch`. Please prefix
changeset messages with `feat:`, `fix:`, or `chore:`.

### Edits

- [x] Please ensure that 'Allow edits from maintainers' is checked. PRs
without this option may be closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants