remove getters

.changeset/khaki-foxes-applaud.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+more granular URL property tracking during load

.changeset/loud-lions-walk.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[breaking] change event.clientAddress to event.getClientAddress()

.changeset/shiny-gifts-drive.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Remove all enumerable getters from RequestEvent and LoadEvent

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

@@ -1,6 +1,6 @@
 import { onMount, tick } from 'svelte';
 import { normalize_error } from '../../utils/error.js';
-import { LoadURL, decode_params, normalize_path } from '../../utils/url.js';
+import { make_trackable, decode_params, normalize_path } from '../../utils/url.js';
 import { find_anchor, get_base_uri, get_href, scroll_state } from './utils.js';
 import { lock_fetch, unlock_fetch, initial_fetch, native_fetch } from './fetcher.js';
 import { parse } from './parse.js';
@@ -510,17 +510,14 @@ export function create_client({ target, base, trailing_slash }) {
 				});
 			}
 
-			const load_url = new LoadURL(url);
-
 			/** @type {import('types').LoadEvent} */
 			const load_input = {
 				routeId,
 				params: uses_params,
 				data: server_data_node?.data ?? null,
-				get url() {
+				url: make_trackable(url, () => {
 					uses.url = true;
-					return load_url;
-				},
+				}),
 				async fetch(resource, init) {
 					let requested;
 

packages/kit/src/runtime/server/index.js

@@ -4,7 +4,7 @@ import { render_response } from './page/render.js';
 import { respond_with_error } from './page/respond_with_error.js';
 import { coalesce_to_error, normalize_error } from '../../utils/error.js';
 import { serialize_error, GENERIC_ERROR, error_to_pojo } from './utils.js';
-import { decode_params, normalize_path } from '../../utils/url.js';
+import { decode_params, disable_search, normalize_path } from '../../utils/url.js';
 import { exec } from '../../utils/routing.js';
 import { negotiate } from '../../utils/http.js';
 import { HttpError, Redirect } from '../../index/private.js';
@@ -122,21 +122,17 @@ export async function respond(request, options, state) {
 	/** @type {string[]} */
 	const cookies = [];
 
+	if (state.prerendering) disable_search(url);
+
 	/** @type {import('types').RequestEvent} */
 	const event = {
-		get clientAddress() {
-			if (!state.getClientAddress) {
+		getClientAddress:
+			state.getClientAddress ||
+			(() => {
 				throw new Error(
 					`${__SVELTEKIT_ADAPTER_NAME__} does not specify getClientAddress. Please raise an issue`
 				);
-			}
-
-			Object.defineProperty(event, 'clientAddress', {
-				value: state.getClientAddress()
-			});
-
-			return event.clientAddress;
-		},
+			}),
 		locals: {},
 		params,
 		platform: state.platform,
@@ -195,6 +191,7 @@ export async function respond(request, options, state) {
 	};
 
 	Object.defineProperties(event, {
+		clientAddress: removed('clientAddress', 'getClientAddress'),
 		method: removed('method', 'request.method', details),
 		headers: removed('headers', 'request.headers', details),
 		origin: removed('origin', 'url.origin'),
@@ -277,6 +274,7 @@ export async function respond(request, options, state) {
 										return load_server_data({
 											dev: options.dev,
 											event,
+											state,
 											node,
 											parent: async () => {
 												/** @type {Record<string, any>} */

packages/kit/src/runtime/server/page/index.js

@@ -165,6 +165,7 @@ export async function render_page(event, route, page, options, state, resolve_op
 					return await load_server_data({
 						dev: options.dev,
 						event,
+						state,
 						node,
 						parent: async () => {
 							/** @type {Record<string, any>} */

packages/kit/src/runtime/server/page/load_data.js

@@ -1,16 +1,17 @@
-import { LoadURL, PrerenderingURL } from '../../../utils/url.js';
+import { disable_search, make_trackable } from '../../../utils/url.js';
 
 /**
  * Calls the user's `load` function.
  * @param {{
  *   dev: boolean;
  *   event: import('types').RequestEvent;
+ *   state: import('types').SSRState;
  *   node: import('types').SSRNode | undefined;
  *   parent: () => Promise<Record<string, any>>;
  * }} opts
  * @returns {Promise<import('types').ServerDataNode | null>}
  */
-export async function load_server_data({ dev, event, node, parent }) {
+export async function load_server_data({ dev, event, state, node, parent }) {
 	if (!node?.server) return null;
 
 	const uses = {
@@ -20,39 +21,34 @@ export async function load_server_data({ dev, event, node, parent }) {
 		url: false
 	};
 
-	/** @param {string[]} deps */
-	function depends(...deps) {
-		for (const dep of deps) {
-			const { href } = new URL(dep, event.url);
-			uses.dependencies.add(href);
-		}
-	}
-
-	const params = new Proxy(event.params, {
-		get: (target, key) => {
-			uses.params.add(key);
-			return target[/** @type {string} */ (key)];
-		}
+	const url = make_trackable(event.url, () => {
+		uses.url = true;
 	});
 
+	if (state.prerendering) {
+		disable_search(url);
+	}
+
 	const result = await node.server.load?.call(null, {
-		// can't use destructuring here because it will always
-		// invoke event.clientAddress, which breaks prerendering
-		get clientAddress() {
-			return event.clientAddress;
+		...event,
+		/** @param {string[]} deps */
+		depends: (...deps) => {
+			for (const dep of deps) {
+				const { href } = new URL(dep, event.url);
+				uses.dependencies.add(href);
+			}
 		},
-		depends,
-		locals: event.locals,
-		params,
+		params: new Proxy(event.params, {
+			get: (target, key) => {
+				uses.params.add(key);
+				return target[/** @type {string} */ (key)];
+			}
+		}),
 		parent: async () => {
 			uses.parent = true;
 			return parent();
 		},
-		platform: event.platform,
-		request: event.request,
-		routeId: event.routeId,
-		setHeaders: event.setHeaders,
-		url: event.url
+		url
 	});
 
 	const data = result ? await unwrap_promises(result) : null;
@@ -85,15 +81,15 @@ export async function load_server_data({ dev, event, node, parent }) {
  * }} opts
  * @returns {Promise<Record<string, any> | null>}
  */
-export async function load_data({ event, fetcher, node, parent, server_data_promise, state }) {
+export async function load_data({ event, fetcher, node, parent, server_data_promise }) {
 	const server_data_node = await server_data_promise;
 
 	if (!node?.shared?.load) {
 		return server_data_node?.data ?? null;
 	}
 
-	const load_input = {
-		url: state.prerendering ? new PrerenderingURL(event.url) : new LoadURL(event.url),
+	const load_event = {
+		url: event.url,
 		params: event.params,
 		data: server_data_node?.data ?? null,
 		routeId: event.routeId,
@@ -104,7 +100,7 @@ export async function load_data({ event, fetcher, node, parent, server_data_prom
 	};
 
 	// TODO remove this for 1.0
-	Object.defineProperties(load_input, {
+	Object.defineProperties(load_event, {
 		session: {
 			get() {
 				throw new Error(
@@ -115,7 +111,7 @@ export async function load_data({ event, fetcher, node, parent, server_data_prom
 		}
 	});
 
-	const data = await node.shared.load.call(null, load_input);
+	const data = await node.shared.load.call(null, load_event);
 
 	return data ? unwrap_promises(data) : null;
 }

packages/kit/src/runtime/server/page/render.js

@@ -5,7 +5,6 @@ import { hash } from '../../hash.js';
 import { render_json_payload_script } from '../../../utils/escape.js';
 import { s } from '../../../utils/misc.js';
 import { Csp } from './csp.js';
-import { PrerenderingURL } from '../../../utils/url.js';
 import { serialize_error } from '../utils.js';
 import { HttpError } from '../../../index/private.js';
 
@@ -100,7 +99,7 @@ export async function render_response({
 			params: /** @type {Record<string, any>} */ (event.params),
 			routeId: event.routeId,
 			status,
-			url: state.prerendering ? new PrerenderingURL(event.url) : event.url,
+			url: event.url,
 			data
 		};
 

packages/kit/src/runtime/server/page/respond_with_error.js

@@ -37,6 +37,7 @@ export async function respond_with_error({ event, options, state, status, error,
 			const server_data_promise = load_server_data({
 				dev: options.dev,
 				event,
+				state,
 				node: default_layout,
 				parent: async () => ({})
 			});

packages/kit/src/utils/url.js

@@ -75,23 +75,68 @@ export function decode_params(params) {
 	return params;
 }
 
-export class LoadURL extends URL {
-	/** @returns {string} */
-	get hash() {
-		throw new Error(
-			'url.hash is inaccessible from load. Consider accessing hash from the page store within the script tag of your component.'
-		);
+/**
+ * URL properties that could change during the lifetime of the page,
+ * which excludes things like `origin`
+ * @type {Array<keyof URL>}
+ */
+const tracked_url_properties = ['href', 'pathname', 'search', 'searchParams', 'toString', 'toJSON'];
+
+/**
+ * @param {URL} url
+ * @param {() => void} callback
+ */
+export function make_trackable(url, callback) {
+	const tracked = new URL(url);
+
+	for (const property of tracked_url_properties) {
+		let value = tracked[property];
+
+		Object.defineProperty(tracked, property, {
+			get() {
+				callback();
+				return value;
+			},
+
+			enumerable: true,
+			configurable: true
+		});
 	}
+
+	// @ts-ignore
+	tracked[Symbol.for('nodejs.util.inspect.custom')] = (depth, opts, inspect) => {
+		return inspect(url, opts);
+	};
+
+	disable_hash(tracked);
+
+	return tracked;
 }
 
-export class PrerenderingURL extends URL {
-	/** @returns {string} */
-	get search() {
-		throw new Error('Cannot access url.search on a page with prerendering enabled');
-	}
+/**
+ * Disallow access to `url.hash` on the server and in `load`
+ * @param {URL} url
+ */
+export function disable_hash(url) {
+	Object.defineProperty(url, 'hash', {
+		get() {
+			throw new Error(
+				'Cannot access event.url.hash. Consider using `$page.url.hash` inside a component instead'
+			);
+		}
+	});
+}
 
-	/** @returns {URLSearchParams} */
-	get searchParams() {
-		throw new Error('Cannot access url.searchParams on a page with prerendering enabled');
+/**
+ * Disallow access to `url.search` and `url.searchParams` during prerendering
+ * @param {URL} url
+ */
+export function disable_search(url) {
+	for (const property of ['search', 'searchParams']) {
+		Object.defineProperty(url, property, {
+			get() {
+				throw new Error(`Cannot access url.${property} on a page with prerendering enabled`);
+			}
+		});
 	}
 }

packages/kit/src/utils/url.spec.js

@@ -1,6 +1,6 @@
 import { suite } from 'uvu';
 import * as assert from 'uvu/assert';
-import { resolve, normalize_path, LoadURL, PrerenderingURL } from './url.js';
+import { resolve, normalize_path, make_trackable, disable_search } from './url.js';
 
 /**
  *
@@ -94,12 +94,23 @@ describe('normalize_path', (test) => {
 	});
 });
 
-describe('LoadURL', (test) => {
+describe('make_trackable', (test) => {
+	test('makes URL properties trackable', () => {
+		let tracked = false;
+
+		const url = make_trackable(new URL('https://kit.svelte.dev/docs'), () => {
+			tracked = true;
+		});
+
+		url.origin;
+		assert.ok(!tracked);
+
+		url.pathname;
+		assert.ok(tracked);
+	});
+
 	test('throws an error when its hash property is accessed', () => {
-		/**
-		 * @type {URL}
-		 */
-		const url = new LoadURL('https://kit.svelte.dev/docs');
+		const url = make_trackable(new URL('https://kit.svelte.dev/docs'), () => {});
 
 		assert.throws(
 			() => url.hash,
@@ -108,16 +119,12 @@ describe('LoadURL', (test) => {
 	});
 });
 
-describe('PrerenderingURL', (test) => {
+describe('disable_search', (test) => {
 	test('throws an error when its search property is accessed', () => {
-		/**
-		 * @type {URL}
-		 */
-		const url = new PrerenderingURL('https://kit.svelte.dev/docs');
-
-		/**
-		 * @type {Array<'search' | 'searchParams'>}
-		 */
+		const url = new URL('https://kit.svelte.dev/docs');
+		disable_search(url);
+
+		/** @type {Array<keyof URL>} */
 		const props = ['search', 'searchParams'];
 		props.forEach((prop) => {
 			assert.throws(

packages/kit/test/apps/basics/src/routes/load/invalidation/url/+page.server.js

@@ -0,0 +1,6 @@
+/** @type {import('./$types').PageServerLoad} */
+export function load({ url }) {
+	return {
+		a: url.searchParams.get('a')
+	};
+}