feat: simpler effect DOM boundaries (#12258)

* simpler effect dom boundaries

* remove unused argument

* tidy up

* simplify

* skip redundant comment templates for components (and others TODO)

* same optimisation for render tags

* DRY out

* appease typescript

* changeset

* tighten up, leave note to self

* reinstate $.comment optimisation

* add explanation

* comments

Author: Rich-Harris

.changeset/beige-gifts-appear.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: simpler effect DOM boundaries

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

@@ -36,7 +36,6 @@ import {
 	EACH_KEYED,
 	is_capture_event,
 	TEMPLATE_FRAGMENT,
-	TEMPLATE_UNSET_START,
 	TEMPLATE_USE_IMPORT_NODE,
 	TRANSITION_GLOBAL,
 	TRANSITION_IN,
@@ -1561,7 +1560,7 @@ export const template_visitors = {
 
 		const namespace = infer_namespace(context.state.metadata.namespace, parent, node.nodes);
 
-		const { hoisted, trimmed } = clean_nodes(
+		const { hoisted, trimmed, is_standalone } = clean_nodes(
 			parent,
 			node.nodes,
 			context.path,
@@ -1676,56 +1675,38 @@ export const template_visitors = {
 				);
 				close = b.stmt(b.call('$.append', b.id('$$anchor'), id));
 			} else {
-				/** @type {(is_text: boolean) => import('estree').Expression} */
-				const expression = (is_text) =>
-					is_text ? b.call('$.first_child', id, b.true) : b.call('$.first_child', id);
-
-				process_children(trimmed, expression, false, { ...context, state });
-
-				var first = trimmed[0];
-
-				/**
-				 * If the first item in an effect is a static slot or render tag, it will clone
-				 * a template but without creating a child effect. In these cases, we need to keep
-				 * the current `effect.nodes.start` undefined, so that it can be populated by
-				 * the item in question
-				 * TODO come up with a better name than `unset`
-				 */
-				var unset = false;
-
-				if (first.type === 'SlotElement') unset = true;
-				if (first.type === 'RenderTag' && !first.metadata.dynamic) unset = true;
-				if (first.type === 'Component' && !first.metadata.dynamic && !context.state.options.hmr) {
-					unset = true;
-				}
+				if (is_standalone) {
+					// no need to create a template, we can just use the existing block's anchor
+					process_children(trimmed, () => b.id('$$anchor'), false, { ...context, state });
+				} else {
+					/** @type {(is_text: boolean) => import('estree').Expression} */
+					const expression = (is_text) =>
+						is_text ? b.call('$.first_child', id, b.true) : b.call('$.first_child', id);
 
-				const use_comment_template = state.template.length === 1 && state.template[0] === '<!>';
+					process_children(trimmed, expression, false, { ...context, state });
 
-				if (use_comment_template) {
-					// special case — we can use `$.comment` instead of creating a unique template
-					body.push(b.var(id, b.call('$.comment', unset && b.literal(unset))));
-				} else {
 					let flags = TEMPLATE_FRAGMENT;
 
-					if (unset) {
-						flags |= TEMPLATE_UNSET_START;
-					}
-
 					if (state.metadata.context.template_needs_import_node) {
 						flags |= TEMPLATE_USE_IMPORT_NODE;
 					}
 
-					add_template(template_name, [
-						b.template([b.quasi(state.template.join(''), true)], []),
-						b.literal(flags)
-					]);
+					if (state.template.length === 1 && state.template[0] === '<!>') {
+						// special case — we can use `$.comment` instead of creating a unique template
+						body.push(b.var(id, b.call('$.comment')));
+					} else {
+						add_template(template_name, [
+							b.template([b.quasi(state.template.join(''), true)], []),
+							b.literal(flags)
+						]);
+
+						body.push(b.var(id, b.call(template_name)));
+					}
 
-					body.push(b.var(id, b.call(template_name)));
+					close = b.stmt(b.call('$.append', b.id('$$anchor'), id));
 				}
 
 				body.push(...state.before_init, ...state.init);
-
-				close = b.stmt(b.call('$.append', b.id('$$anchor'), id));
 			}
 		} else {
 			body.push(...state.before_init, ...state.init);

packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

@@ -1002,6 +1002,8 @@ function serialize_inline_component(node, expression, context) {
 			)
 		);
 
+		context.state.template.push(statement);
+	} else if (context.state.skip_hydration_boundaries) {
 		context.state.template.push(statement);
 	} else {
 		context.state.template.push(block_open, statement, block_close);
@@ -1112,7 +1114,7 @@ const template_visitors = {
 		const parent = context.path.at(-1) ?? node;
 		const namespace = infer_namespace(context.state.namespace, parent, node.nodes);
 
-		const { hoisted, trimmed } = clean_nodes(
+		const { hoisted, trimmed, is_standalone } = clean_nodes(
 			parent,
 			node.nodes,
 			context.path,
@@ -1127,7 +1129,8 @@ const template_visitors = {
 			...context.state,
 			init: [],
 			template: [],
-			namespace
+			namespace,
+			skip_hydration_boundaries: is_standalone
 		};
 
 		for (const node of hoisted) {
@@ -1180,17 +1183,23 @@ const template_visitors = {
 			return /** @type {import('estree').Expression} */ (context.visit(arg));
 		});
 
+		if (!context.state.skip_hydration_boundaries) {
+			context.state.template.push(block_open);
+		}
+
 		context.state.template.push(
-			block_open,
 			b.stmt(
 				(node.expression.type === 'CallExpression' ? b.call : b.maybe_call)(
 					snippet_function,
 					b.id('$$payload'),
 					...snippet_args
 				)
-			),
-			block_close
+			)
 		);
+
+		if (!context.state.skip_hydration_boundaries) {
+			context.state.template.push(block_close);
+		}
 	},
 	ClassDirective() {
 		throw new Error('Node should have been handled elsewhere');
@@ -1925,7 +1934,8 @@ export function server_component(analysis, options) {
 		template: /** @type {any} */ (null),
 		namespace: options.namespace,
 		preserve_whitespace: options.preserveWhitespace,
-		private_derived: new Map()
+		private_derived: new Map(),
+		skip_hydration_boundaries: false
 	};
 
 	const module = /** @type {import('estree').Program} */ (

packages/svelte/src/compiler/phases/3-transform/server/types.d.ts

@@ -22,6 +22,7 @@ export interface ComponentServerTransformState extends ServerTransformState {
 	readonly template: Array<Statement | Expression>;
 	readonly namespace: Namespace;
 	readonly preserve_whitespace: boolean;
+	readonly skip_hydration_boundaries: boolean;
 }
 
 export type Context = import('zimmerframe').Context<SvelteNode, ServerTransformState>;

packages/svelte/src/compiler/phases/3-transform/utils.js

@@ -185,83 +185,105 @@ export function clean_nodes(
 		}
 	}
 
-	if (preserve_whitespace) {
-		return { hoisted, trimmed: regular };
-	}
+	let trimmed = regular;
 
-	let first, last;
+	if (!preserve_whitespace) {
+		trimmed = [];
 
-	while ((first = regular[0]) && first.type === 'Text' && !regex_not_whitespace.test(first.data)) {
-		regular.shift();
-	}
+		let first, last;
 
-	if (first?.type === 'Text') {
-		first.raw = first.raw.replace(regex_starts_with_whitespaces, '');
-		first.data = first.data.replace(regex_starts_with_whitespaces, '');
-	}
+		while (
+			(first = regular[0]) &&
+			first.type === 'Text' &&
+			!regex_not_whitespace.test(first.data)
+		) {
+			regular.shift();
+		}
 
-	while ((last = regular.at(-1)) && last.type === 'Text' && !regex_not_whitespace.test(last.data)) {
-		regular.pop();
-	}
+		if (first?.type === 'Text') {
+			first.raw = first.raw.replace(regex_starts_with_whitespaces, '');
+			first.data = first.data.replace(regex_starts_with_whitespaces, '');
+		}
 
-	if (last?.type === 'Text') {
-		last.raw = last.raw.replace(regex_ends_with_whitespaces, '');
-		last.data = last.data.replace(regex_ends_with_whitespaces, '');
-	}
+		while (
+			(last = regular.at(-1)) &&
+			last.type === 'Text' &&
+			!regex_not_whitespace.test(last.data)
+		) {
+			regular.pop();
+		}
 
-	const can_remove_entirely =
-		(namespace === 'svg' &&
-			(parent.type !== 'RegularElement' || parent.name !== 'text') &&
-			!path.some((n) => n.type === 'RegularElement' && n.name === 'text')) ||
-		(parent.type === 'RegularElement' &&
-			// TODO others?
-			(parent.name === 'select' ||
-				parent.name === 'tr' ||
-				parent.name === 'table' ||
-				parent.name === 'tbody' ||
-				parent.name === 'thead' ||
-				parent.name === 'tfoot' ||
-				parent.name === 'colgroup' ||
-				parent.name === 'datalist'));
+		if (last?.type === 'Text') {
+			last.raw = last.raw.replace(regex_ends_with_whitespaces, '');
+			last.data = last.data.replace(regex_ends_with_whitespaces, '');
+		}
 
-	/** @type {Compiler.SvelteNode[]} */
-	const trimmed = [];
-
-	// Replace any whitespace between a text and non-text node with a single spaceand keep whitespace
-	// as-is within text nodes, or between text nodes and expression tags (because in the end they count
-	// as one text). This way whitespace is mostly preserved when using CSS with `white-space: pre-line`
-	// and default slot content going into a pre tag (which we can't see).
-	for (let i = 0; i < regular.length; i++) {
-		const prev = regular[i - 1];
-		const node = regular[i];
-		const next = regular[i + 1];
-
-		if (node.type === 'Text') {
-			if (prev?.type !== 'ExpressionTag') {
-				const prev_is_text_ending_with_whitespace =
-					prev?.type === 'Text' && regex_ends_with_whitespaces.test(prev.data);
-				node.data = node.data.replace(
-					regex_starts_with_whitespaces,
-					prev_is_text_ending_with_whitespace ? '' : ' '
-				);
-				node.raw = node.raw.replace(
-					regex_starts_with_whitespaces,
-					prev_is_text_ending_with_whitespace ? '' : ' '
-				);
-			}
-			if (next?.type !== 'ExpressionTag') {
-				node.data = node.data.replace(regex_ends_with_whitespaces, ' ');
-				node.raw = node.raw.replace(regex_ends_with_whitespaces, ' ');
-			}
-			if (node.data && (node.data !== ' ' || !can_remove_entirely)) {
+		const can_remove_entirely =
+			(namespace === 'svg' &&
+				(parent.type !== 'RegularElement' || parent.name !== 'text') &&
+				!path.some((n) => n.type === 'RegularElement' && n.name === 'text')) ||
+			(parent.type === 'RegularElement' &&
+				// TODO others?
+				(parent.name === 'select' ||
+					parent.name === 'tr' ||
+					parent.name === 'table' ||
+					parent.name === 'tbody' ||
+					parent.name === 'thead' ||
+					parent.name === 'tfoot' ||
+					parent.name === 'colgroup' ||
+					parent.name === 'datalist'));
+
+		// Replace any whitespace between a text and non-text node with a single spaceand keep whitespace
+		// as-is within text nodes, or between text nodes and expression tags (because in the end they count
+		// as one text). This way whitespace is mostly preserved when using CSS with `white-space: pre-line`
+		// and default slot content going into a pre tag (which we can't see).
+		for (let i = 0; i < regular.length; i++) {
+			const prev = regular[i - 1];
+			const node = regular[i];
+			const next = regular[i + 1];
+
+			if (node.type === 'Text') {
+				if (prev?.type !== 'ExpressionTag') {
+					const prev_is_text_ending_with_whitespace =
+						prev?.type === 'Text' && regex_ends_with_whitespaces.test(prev.data);
+					node.data = node.data.replace(
+						regex_starts_with_whitespaces,
+						prev_is_text_ending_with_whitespace ? '' : ' '
+					);
+					node.raw = node.raw.replace(
+						regex_starts_with_whitespaces,
+						prev_is_text_ending_with_whitespace ? '' : ' '
+					);
+				}
+				if (next?.type !== 'ExpressionTag') {
+					node.data = node.data.replace(regex_ends_with_whitespaces, ' ');
+					node.raw = node.raw.replace(regex_ends_with_whitespaces, ' ');
+				}
+				if (node.data && (node.data !== ' ' || !can_remove_entirely)) {
+					trimmed.push(node);
+				}
+			} else {
 				trimmed.push(node);
 			}
-		} else {
-			trimmed.push(node);
 		}
 	}
 
-	return { hoisted, trimmed };
+	var first = trimmed[0];
+
+	/**
+	 * In a case like `{#if x}<Foo />{/if}`, we don't need to wrap the child in
+	 * comments — we can just use the parent block's anchor for the component.
+	 * TODO extend this optimisation to other cases
+	 */
+	const is_standalone =
+		trimmed.length === 1 &&
+		((first.type === 'RenderTag' && !first.metadata.dynamic) ||
+			(first.type === 'Component' &&
+				!first.attributes.some(
+					(attribute) => attribute.type === 'Attribute' && attribute.name.startsWith('--')
+				)));
+
+	return { hoisted, trimmed, is_standalone };
 }
 
 /**

packages/svelte/src/constants.js

@@ -18,7 +18,6 @@ export const TRANSITION_GLOBAL = 1 << 2;
 
 export const TEMPLATE_FRAGMENT = 1;
 export const TEMPLATE_USE_IMPORT_NODE = 1 << 1;
-export const TEMPLATE_UNSET_START = 1 << 2;
 
 export const HYDRATION_START = '[';
 export const HYDRATION_END = ']';

packages/svelte/src/internal/client/dev/hmr.js

@@ -1,4 +1,5 @@
 /** @import { Source, Effect } from '#client' */
+import { empty } from '../dom/operations.js';
 import { block, branch, destroy_effect } from '../reactivity/effects.js';
 import { set_should_intro } from '../render.js';
 import { get } from '../runtime.js';
@@ -19,7 +20,7 @@ export function hmr(source) {
 		/** @type {Effect} */
 		let effect;
 
-		block(anchor, 0, () => {
+		block(() => {
 			const component = get(source);
 
 			if (effect) {

packages/svelte/src/internal/client/dom/blocks/await.js

@@ -105,7 +105,7 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) {
 		}
 	}
 
-	var effect = block(anchor, 0, () => {
+	var effect = block(() => {
 		if (input === (input = get_input())) return;
 
 		if (is_promise(input)) {