fix: improve hydration comments for `$props.id`

.changeset/brown-rocks-swim.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: improve hydration comments for `$props.id`

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

@@ -250,7 +250,8 @@ export function analyze_module(ast, options) {
 		accessors: false,
 		runes: true,
 		immutable: true,
-		tracing: false
+		tracing: false,
+		props_id_needs_hydration: true
 	};
 
 	walk(
@@ -269,7 +270,8 @@ export function analyze_module(ast, options) {
 			has_props_rune: false,
 			options: /** @type {ValidatedCompileOptions} */ (options),
 			parent_element: null,
-			reactive_statement: null
+			reactive_statement: null,
+			props_id_needs_hydration: { value: true }
 		},
 		visitors
 	);
@@ -460,7 +462,8 @@ export function analyze_component(root, source, options) {
 		source,
 		undefined_exports: new Map(),
 		snippet_renderers: new Map(),
-		snippets: new Set()
+		snippets: new Set(),
+		props_id_needs_hydration: true
 	};
 
 	if (!runes) {
@@ -618,10 +621,16 @@ export function analyze_component(root, source, options) {
 				expression: null,
 				derived_state: [],
 				function_depth: scope.function_depth,
-				reactive_statement: null
+				reactive_statement: null,
+				// we need to use an object instead of the value because state is spread by the global visitor
+				props_id_needs_hydration: { value: true }
 			};
 
 			walk(/** @type {AST.SvelteNode} */ (ast), state, visitors);
+
+			if (!state.props_id_needs_hydration.value) {
+				analysis.props_id_needs_hydration = false;
+			}
 		}
 
 		// warn on any nonstate declarations that are a) reassigned and b) referenced in the template
@@ -684,7 +693,9 @@ export function analyze_component(root, source, options) {
 				component_slots: new Set(),
 				expression: null,
 				derived_state: [],
-				function_depth: scope.function_depth
+				function_depth: scope.function_depth,
+				// we need to use an object instead of the value because state is spread by the global visitor
+				props_id_needs_hydration: { value: true }
 			};
 
 			walk(/** @type {AST.SvelteNode} */ (ast), state, visitors);

packages/svelte/src/compiler/phases/2-analyze/types.d.ts

@@ -20,6 +20,8 @@ export interface AnalysisState {
 	expression: ExpressionMetadata | null;
 	derived_state: { name: string; private: boolean }[];
 	function_depth: number;
+	// we need to use an object instead of the value because state is spread by the global visitor
+	props_id_needs_hydration: { value: boolean };
 
 	// legacy stuff
 	reactive_statement: null | ReactiveStatement;

packages/svelte/src/compiler/phases/2-analyze/utils/props_id_needs_hydration.js

@@ -0,0 +1,21 @@
+/**
+ * @param {import("#compiler").AST.BaseNode} node
+ * @param {import("../types").Context["path"]} path
+ */
+export function props_id_needs_hydration(node, path) {
+	return (
+		path.length === 1 &&
+		path[0].type === 'Fragment' &&
+		path[0].nodes &&
+		first_non_blank_text(path[0]) === node
+	);
+}
+
+/**
+ * @param {import("#compiler").AST.Fragment} fragment
+ */
+function first_non_blank_text(fragment) {
+	return fragment.nodes[0].type === 'Text' && fragment.nodes[0].data.trim() !== ''
+		? fragment.nodes[0]
+		: fragment.nodes[1];
+}

packages/svelte/src/compiler/phases/2-analyze/visitors/AwaitBlock.js

@@ -3,12 +3,16 @@
 import { validate_block_not_empty, validate_opening_tag } from './shared/utils.js';
 import * as e from '../../../errors.js';
 import { mark_subtree_dynamic } from './shared/fragment.js';
+import { props_id_needs_hydration } from '../utils/props_id_needs_hydration.js';
 
 /**
  * @param {AST.AwaitBlock} node
  * @param {Context} context
  */
 export function AwaitBlock(node, context) {
+	if (props_id_needs_hydration(node, context.path)) {
+		context.state.props_id_needs_hydration.value = false;
+	}
 	validate_block_not_empty(node.pending, context);
 	validate_block_not_empty(node.then, context);
 	validate_block_not_empty(node.catch, context);

packages/svelte/src/compiler/phases/2-analyze/visitors/KeyBlock.js

@@ -1,5 +1,6 @@
 /** @import { AST } from '#compiler' */
 /** @import { Context } from '../types' */
+import { props_id_needs_hydration } from '../utils/props_id_needs_hydration.js';
 import { mark_subtree_dynamic } from './shared/fragment.js';
 import { validate_block_not_empty, validate_opening_tag } from './shared/utils.js';
 
@@ -8,6 +9,9 @@ import { validate_block_not_empty, validate_opening_tag } from './shared/utils.j
  * @param {Context} context
  */
 export function KeyBlock(node, context) {
+	if (props_id_needs_hydration(node, context.path)) {
+		context.state.props_id_needs_hydration.value = false;
+	}
 	validate_block_not_empty(node.fragment, context);
 
 	if (context.state.analysis.runes) {

packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js

@@ -3,12 +3,16 @@
 import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js';
 import { regex_not_whitespace } from '../../patterns.js';
 import * as e from '../../../errors.js';
+import { props_id_needs_hydration } from '../utils/props_id_needs_hydration.js';
 
 /**
  * @param {AST.Text} node
  * @param {Context} context
  */
 export function Text(node, context) {
+	if (props_id_needs_hydration(node, context.path)) {
+		context.state.props_id_needs_hydration.value = false;
+	}
 	const in_template = context.path.at(-1)?.type === 'Fragment';
 
 	if (in_template && context.state.parent_element && regex_not_whitespace.test(node.data)) {

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

@@ -564,7 +564,10 @@ export function client_component(analysis, options) {
 
 	if (analysis.props_id) {
 		// need to be placed on first line of the component for hydration
-		component_block.body.unshift(b.const(analysis.props_id, b.call('$.props_id')));
+		component_block.body.unshift(
+			// if it needs hydration we need to also call next inside `props_id`
+			b.const(analysis.props_id, b.call('$.props_id', analysis.props_id_needs_hydration && b.true))
+		);
 	}
 
 	if (state.events.size > 0) {

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

@@ -3,10 +3,10 @@
 /** @import { ComponentServerTransformState, ComponentVisitors, ServerTransformState, Visitors } from './types.js' */
 /** @import { Analysis, ComponentAnalysis } from '../../types.js' */
 import { walk } from 'zimmerframe';
-import { set_scope } from '../../scope.js';
+import { dev, filename } from '../../../state.js';
 import { extract_identifiers } from '../../../utils/ast.js';
 import * as b from '../../../utils/builders.js';
-import { dev, filename } from '../../../state.js';
+import { set_scope } from '../../scope.js';
 import { render_stylesheet } from '../css/index.js';
 import { AssignmentExpression } from './visitors/AssignmentExpression.js';
 import { AwaitBlock } from './visitors/AwaitBlock.js';
@@ -30,6 +30,7 @@ import { RenderTag } from './visitors/RenderTag.js';
 import { SlotElement } from './visitors/SlotElement.js';
 import { SnippetBlock } from './visitors/SnippetBlock.js';
 import { SpreadAttribute } from './visitors/SpreadAttribute.js';
+import { SvelteBoundary } from './visitors/SvelteBoundary.js';
 import { SvelteComponent } from './visitors/SvelteComponent.js';
 import { SvelteElement } from './visitors/SvelteElement.js';
 import { SvelteFragment } from './visitors/SvelteFragment.js';
@@ -38,7 +39,6 @@ import { SvelteSelf } from './visitors/SvelteSelf.js';
 import { TitleElement } from './visitors/TitleElement.js';
 import { UpdateExpression } from './visitors/UpdateExpression.js';
 import { VariableDeclaration } from './visitors/VariableDeclaration.js';
-import { SvelteBoundary } from './visitors/SvelteBoundary.js';
 
 /** @type {Visitors} */
 const global_visitors = {
@@ -100,7 +100,9 @@ export function server_component(analysis, options) {
 		namespace: options.namespace,
 		preserve_whitespace: options.preserveWhitespace,
 		private_derived: new Map(),
-		skip_hydration_boundaries: false
+		skip_hydration_boundaries: false,
+		props_id: analysis.props_id?.name,
+		props_id_needs_hydration: analysis.props_id_needs_hydration
 	};
 
 	const module = /** @type {Program} */ (
@@ -247,7 +249,10 @@ export function server_component(analysis, options) {
 	if (analysis.props_id) {
 		// need to be placed on first line of the component for hydration
 		component_block.body.unshift(
-			b.const(analysis.props_id, b.call('$.props_id', b.id('$$payload')))
+			b.const(
+				analysis.props_id,
+				b.call('$.props_id', b.id('$$payload'), analysis.props_id_needs_hydration && b.true)
+			)
 		);
 	}
 

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

@@ -13,6 +13,8 @@ export interface ServerTransformState extends TransformState {
 export interface ComponentServerTransformState extends ServerTransformState {
 	readonly analysis: ComponentAnalysis;
 	readonly options: ValidatedCompileOptions;
+	readonly props_id?: string;
+	readonly props_id_needs_hydration: boolean;
 
 	readonly init: Statement[];
 

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

@@ -42,5 +42,14 @@ export function Fragment(node, context) {
 
 	process_children(trimmed, { ...context, state });
 
-	return b.block([...state.init, ...build_template(state.template)]);
+	return b.block([
+		...state.init,
+		...build_template(
+			state.template,
+			undefined,
+			undefined,
+			state.props_id,
+			state.props_id_needs_hydration
+		)
+	]);
 }

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

@@ -95,9 +95,17 @@ function is_statement(node) {
  * @param {Array<Statement | Expression>} template
  * @param {Identifier} out
  * @param {AssignmentOperator} operator
+ * @param {string} [props_id]
+ * @param {boolean} [props_id_needs_hydration]
  * @returns {Statement[]}
  */
-export function build_template(template, out = b.id('$$payload.out'), operator = '+=') {
+export function build_template(
+	template,
+	out = b.id('$$payload.out'),
+	operator = '+=',
+	props_id,
+	props_id_needs_hydration
+) {
 	/** @type {string[]} */
 	let strings = [];
 
@@ -139,7 +147,18 @@ export function build_template(template, out = b.id('$$payload.out'), operator =
 			}
 
 			if (node.type === 'Literal') {
+				if (
+					i === 0 &&
+					node.value === empty_comment.value &&
+					!props_id_needs_hydration &&
+					props_id != null
+				) {
+					strings[strings.length - 1] += `<!--#`;
+					expressions.push(b.id(props_id));
+					strings.push(`-->`);
+				} else {
 					strings[strings.length - 1] += node.value;
+				}
 			} else if (node.type === 'TemplateLiteral') {
 				strings[strings.length - 1] += node.quasis[0].value.cooked;
 				strings.push(...node.quasis.slice(1).map((q) => /** @type {string} */ (q.value.cooked)));

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

@@ -28,6 +28,7 @@ export interface Analysis {
 	runes: boolean;
 	immutable: boolean;
 	tracing: boolean;
+	props_id_needs_hydration: boolean;
 
 	// TODO figure out if we can move this to ComponentAnalysis
 	accessors: boolean;

packages/svelte/src/internal/client/dom/template.js

@@ -258,8 +258,9 @@ export function reset_props_id() {
 
 /**
  * Create (or hydrate) an unique UID for the component instance.
+ * @param {boolean} needs_next
  */
-export function props_id() {
+export function props_id(needs_next) {
 	if (
 		hydrating &&
 		hydrate_node &&
@@ -267,7 +268,10 @@ export function props_id() {
 		hydrate_node.textContent?.startsWith('#s')
 	) {
 		const id = hydrate_node.textContent.substring(1);
+		// in some cases we reuse the empty comment to store the id...in those cases we don't need to call next
+		if (needs_next) {
 			hydrate_next();
+		}
 		return id;
 	}
 

packages/svelte/src/internal/server/index.js

@@ -536,11 +536,15 @@ export function once(get_value) {
 /**
  * Create an unique ID
  * @param {Payload} payload
+ * @param {boolean} [needs_hydration]
  * @returns {string}
  */
-export function props_id(payload) {
+export function props_id(payload, needs_hydration) {
 	const uid = payload.uid();
+	// we only add an hydration marker if the first comment of the component is not an empty comment
+	if (needs_hydration) {
 		payload.out += '<!--#' + uid + '-->';
+	}
 	return uid;
 }
 

packages/svelte/tests/hydration/samples/props-id/Child1.svelte

@@ -0,0 +1,6 @@
+<script>
+	let id = $props.id();
+</script>
+
+Text first
+<p>{id}</p>

packages/svelte/tests/hydration/samples/props-id/Child2.svelte

@@ -0,0 +1,7 @@
+<script>
+	let id = $props.id();
+</script>
+
+
+{#await Promise.resolve() then x}{/await}
+<p>{id}</p>

packages/svelte/tests/hydration/samples/props-id/Child3.svelte

@@ -0,0 +1,8 @@
+<script>
+	let id = $props.id();
+</script>
+
+
+{#key id}
+	<p>{id}</p>
+{/key}

packages/svelte/tests/hydration/samples/props-id/_config.js

@@ -0,0 +1,17 @@
+import { test } from '../../test';
+
+export default test({
+	test(assert, target) {
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<main>
+				Text first
+				<p>s1</p>
+				<p>s2</p>
+				<p>s3</p>
+			</main>
+			`
+		);
+	}
+});

packages/svelte/tests/hydration/samples/props-id/_expected.html

@@ -0,0 +1 @@
+<!--[--><main><!--#s1-->Text first <p>s1</p><!----> <!--#s2--><!----> <p>s2</p><!----> <!--#s3--><p>s3</p><!----><!----></main><!--]-->

packages/svelte/tests/hydration/samples/props-id/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	import Child1 from "./Child1.svelte";
+	import Child2 from "./Child2.svelte";
+	import Child3 from "./Child3.svelte";
+</script>
+
+<main>
+	<Child1 />
+	<Child2 />
+	<Child3 />
+</main>