fix: store DOM boundaries on effects (#12215)

* WIP

* progress

* fix

* add comment

* update tests

* mostly fix dynamic elements

* delete unused code

* remove unused code

* more

* tidy up

* fix

* more

* relink effects inside each block

* simpler to just leave these in (without children) and ignore them

* fix head stuff

* tidy up

* fix some type errors

* simplify

* use hydration marker as effect boundary where possible

* all tests passing

* tidy up

* tidy up a bit

* tidy up

* beat typescript into submission

* bring tests over from fix-each-dom-bug

* tweaks

* simplify a tad

* tidy up

* simplify

* reduce indirection

* belt and braces

* tidy

* revert config change

* missed a spot

* regenerate types

* cleaner separation between EachState and EachItem - precursor to efficient relinking

* HMR fix

* set effects

* FINALLY

Author: Rich-Harris

packages/svelte/src/compiler/phases/1-parse/state/tag.js

@@ -594,7 +594,10 @@ function special(parser) {
 			type: 'RenderTag',
 			start,
 			end: parser.index,
-			expression: expression
+			expression: expression,
+			metadata: {
+				dynamic: false
+			}
 		});
 	}
 }

packages/svelte/src/compiler/phases/2-analyze/index.js

@@ -1520,6 +1520,13 @@ const common_visitors = {
 				return;
 			}
 		}
+	},
+	Component(node, context) {
+		const binding = context.state.scope.get(
+			node.name.includes('.') ? node.name.slice(0, node.name.indexOf('.')) : node.name
+		);
+
+		node.metadata.dynamic = binding !== null && binding.kind !== 'normal';
 	}
 };
 

packages/svelte/src/compiler/phases/2-analyze/validation.js

@@ -633,6 +633,11 @@ const validation = {
 		});
 	},
 	RenderTag(node, context) {
+		const callee = unwrap_optional(node.expression).callee;
+
+		node.metadata.dynamic =
+			callee.type !== 'Identifier' || context.state.scope.get(callee.name)?.kind !== 'normal';
+
 		context.state.analysis.uses_render_tags = true;
 
 		const raw_args = unwrap_optional(node.expression).arguments;
@@ -642,7 +647,6 @@ const validation = {
 			}
 		}
 
-		const callee = unwrap_optional(node.expression).callee;
 		if (
 			callee.type === 'MemberExpression' &&
 			callee.property.type === 'Identifier' &&

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

@@ -36,6 +36,7 @@ import {
 	EACH_KEYED,
 	is_capture_event,
 	TEMPLATE_FRAGMENT,
+	TEMPLATE_UNSET_START,
 	TEMPLATE_USE_IMPORT_NODE,
 	TRANSITION_GLOBAL,
 	TRANSITION_IN,
@@ -942,6 +943,7 @@ function serialize_inline_component(node, component_name, context) {
 		fn = (node_id) => {
 			return b.call(
 				'$.component',
+				node_id,
 				b.thunk(/** @type {import('estree').Expression} */ (context.visit(node.expression))),
 				b.arrow(
 					[b.id(component_name)],
@@ -1680,14 +1682,35 @@ export const template_visitors = {
 
 				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;
+				}
+
 				const use_comment_template = state.template.length === 1 && state.template[0] === '<!>';
 
 				if (use_comment_template) {
 					// special case — we can use `$.comment` instead of creating a unique template
-					body.push(b.var(id, b.call('$.comment')));
+					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;
 					}
@@ -1832,27 +1855,26 @@ export const template_visitors = {
 		context.state.template.push('<!>');
 		const callee = unwrap_optional(node.expression).callee;
 		const raw_args = unwrap_optional(node.expression).arguments;
-		const is_reactive =
-			callee.type !== 'Identifier' || context.state.scope.get(callee.name)?.kind !== 'normal';
 
-		/** @type {import('estree').Expression[]} */
-		const args = [context.state.node];
-		for (const arg of raw_args) {
-			args.push(b.thunk(/** @type {import('estree').Expression} */ (context.visit(arg))));
-		}
+		const args = raw_args.map((arg) =>
+			b.thunk(/** @type {import('estree').Expression} */ (context.visit(arg)))
+		);
 
 		let snippet_function = /** @type {import('estree').Expression} */ (context.visit(callee));
 		if (context.state.options.dev) {
 			snippet_function = b.call('$.validate_snippet', snippet_function);
 		}
 
-		if (is_reactive) {
-			context.state.init.push(b.stmt(b.call('$.snippet', b.thunk(snippet_function), ...args)));
+		if (node.metadata.dynamic) {
+			context.state.init.push(
+				b.stmt(b.call('$.snippet', context.state.node, b.thunk(snippet_function), ...args))
+			);
 		} else {
 			context.state.init.push(
 				b.stmt(
 					(node.expression.type === 'CallExpression' ? b.call : b.maybe_call)(
 						snippet_function,
+						context.state.node,
 						...args
 					)
 				)
@@ -1915,7 +1937,7 @@ export const template_visitors = {
 		}
 
 		if (node.name === 'noscript') {
-			context.state.template.push('<!>');
+			context.state.template.push('<noscript></noscript>');
 			return;
 		}
 		if (node.name === 'script') {
@@ -2985,16 +3007,14 @@ export const template_visitors = {
 		}
 	},
 	Component(node, context) {
-		const binding = context.state.scope.get(
-			node.name.includes('.') ? node.name.slice(0, node.name.indexOf('.')) : node.name
-		);
-		if (binding !== null && binding.kind !== 'normal') {
+		if (node.metadata.dynamic) {
 			// Handle dynamic references to what seems like static inline components
 			const component = serialize_inline_component(node, '$$component', context);
 			context.state.init.push(
 				b.stmt(
 					b.call(
 						'$.component',
+						context.state.node,
 						// TODO use untrack here to not update when binding changes?
 						// Would align with Svelte 4 behavior, but it's arguably nicer/expected to update this
 						b.thunk(
@@ -3006,6 +3026,7 @@ export const template_visitors = {
 			);
 			return;
 		}
+
 		const component = serialize_inline_component(node, node.name, context);
 		context.state.init.push(component);
 	},

packages/svelte/src/compiler/types/template.d.ts

@@ -152,6 +152,9 @@ export interface DebugTag extends BaseNode {
 export interface RenderTag extends BaseNode {
 	type: 'RenderTag';
 	expression: SimpleCallExpression | (ChainExpression & { expression: SimpleCallExpression });
+	metadata: {
+		dynamic: boolean;
+	};
 }
 
 type Tag = ExpressionTag | HtmlTag | ConstTag | DebugTag | RenderTag;
@@ -271,6 +274,9 @@ interface BaseElement extends BaseNode {
 
 export interface Component extends BaseElement {
 	type: 'Component';
+	metadata: {
+		dynamic: boolean;
+	};
 }
 
 interface TitleElement extends BaseElement {

packages/svelte/src/constants.js

@@ -18,6 +18,7 @@ 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/constants.js

@@ -17,6 +17,7 @@ export const EFFECT_TRANSPARENT = 1 << 15;
 /** Svelte 4 legacy mode props need to be handled with deriveds and be recognized elsewhere, hence the dedicated flag */
 export const LEGACY_DERIVED_PROP = 1 << 16;
 export const INSPECT_EFFECT = 1 << 17;
+export const HEAD_EFFECT = 1 << 18;
 
 export const STATE_SYMBOL = Symbol('$state');
 export const STATE_FROZEN_SYMBOL = Symbol('$state.frozen');

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

@@ -18,7 +18,7 @@ export function hmr(source) {
 		/** @type {import("#client").Effect} */
 		let effect;
 
-		block(() => {
+		block(anchor, 0, () => {
 			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(() => {
+	var effect = block(anchor, 0, () => {
 		if (input === (input = get_input())) return;
 
 		if (is_promise(input)) {