feat: allow comments in tags (#17671)

Alternative to #17188. I prefer this syntax, it's lighter and feels much
more natural to me. For me it's less about commenting things _out_ than
about just, well... commenting — I frequently want to do this sort of
thing:

```svelte
<button
  // when the user clicks the button, the thing should happen
  onclick={doTheThing}
>click me</button>
```

One difference between this and #17188 is that this doesn't add a node
to the AST, just like comments in CSS/JS. Haven't decided if that's
desirable or not. I think it's more correct (it's an AST, not a CST;
HTML comments are different insofar as they _can_ represent 'real'
nodes) but it might be less convenient when (for example)
pretty-printing.

### Before submitting the PR, please make sure you do the following

- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

Author: Rich-Harris

.changeset/angry-suns-call.md

@@ -0,0 +1,5 @@
+---
+'svelte': minor
+---
+
+feat: allow comments in tags

packages/svelte/src/compiler/legacy.js

@@ -101,7 +101,12 @@ export function convert(source, ast) {
 					},
 					instance,
 					module,
-					css: ast.css ? visit(ast.css) : undefined
+					css: ast.css ? visit(ast.css) : undefined,
+					// put it on _comments not comments because the latter is checked by prettier and then fails
+					// if we don't adjust stuff accordingly in our prettier plugin, and so it would be kind of an
+					// indirect breaking change for people updating their Svelte version but not their prettier plugin version.
+					// We can keep it as comments for the modern AST because the modern AST is not used in the plugin yet.
+					_comments: ast.comments?.length > 0 ? ast.comments : undefined
 				};
 			},
 			AnimateDirective(node) {

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

@@ -506,6 +506,15 @@ function read_static_attribute(parser) {
  * @returns {AST.Attribute | AST.SpreadAttribute | AST.Directive | AST.AttachTag | null}
  */
 function read_attribute(parser) {
+	/** @type {AST.JSComment | null} */
+	// eslint-disable-next-line no-useless-assignment -- it is, in fact, eslint that is useless
+	let comment = null;
+
+	while ((comment = read_comment(parser))) {
+		parser.root.comments.push(comment);
+		parser.allow_whitespace();
+	}
+
 	const start = parser.index;
 
 	if (parser.eat('{')) {
@@ -702,6 +711,50 @@ function read_attribute(parser) {
 	return create_attribute(tag.name, tag.loc, start, end, value);
 }
 
+/**
+ * @param {Parser} parser
+ * @returns {AST.JSComment | null}
+ */
+function read_comment(parser) {
+	const start = parser.index;
+
+	if (parser.eat('//')) {
+		const value = parser.read_until(/\n/);
+		const end = parser.index;
+
+		return {
+			type: 'Line',
+			start,
+			end,
+			value,
+			loc: {
+				start: locator(start),
+				end: locator(end)
+			}
+		};
+	}
+
+	if (parser.eat('/*')) {
+		const value = parser.read_until(/\*\//);
+
+		parser.eat('*/');
+		const end = parser.index;
+
+		return {
+			type: 'Block',
+			start,
+			end,
+			value,
+			loc: {
+				start: locator(start),
+				end: locator(end)
+			}
+		};
+	}
+
+	return null;
+}
+
 /**
  * @param {string} name
  * @returns {any}

packages/svelte/src/compiler/print/index.js

@@ -18,15 +18,17 @@ const LINE_BREAK_THRESHOLD = 50;
  * @param {import('./types.js').Options | undefined} options
  */
 export function print(ast, options = undefined) {
+	const comments = (ast.type === 'Root' && ast.comments) || [];
+
 	return esrap.print(
 		ast,
 		/** @type {Visitors<AST.SvelteNode>} */ ({
 			...ts({
-				comments: ast.type === 'Root' ? ast.comments : [],
+				comments,
 				getLeadingComments: options?.getLeadingComments,
 				getTrailingComments: options?.getTrailingComments
 			}),
-			...svelte_visitors,
+			...svelte_visitors(comments),
 			...css_visitors
 		})
 	);
@@ -57,35 +59,72 @@ function block(context, node, allow_inline = false) {
 }
 
 /**
+ * @param {AST.BaseNode} node
  * @param {AST.BaseElement['attributes']} attributes
  * @param {Context} context
+ * @param {AST.JSComment[]} comments
  * @returns {boolean} true if attributes were formatted on multiple lines
  */
-function attributes(attributes, context) {
+function attributes(node, attributes, context, comments) {
 	if (attributes.length === 0) {
 		return false;
 	}
 
-	// Measure total width of all attributes when rendered inline
-	const child_context = context.new();
+	let length = -1;
 
-	for (const attribute of attributes) {
-		child_context.write(' ');
-		child_context.visit(attribute);
+	let comment_index = comments.findIndex((comment) => comment.start > node.start);
+
+	if (comment_index === -1) {
+		comment_index = comments.length;
 	}
 
-	const multiline = child_context.measure() > LINE_BREAK_THRESHOLD;
+	const separator = context.new();
+
+	const children = attributes.map((attribute) => {
+		const child_context = context.new();
+
+		while (comment_index < comments.length) {
+			const comment = comments[comment_index];
+
+			if (comment.start < attribute.start) {
+				if (comment.type === 'Line') {
+					child_context.write('//' + comment.value);
+					child_context.newline();
+				} else {
+					child_context.write('/*' + comment.value + '*/'); // TODO match indentation?
+					child_context.append(separator);
+				}
+
+				comment_index += 1;
+			} else {
+				break;
+			}
+		}
+
+		child_context.visit(attribute);
+
+		length += child_context.measure() + 1;
+
+		return child_context;
+	});
+
+	let multiline = context.multiline || length > LINE_BREAK_THRESHOLD;
 
 	if (multiline) {
+		separator.newline();
 		context.indent();
-		for (const attribute of attributes) {
+		for (const child of children) {
 			context.newline();
-			context.visit(attribute);
+			context.append(child);
 		}
 		context.dedent();
 		context.newline();
 	} else {
-		context.append(child_context);
+		separator.write(' ');
+		for (const child of children) {
+			context.write(' ');
+			context.append(child);
+		}
 	}
 
 	return multiline;
@@ -94,8 +133,9 @@ function attributes(attributes, context) {
 /**
  * @param {AST.BaseElement} node
  * @param {Context} context
+ * @param {AST.JSComment[]} comments
  */
-function base_element(node, context) {
+function base_element(node, context, comments) {
 	const child_context = context.new();
 
 	child_context.write('<' + node.name);
@@ -111,7 +151,7 @@ function base_element(node, context) {
 		child_context.write('}');
 	}
 
-	const multiline_attributes = attributes(node.attributes, child_context);
+	const multiline_attributes = attributes(node, node.attributes, child_context, comments);
 	const is_doctype_node = node.name.toLowerCase() === '!doctype';
 	const is_self_closing =
 		is_void(node.name) || (node.type === 'Component' && node.fragment.nodes.length === 0);
@@ -284,8 +324,11 @@ const css_visitors = {
 	}
 };
 
-/** @type {Visitors<AST.SvelteNode>} */
-const svelte_visitors = {
+/**
+ * @param {AST.JSComment[]} comments
+ * @returns {Visitors<AST.SvelteNode>}
+ */
+const svelte_visitors = (comments) => ({
 	Root(node, context) {
 		if (node.options) {
 			context.write('<svelte:options');
@@ -315,7 +358,7 @@ const svelte_visitors = {
 
 	Script(node, context) {
 		context.write('<script');
-		attributes(node.attributes, context);
+		attributes(node, node.attributes, context, comments);
 		context.write('>');
 		block(context, node.content);
 		context.write('</script>');
@@ -545,7 +588,7 @@ const svelte_visitors = {
 	},
 
 	Component(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	ConstTag(node, context) {
@@ -681,7 +724,7 @@ const svelte_visitors = {
 	},
 
 	RegularElement(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	RenderTag(node, context) {
@@ -691,7 +734,7 @@ const svelte_visitors = {
 	},
 
 	SlotElement(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	SnippetBlock(node, context) {
@@ -747,7 +790,7 @@ const svelte_visitors = {
 
 	StyleSheet(node, context) {
 		context.write('<style');
-		attributes(node.attributes, context);
+		attributes(node, node.attributes, context, comments);
 		context.write('>');
 
 		if (node.children.length > 0) {
@@ -774,7 +817,7 @@ const svelte_visitors = {
 	},
 
 	SvelteBoundary(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	SvelteComponent(node, context) {
@@ -783,7 +826,7 @@ const svelte_visitors = {
 		context.write(' this={');
 		context.visit(node.expression);
 		context.write('}');
-		attributes(node.attributes, context);
+		attributes(node, node.attributes, context, comments);
 		if (node.fragment && node.fragment.nodes.length > 0) {
 			context.write('>');
 			block(context, node.fragment, true);
@@ -794,7 +837,7 @@ const svelte_visitors = {
 	},
 
 	SvelteDocument(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	SvelteElement(node, context) {
@@ -803,7 +846,7 @@ const svelte_visitors = {
 		context.write('this={');
 		context.visit(node.tag);
 		context.write('}');
-		attributes(node.attributes, context);
+		attributes(node, node.attributes, context, comments);
 
 		if (node.fragment && node.fragment.nodes.length > 0) {
 			context.write('>');
@@ -815,27 +858,27 @@ const svelte_visitors = {
 	},
 
 	SvelteFragment(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	SvelteHead(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	SvelteSelf(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	SvelteWindow(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	Text(node, context) {
 		context.write(node.data);
 	},
 
 	TitleElement(node, context) {
-		base_element(node, context);
+		base_element(node, context, comments);
 	},
 
 	TransitionDirective(node, context) {
@@ -865,4 +908,4 @@ const svelte_visitors = {
 			context.write('}');
 		}
 	}
-};
+});