fix: prevent JSON parse error when loading data

[eltigerchino]

fixes #8751

This PR handles the edge case where loading the data of a non-existent prerendered dynamic route returns a 404 HTML page instead of JSON data. No test because our /basics test app has a root server layout load function, which causes it to do a native navigation instead, returning a 404 SSR response. The error only occurs when there is no root server layout load.

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:.

[changeset-bot]

🦋 Changeset detected

Latest commit: a1dace1

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

[dummdidumm]

Why is the response to the non-existent file returned as HTML and not as json? Would it be possible to solve this at the prerendering stage instead by creating a __data.json that tells the client "this is a 404" somehow?
Alternatively, if we detect that the return type is HTML, should we do a full page navigation instead? These cases are probably rare enough (and something's went wrong badly) that a full page reload is fine.

[eltigerchino]

Why is the response to the non-existent file returned as HTML and not as json?

My guess is it's because the route doesn't exist in the server manifest (due to it being prerendered). So a request to /doesnt-exist/__data.json returns a 404 page. In contrast, a non-prerendered dynamic page will always return JSON when requesting the __data.json because the route exists in the server manifest.

Would it be possible to solve this at the prerendering stage instead by creating a __data.json that tells the client "this is a 404" somehow?

Right now we always fetch the __data.json relative to the missing route such as /a/missing/__data.json and /a/non-existent/__data.json. Maybe when the data fetch fails we fetch again from a special 404 data route?

kit/packages/kit/src/runtime/client/client.js

Lines 1847 to 1858 in 87a18b7

	async function load_data(url, invalid) {
	const data_url = new URL(url);
	data_url.pathname = add_data_suffix(url.pathname);
	if (url.pathname.endsWith('/')) {
	data_url.searchParams.append(TRAILING_SLASH_PARAM, '1');
	}
	if (DEV && url.searchParams.has(INVALIDATED_PARAM)) {
	throw new Error(`Cannot used reserved query parameter "${INVALIDATED_PARAM}"`);
	}
	data_url.searchParams.append(INVALIDATED_PARAM, invalid.map((i) => (i ? '1' : '0')).join(''));
	const res = await native_fetch(data_url.href);

Alternatively, if we detect that the return type is HTML, should we do a full page navigation instead? These cases are probably rare enough (and something's went wrong badly) that a full page reload is fine.

Yes, this is another good alternative solution.

Another alternative, is to always return JSON when __data.json is requested from the server. But I'm not sure what the negative effects of this could be. EDIT: Thinking about it again, this alternative would not work for a CSR only application.

[eltigerchino]

updated to fallback to native navigation when receiving a HTML response instead of JSON when loading __data.json

[dummdidumm]

What's this change about btw?

[eltigerchino]

ah I was trying to resolve to the TODO about the eslint error https://eslint.org/docs/latest/rules/no-async-promise-executor I can revert it though

[dummdidumm]

I don't think that lint rule applies here, so we can revert it