feat: omit unnecessary setters from bindings (#13269)

* remove unnecessary closure

* lint

* reuse logic

* feat: omit unnecessary setters in bindings

* changeset

---------

Co-authored-by: adiguba <[email protected]>

Author: Rich-Harris

.changeset/stupid-dodos-hide.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: unwrap function expressions where possible, and optimise bindings

packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js

@@ -36,13 +36,20 @@ export function BindDirective(node, context) {
 		);
 	}
 
-	const getter = b.thunk(/** @type {Expression} */ (context.visit(expression)));
-
-	const setter = b.arrow(
-		[b.id('$$value')],
-		/** @type {Expression} */ (context.visit(b.assignment('=', expression, b.id('$$value'))))
+	const get = b.thunk(/** @type {Expression} */ (context.visit(expression)));
+
+	/** @type {Expression | undefined} */
+	let set = b.unthunk(
+		b.arrow(
+			[b.id('$$value')],
+			/** @type {Expression} */ (context.visit(b.assignment('=', expression, b.id('$$value'))))
+		)
 	);
 
+	if (get === set) {
+		set = undefined;
+	}
+
 	/** @type {CallExpression} */
 	let call;
 
@@ -52,101 +59,106 @@ export function BindDirective(node, context) {
 			b.literal(node.name),
 			b.literal(property.event),
 			context.state.node,
-			setter,
-			property.bidirectional && getter
+			set ?? get,
+			property.bidirectional && get
 		);
 	} else {
 		// special cases
 		switch (node.name) {
 			// window
 			case 'online':
-				call = b.call(`$.bind_online`, setter);
+				call = b.call(`$.bind_online`, set ?? get);
 				break;
 
 			case 'scrollX':
 			case 'scrollY':
 				call = b.call(
 					'$.bind_window_scroll',
 					b.literal(node.name === 'scrollX' ? 'x' : 'y'),
-					getter,
-					setter
+					get,
+					set
 				);
 				break;
 
 			case 'innerWidth':
 			case 'innerHeight':
 			case 'outerWidth':
 			case 'outerHeight':
-				call = b.call('$.bind_window_size', b.literal(node.name), setter);
+				call = b.call('$.bind_window_size', b.literal(node.name), set ?? get);
 				break;
 
 			// document
 			case 'activeElement':
-				call = b.call('$.bind_active_element', setter);
+				call = b.call('$.bind_active_element', set ?? get);
 				break;
 
 			// media
 			case 'muted':
-				call = b.call(`$.bind_muted`, context.state.node, getter, setter);
+				call = b.call(`$.bind_muted`, context.state.node, get, set);
 				break;
 			case 'paused':
-				call = b.call(`$.bind_paused`, context.state.node, getter, setter);
+				call = b.call(`$.bind_paused`, context.state.node, get, set);
 				break;
 			case 'volume':
-				call = b.call(`$.bind_volume`, context.state.node, getter, setter);
+				call = b.call(`$.bind_volume`, context.state.node, get, set);
 				break;
 			case 'playbackRate':
-				call = b.call(`$.bind_playback_rate`, context.state.node, getter, setter);
+				call = b.call(`$.bind_playback_rate`, context.state.node, get, set);
 				break;
 			case 'currentTime':
-				call = b.call(`$.bind_current_time`, context.state.node, getter, setter);
+				call = b.call(`$.bind_current_time`, context.state.node, get, set);
 				break;
 			case 'buffered':
-				call = b.call(`$.bind_buffered`, context.state.node, setter);
+				call = b.call(`$.bind_buffered`, context.state.node, set ?? get);
 				break;
 			case 'played':
-				call = b.call(`$.bind_played`, context.state.node, setter);
+				call = b.call(`$.bind_played`, context.state.node, set ?? get);
 				break;
 			case 'seekable':
-				call = b.call(`$.bind_seekable`, context.state.node, setter);
+				call = b.call(`$.bind_seekable`, context.state.node, set ?? get);
 				break;
 			case 'seeking':
-				call = b.call(`$.bind_seeking`, context.state.node, setter);
+				call = b.call(`$.bind_seeking`, context.state.node, set ?? get);
 				break;
 			case 'ended':
-				call = b.call(`$.bind_ended`, context.state.node, setter);
+				call = b.call(`$.bind_ended`, context.state.node, set ?? get);
 				break;
 			case 'readyState':
-				call = b.call(`$.bind_ready_state`, context.state.node, setter);
+				call = b.call(`$.bind_ready_state`, context.state.node, set ?? get);
 				break;
 
 			// dimensions
 			case 'contentRect':
 			case 'contentBoxSize':
 			case 'borderBoxSize':
 			case 'devicePixelContentBoxSize':
-				call = b.call('$.bind_resize_observer', context.state.node, b.literal(node.name), setter);
+				call = b.call(
+					'$.bind_resize_observer',
+					context.state.node,
+					b.literal(node.name),
+					set ?? get
+				);
 				break;
 
 			case 'clientWidth':
 			case 'clientHeight':
 			case 'offsetWidth':
 			case 'offsetHeight':
-				call = b.call('$.bind_element_size', context.state.node, b.literal(node.name), setter);
+				call = b.call('$.bind_element_size', context.state.node, b.literal(node.name), set ?? get);
 				break;
 
 			// various
 			case 'value': {
 				if (parent?.type === 'RegularElement' && parent.name === 'select') {
-					call = b.call(`$.bind_select_value`, context.state.node, getter, setter);
+					call = b.call(`$.bind_select_value`, context.state.node, get, set);
 				} else {
-					call = b.call(`$.bind_value`, context.state.node, getter, setter);
+					call = b.call(`$.bind_value`, context.state.node, get, set);
 				}
 				break;
 			}
 
 			case 'files':
-				call = b.call(`$.bind_files`, context.state.node, getter, setter);
+				call = b.call(`$.bind_files`, context.state.node, get, set);
 				break;
 
 			case 'this':
@@ -160,18 +172,18 @@ export function BindDirective(node, context) {
 					'$.bind_content_editable',
 					b.literal(node.name),
 					context.state.node,
-					getter,
-					setter
+					get,
+					set
 				);
 				break;
 
 			// checkbox/radio
 			case 'checked':
-				call = b.call(`$.bind_checked`, context.state.node, getter, setter);
+				call = b.call(`$.bind_checked`, context.state.node, get, set);
 				break;
 
 			case 'focused':
-				call = b.call(`$.bind_focused`, context.state.node, setter);
+				call = b.call(`$.bind_focused`, context.state.node, set ?? get);
 				break;
 
 			case 'group': {
@@ -184,7 +196,7 @@ export function BindDirective(node, context) {
 
 				// We need to additionally invoke the value attribute signal to register it as a dependency,
 				// so that when the value is updated, the group binding is updated
-				let group_getter = getter;
+				let group_getter = get;
 
 				if (parent?.type === 'RegularElement') {
 					const value = /** @type {any[]} */ (
@@ -215,7 +227,7 @@ export function BindDirective(node, context) {
 					b.array(indexes),
 					context.state.node,
 					group_getter,
-					setter
+					set ?? get
 				);
 				break;
 			}

packages/svelte/src/compiler/utils/builders.js

@@ -419,19 +419,31 @@ export function template(elements, expressions) {
  * @returns {ESTree.Expression}
  */
 export function thunk(expression, async = false) {
+	const fn = arrow([], expression);
+	if (async) fn.async = true;
+	return unthunk(fn);
+}
+
+/**
+ * Replace "(arg) => func(arg)" to "func"
+ * @param {ESTree.Expression} expression
+ * @returns {ESTree.Expression}
+ */
+export function unthunk(expression) {
 	if (
-		expression.type === 'CallExpression' &&
-		expression.callee.type !== 'Super' &&
-		expression.callee.type !== 'MemberExpression' &&
-		expression.callee.type !== 'CallExpression' &&
-		expression.arguments.length === 0
+		expression.type === 'ArrowFunctionExpression' &&
+		expression.async === false &&
+		expression.body.type === 'CallExpression' &&
+		expression.body.callee.type === 'Identifier' &&
+		expression.params.length === expression.body.arguments.length &&
+		expression.params.every((param, index) => {
+			const arg = /** @type {ESTree.SimpleCallExpression} */ (expression.body).arguments[index];
+			return param.type === 'Identifier' && arg.type === 'Identifier' && param.name === arg.name;
+		})
 	) {
-		return expression.callee;
+		return expression.body.callee;
 	}
-
-	const fn = arrow([], expression);
-	if (async) fn.async = true;
-	return fn;
+	return expression;
 }
 
 /**

packages/svelte/src/internal/client/dom/elements/bindings/input.js

@@ -8,18 +8,18 @@ import { hydrating } from '../../hydration.js';
 
 /**
  * @param {HTMLInputElement} input
- * @param {() => unknown} get_value
- * @param {(value: unknown) => void} update
+ * @param {() => unknown} get
+ * @param {(value: unknown) => void} set
  * @returns {void}
  */
-export function bind_value(input, get_value, update) {
+export function bind_value(input, get, set = get) {
 	listen_to_event_and_reset_event(input, 'input', () => {
 		if (DEV && input.type === 'checkbox') {
 			// TODO should this happen in prod too?
 			e.bind_invalid_checkbox_value();
 		}
 
-		update(is_numberlike_input(input) ? to_number(input.value) : input.value);
+		set(is_numberlike_input(input) ? to_number(input.value) : input.value);
 	});
 
 	render_effect(() => {
@@ -28,12 +28,12 @@ export function bind_value(input, get_value, update) {
 			e.bind_invalid_checkbox_value();
 		}
 
-		var value = get_value();
+		var value = get();
 
 		// If we are hydrating and the value has since changed, then use the update value
 		// from the input instead.
 		if (hydrating && input.defaultValue !== input.value) {
-			update(input.value);
+			set(input.value);
 			return;
 		}
 
@@ -60,11 +60,11 @@ const pending = new Set();
  * @param {HTMLInputElement[]} inputs
  * @param {null | [number]} group_index
  * @param {HTMLInputElement} input
- * @param {() => unknown} get_value
- * @param {(value: unknown) => void} update
+ * @param {() => unknown} get
+ * @param {(value: unknown) => void} set
  * @returns {void}
  */
-export function bind_group(inputs, group_index, input, get_value, update) {
+export function bind_group(inputs, group_index, input, get, set = get) {
 	var is_checkbox = input.getAttribute('type') === 'checkbox';
 	var binding_group = inputs;
 
@@ -91,14 +91,14 @@ export function bind_group(inputs, group_index, input, get_value, update) {
 				value = get_binding_group_value(binding_group, value, input.checked);
 			}
 
-			update(value);
+			set(value);
 		},
 		// TODO better default value handling
-		() => update(is_checkbox ? [] : null)
+		() => set(is_checkbox ? [] : null)
 	);
 
 	render_effect(() => {
-		var value = get_value();
+		var value = get();
 
 		// If we are hydrating and the value has since changed, then use the update value
 		// from the input instead.
@@ -147,29 +147,29 @@ export function bind_group(inputs, group_index, input, get_value, update) {
 				value = hydration_input?.__value;
 			}
 
-			update(value);
+			set(value);
 		}
 	});
 }
 
 /**
  * @param {HTMLInputElement} input
- * @param {() => unknown} get_value
- * @param {(value: unknown) => void} update
+ * @param {() => unknown} get
+ * @param {(value: unknown) => void} set
  * @returns {void}
  */
-export function bind_checked(input, get_value, update) {
+export function bind_checked(input, get, set = get) {
 	listen_to_event_and_reset_event(input, 'change', () => {
 		var value = input.checked;
-		update(value);
+		set(value);
 	});
 
-	if (get_value() == undefined) {
-		update(false);
+	if (get() == undefined) {
+		set(false);
 	}
 
 	render_effect(() => {
-		var value = get_value();
+		var value = get();
 		input.checked = Boolean(value);
 	});
 }
@@ -215,15 +215,15 @@ function to_number(value) {
 
 /**
  * @param {HTMLInputElement} input
- * @param {() => FileList | null} get_value
- * @param {(value: FileList | null) => void} update
+ * @param {() => FileList | null} get
+ * @param {(value: FileList | null) => void} set
  */
-export function bind_files(input, get_value, update) {
+export function bind_files(input, get, set = get) {
 	listen_to_event_and_reset_event(input, 'change', () => {
-		update(input.files);
+		set(input.files);
 	});
 
 	render_effect(() => {
-		input.files = get_value();
+		input.files = get();
 	});
 }