breaking: Remove dangerZone.trackServerFetches (#11235)

elliott-with-the-longest-name-on-github · Rich-Harris · web-flow · commit ffca61dd8b1e · 2023-12-10T13:31:27.000-05:00
* breaking: Remove  and related code

* changeset

* remove dangerZone per discussion

---------

Co-authored-by: Rich Harris &lt;rich.harris@vercel.com&gt;
diff --git a/.changeset/twenty-birds-eat.md b/.changeset/twenty-birds-eat.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': major
+---
+
+breaking: remove `dangerZone.trackServerFetches`
diff --git a/packages/kit/src/core/config/index.spec.js b/packages/kit/src/core/config/index.spec.js
@@ -69,9 +69,6 @@ const get_defaults = (prefix = '') => ({
 		csrf: {
 			checkOrigin: true
 		},
-		dangerZone: {
-			trackServerFetches: false
-		},
 		embedded: false,
 		env: {
 			dir: process.cwd(),
diff --git a/packages/kit/src/core/config/options.js b/packages/kit/src/core/config/options.js
@@ -111,11 +111,6 @@ const options = object(
 				checkOrigin: boolean(true)
 			}),
 
-			dangerZone: object({
-				// TODO 2.0: Remove this
-				trackServerFetches: boolean(false)
-			}),
-
 			embedded: boolean(false),
 
 			env: object({
diff --git a/packages/kit/src/core/sync/write_server.js b/packages/kit/src/core/sync/write_server.js
@@ -34,7 +34,6 @@ export const options = {
 	app_template_contains_nonce: ${template.includes('%sveltekit.nonce%')},
 	csp: ${s(config.kit.csp)},
 	csrf_check_origin: ${s(config.kit.csrf.checkOrigin)},
-	track_server_fetches: ${s(config.kit.dangerZone.trackServerFetches)},
 	embedded: ${config.kit.embedded},
 	env_public_prefix: '${config.kit.env.publicPrefix}',
 	env_private_prefix: '${config.kit.env.privatePrefix}',
diff --git a/packages/kit/src/exports/public.d.ts b/packages/kit/src/exports/public.d.ts
@@ -343,16 +343,6 @@ export interface KitConfig {
 		 */
 		checkOrigin?: boolean;
 	};
-	/**
-	 * Here be dragons. Enable at your peril.
-	 */
-	dangerZone?: {
-		/**
-		 * Automatically add server-side `fetch`ed URLs to the `dependencies` map of `load` functions. This will expose secrets
-		 * to the client if your URL contains them.
-		 */
-		trackServerFetches?: boolean;
-	};
 	/**
 	 * Whether or not the app is embedded inside a larger app. If `true`, SvelteKit will add its event listeners related to navigation etc on the parent of `%sveltekit.body%` instead of `window`, and will pass `params` from the server rather than inferring them from `location.pathname`.
 	 * @default false
diff --git a/packages/kit/src/runtime/server/data/index.js b/packages/kit/src/runtime/server/data/index.js
@@ -76,8 +76,7 @@ export async function render_data(
 								}
 							}
 							return data;
-						},
-						track_server_fetches: options.track_server_fetches
+						}
 					});
 				} catch (e) {
 					aborted = true;
diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js
@@ -150,8 +150,7 @@ export async function render_page(event, page, options, manifest, state, resolve
 								if (parent) Object.assign(data, await parent.data);
 							}
 							return data;
-						},
-						track_server_fetches: options.track_server_fetches
+						}
 					});
 				} catch (e) {
 					load_error = /** @type {Error} */ (e);
diff --git a/packages/kit/src/runtime/server/page/load_data.js b/packages/kit/src/runtime/server/page/load_data.js
@@ -10,18 +10,10 @@ import { validate_depends } from '../../shared.js';
  *   state: import('types').SSRState;
  *   node: import('types').SSRNode | undefined;
  *   parent: () => Promise<Record<string, any>>;
- *   track_server_fetches: boolean;
  * }} opts
  * @returns {Promise<import('types').ServerDataNode | null>}
  */
-export async function load_server_data({
-	event,
-	state,
-	node,
-	parent,
-	// TODO 2.0: Remove this
-	track_server_fetches
-}) {
+export async function load_server_data({ event, state, node, parent }) {
 	if (!node?.server) return null;
 
 	let done = false;
@@ -59,11 +51,6 @@ export async function load_server_data({
 				);
 			}
 
-			// TODO 2.0: Remove this
-			if (track_server_fetches) {
-				uses.dependencies.add(url.href);
-			}
-
 			return event.fetch(info, init);
 		},
 		/** @param {string[]} deps */
diff --git a/packages/kit/src/runtime/server/page/respond_with_error.js b/packages/kit/src/runtime/server/page/respond_with_error.js
@@ -49,8 +49,7 @@ export async function respond_with_error({
 				event,
 				state,
 				node: default_layout,
-				parent: async () => ({}),
-				track_server_fetches: options.track_server_fetches
+				parent: async () => ({})
 			});
 
 			const server_data = await server_data_promise;
diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts
@@ -333,7 +333,6 @@ export interface SSROptions {
 	app_template_contains_nonce: boolean;
 	csp: ValidatedConfig['kit']['csp'];
 	csrf_check_origin: boolean;
-	track_server_fetches: boolean;
 	embedded: boolean;
 	env_public_prefix: string;
 	env_private_prefix: string;
diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js
@@ -483,7 +483,8 @@ test.describe('Invalidation', () => {
 	});
 
 	test('fetch in server load cannot be invalidated', async ({ page, app, request }) => {
-		// TODO 2.0: Can remove this test after `dangerZone.trackServerFetches` and associated code is removed
+		// legacy behavior was to track server dependencies -- this could leak secrets to the client (see github.com/sveltejs/kit/pull/9945)
+		// we keep this test just to make sure the behavior stays the same.
 		await request.get('/load/invalidation/server-fetch/count.json?reset');
 		await page.goto('/load/invalidation/server-fetch');
 		const selector = '[data-testid="count"]';
diff --git a/packages/kit/test/apps/options/svelte.config.js b/packages/kit/test/apps/options/svelte.config.js
@@ -9,9 +9,6 @@ const config = {
 				'require-trusted-types-for': ['script']
 			}
 		},
-		dangerZone: {
-			trackServerFetches: true
-		},
 		files: {
 			assets: 'public',
 			lib: 'source/components',
diff --git a/packages/kit/test/apps/options/test/test.js b/packages/kit/test/apps/options/test/test.js
@@ -302,22 +302,3 @@ test.describe('Routing', () => {
 		await expect(page.locator('h2')).toHaveText('target: 0');
 	});
 });
-
-test.describe('load', () => {
-	// TODO 2.0: Remove this test
-	test('fetch in server load can be invalidated when `dangerZone.trackServerFetches` is set', async ({
-		page,
-		app,
-		request,
-		javaScriptEnabled
-	}) => {
-		test.skip(!javaScriptEnabled, 'JavaScript is disabled');
-		await request.get('/path-base/server-fetch-invalidate/count.json?reset');
-		await page.goto('/path-base/server-fetch-invalidate');
-		const selector = '[data-testid="count"]';
-
-		expect(await page.textContent(selector)).toBe('1');
-		await app.invalidate('/path-base/server-fetch-invalidate/count.json');
-		expect(await page.textContent(selector)).toBe('2');
-	});
-});

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@sveltejs/kit': major
 +---
++
 +breaking: remove `dangerZone.trackServerFetches`
Original file line number	Diff line number	Diff line change
`@@ -76,8 +76,7 @@ export async function render_data(`
`76`	`76`	`}`
`77`	`77`	`}`
`78`	`78`	`return data;`
`79`		`- },`
`80`		`- track_server_fetches: options.track_server_fetches`
	`79`	`+ }`
`81`	`80`	`});`
`82`	`81`	`} catch (e) {`
`83`	`82`	`aborted = true;`