fix: Throw when prerendered routes have unresolvable content negotiation
Closes #9929
~~I'm not positive this is the right solution here, but it definitely isn't breaking any tests and I think it's conceptually sound. The issue is that when prerendering, we don't set the Accept header, meaning routes with both a +page.svelte and +server.ts file with a GET handler will negotiate to the GET handler rather than the page, causing incorrect prerender output. Given that prerendering is supposed to emulate browser behavior (by "visiting" each page), it seems logical to set this header. It certainly fixes the problem in the linked issue.~~
If a prerender request is made to a route that has unresolvable content negotiation, throws. What this means practically is that you can't prerender a route with both a GET method exported from +server.js and also have a page at that route -- static file servers can't negotiate this request after prerendering, so the behavior is undefined.
This also fixes a minor bug: With the other route options, we used the precedence of +page => +page.server => +endpoint for option resolution. For entries, we were incorrectly using a precedence of +endpoint => +page => +page.server, which is all kinds of weird. I think that might technically be breaking, but it's a very new feature and I don't know anyone who'd argue that it's the correct order of precedence, so I'm in favor of treating it as a fix.
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 testand lint the project withpnpm lintandpnpm check
Changesets
- [ ] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.
🦋 Changeset detected
Latest commit: 9999c9999d467f3658eda25bc886415f98bae685
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
Thinking further on this, it strikes me that if a route is being prerendered then it's a straight-up error to have both a +page.svelte and a +server.js, and we should treat it as such rather than trying to figure out which one 'wins'. Thoughts?
@Rich-Harris
This should be ready to go, with the caveat that I didn't add any dev-time errors. I have no idea where to put them. If you point me at the right spot, I'll add 'em.
I'm not sure this is the direction I would have gone in. If you could get either HTML or JSON from the same URL depending on the request header, I would assume we should default to the HTML version. I'm not sure I like the idea of forcing the user to restructure their URLs because they turned on pre-rendering. If there's no HTML version then throwing an error makes sense to me. But I do understand the opposite point of view as well and see that Rich had suggested making this an error above (https://github.com/sveltejs/kit/pull/9994#issuecomment-1559428738).
But the point is, you can't get both if you're pre-rendering. The request to the route is made by the SvelteKit crawler -- if you have both a GET endpoint and a page, we have no way of defining what should happen, other than silently just preferring one over the other.
I imagine 95% of the time that users hit this, you've got an HTML page which gets its JSON data from an endpoint at the same URL. It seems like it'd be nice to be able to prerender that HTML page. I don't think the JSON API is something that's really user visible and so if users don't have the ability to access that when prerendering is turned on that seems fine to me.
It's been awhile since I've looked at the prerendering code though. I don't remember how the fetch responses are handled during prerendering and whether this is merely a decision of choosing whether we want to do that or whether it would not be technically feasible to prerender in that scenario.
We could set the accept header to negotiate content while prerendering. Using the metadata to know which routes are endpoints / pages and send a prerender request them with the correct accept header. Maybe only throw an error if the prerendered endpoint is a HTML file? A static server could still serve /about.html, /about and /about.json.
https://github.com/sveltejs/kit/blob/ff6cd45b42d7c61fb60dd1bb0490a2c8be4c522f/packages/kit/src/core/postbuild/prerender.js#L82-L86
this was superseded by #11256, so i'll close it.
while it's true that some static webservers can be configured to do content negotiation, i really don't think that's something we want to get into. the point of static prerendering is to have an output that works more or less everywhere