fix: Compatibility with SvelteKit

[codercms]

response.clone() breaks compatibility with libraries which hooks fetch on itself, e.g. SvelteKit - https://github.com/sveltejs/kit/blob/5ef7d0b387a8e0ff2f8d631d83585d479595d73c/packages/kit/src/runtime/server/page/load_data.js#L246

Response cloning leading to lost context for SvelteKit and request would be resent during CSR

[codercms]

@sindresorhus I think the test environment should be fixed because there is something wrong with xo or eslint

[sindresorhus]

I think the test environment should be fixed because there is something wrong with xo or eslint

Fixed in main branch. Just rebase from main.

[sindresorhus]

Can you explain your changes? It still does cloning, so I'm not sure how it fixes the issue for SvelteKit? And are you sure this won't have any adverse effects on Ky usage? There is probably a reason we always do cloning, even though I cannot remember why.

[codercms]

@sindresorhus let me explain a little bit more, SvelteKit has a smart caching system.
When you run some requests during SSR they will not run again during CSR (if you met all requirements).
This caching system relies heavily on the "fetch" function that SvelteKit provides for data loading functions (special functions to load data for app routes).

In vanilla JS it looks like this:

/** @type {import('./$types').PageLoad} */
export async function load({ depends, fetch }) {
	const resp = await fetch('...some resource');
	const json = await resp.json();

	return {
		someData: json
	};
}

So, when response is cloned this logic gets broken, because SvelteKit hooks fetch on itself, from https://github.com/sveltejs/kit/blob/5ef7d0b387a8e0ff2f8d631d83585d479595d73c/packages/kit/src/runtime/server/page/load_data.js#L246:

Click Me

const proxy = new Proxy(response, {
	get(response, key, _receiver) {
		async function text() {
			const body = await response.text();

			if (!body || typeof body === 'string') {
				const status_number = Number(response.status);
				if (isNaN(status_number)) {
					throw new Error(
						`response.status is not a number. value: "${
							response.status
						}" type: ${typeof response.status}`
					);
				}

				fetched.push({
					url: same_origin ? url.href.slice(event.url.origin.length) : url.href,
					method: event.request.method,
					request_body: /** @type {string | ArrayBufferView | undefined} */ (
						input instanceof Request && cloned_body
							? await stream_to_string(cloned_body)
							: init?.body
					),
					request_headers: cloned_headers,
					response_body: body,
					response
				});
			}

			if (dependency) {
				dependency.body = body;
			}

			return body;
		}

		if (key === 'arrayBuffer') {
			return async () => {
				const buffer = await response.arrayBuffer();

				if (dependency) {
					dependency.body = new Uint8Array(buffer);
				}

				// TODO should buffer be inlined into the page (albeit base64'd)?
				// any conditions in which it shouldn't be?

				return buffer;
			};
		}

		if (key === 'text') {
			return text;
		}

		if (key === 'json') {
			return async () => {
				return JSON.parse(await text());
			};
		}

		return Reflect.get(response, key, response);
	}
});

As you can see from the source code, SvelteKit hooks the fetch response through the Proxy object. And there is also a special logic with the hooked "text" method - calling fetched.push (the "json" method just calls the "text" method and parses the text into JSON).
This is fundamental for SvelteKit to remember which requests were made during SSR.

So, let's look now at ky's source code:

for (const [type, mimeType] of Object.entries(responseTypes) as ObjectEntries<typeof responseTypes>) {
	result[type] = async () => {
		// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
		ky.request.headers.set('accept', ky.request.headers.get('accept') || mimeType);

                // here is SvelteKit hooked response via Proxy
		const awaitedResult = await result;
                // and here is "un-hooked" response, so SvelteKit will never know that the "text" method was actually called
		const response = awaitedResult.clone();

		if (type === 'json') {
			if (response.status === 204) {
				return '';
			}

			const arrayBuffer = await response.clone().arrayBuffer();
			const responseSize = arrayBuffer.byteLength;
			if (responseSize === 0) {
				return '';
			}


                // but this could be easy fixed by returning "not cloned " response, this will keep SvelteKit's hook on returned response

			if (options.parseJson) {
                                // calling .text() on hooked response
				return options.parseJson(await awaitedResult.text());
			}
		}

                return awaitedResult[type]();
                // instead of this
		// return response[type]();
	};
}

return result;

[codercms]

@sindresorhus

And are you sure this won't have any adverse effects on Ky usage? There is probably a reason we always do cloning, even though I cannot remember why.

The only reason I know for cloning a response is you want to access the response body more than once, so there is already one copy of the response and I think it's safe enough to return body from original response.

[sholladay]

I’d love to get rid of clone() where possible, it’s given us quite a few problems. The main thing that needs to be checked for regressions is hooks that use the response. Each hook needs to receive a cloned response so that they are not dependent upon each other.

[sindresorhus]

Closing as this is not moving forward. We are still happy to improve SvelteKit compatibility, but the submitter must do extensive testing to ensure it doesn't break anything like hooks.