breaking: disallow string literal values in <svelte:element this="..."> (#11454)

* breaking: disallow string literal values in `<svelte:element this="...">`

* note breaking change

* Update sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md

Co-authored-by: Jeremiasz Major <jrh.mjr@gmail.com>

* prettier

* make invalid `<svelte:element this>` a warning instead of an error (#11641)

* make it a warning instead of an error

* format

---------

Co-authored-by: Jeremiasz Major <jrh.mjr@gmail.com>

Author: Rich-Harris

.changeset/dull-worms-relax.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+breaking: disallow string literal values in `<svelte:element this="...">`

packages/svelte/messages/compile-errors/template.md

@@ -260,13 +260,9 @@
 
 > `<svelte:component>` must have a 'this' attribute
 
-## svelte_element_invalid_this
-
-> Invalid element definition — must be an `{expression}`
-
 ## svelte_element_missing_this
 
-> `<svelte:element>` must have a 'this' attribute
+> `<svelte:element>` must have a 'this' attribute with a value
 
 ## svelte_fragment_invalid_attribute
 

packages/svelte/messages/compile-warnings/template.md

@@ -37,3 +37,7 @@
 ## slot_element_deprecated
 
 > Using `<slot>` to render parent content is deprecated. Use `{@render ...}` tags instead
+
+## svelte_element_invalid_this
+
+> `this` should be an `{expression}`. Using a string attribute value will cause an error in future versions of Svelte

packages/svelte/src/compiler/errors.js

@@ -1138,21 +1138,12 @@ export function svelte_component_missing_this(node) {
 }
 
 /**
- * Invalid element definition — must be an `{expression}`
- * @param {null | number | NodeLike} node
- * @returns {never}
- */
-export function svelte_element_invalid_this(node) {
-	e(node, "svelte_element_invalid_this", "Invalid element definition — must be an `{expression}`");
-}
-
-/**
- * `<svelte:element>` must have a 'this' attribute
+ * `<svelte:element>` must have a 'this' attribute with a value
  * @param {null | number | NodeLike} node
  * @returns {never}
  */
 export function svelte_element_missing_this(node) {
-	e(node, "svelte_element_missing_this", "`<svelte:element>` must have a 'this' attribute");
+	e(node, "svelte_element_missing_this", "`<svelte:element>` must have a 'this' attribute with a value");
 }
 
 /**

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

@@ -4,6 +4,7 @@ import { read_script } from '../read/script.js';
 import read_style from '../read/style.js';
 import { closing_tag_omitted, decode_character_references } from '../utils/html.js';
 import * as e from '../../../errors.js';
+import * as w from '../../../warnings.js';
 import { create_fragment } from '../utils/create.js';
 import { create_attribute } from '../../nodes.js';
 
@@ -262,14 +263,28 @@ export default function tag(parser) {
 		const definition = /** @type {import('#compiler').Attribute} */ (
 			element.attributes.splice(index, 1)[0]
 		);
-		if (definition.value === true || definition.value.length !== 1) {
-			e.svelte_element_invalid_this(definition.start);
+
+		if (definition.value === true) {
+			e.svelte_element_missing_this(definition);
 		}
+
 		const chunk = definition.value[0];
-		element.tag =
-			chunk.type === 'Text'
-				? { type: 'Literal', value: chunk.data, raw: `'${chunk.raw}'` }
-				: chunk.expression;
+
+		if (definition.value.length !== 1 || chunk.type !== 'ExpressionTag') {
+			chunk.type;
+			w.svelte_element_invalid_this(definition);
+
+			// note that this is wrong, in the case of e.g. `this="h{n}"` — it will result in `<h>`.
+			// it would be much better to just error here, but we are preserving the existing buggy
+			// Svelte 4 behaviour out of an overabundance of caution regarding breaking changes.
+			// TODO in 6.0, error
+			element.tag =
+				chunk.type === 'Text'
+					? { type: 'Literal', value: chunk.data, raw: `'${chunk.raw}'` }
+					: chunk.expression;
+		} else {
+			element.tag = chunk.expression;
+		}
 	}
 
 	if (is_top_level_script_or_style) {

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

@@ -1449,24 +1449,6 @@ const common_visitors = {
 	SvelteElement(node, context) {
 		context.state.analysis.elements.push(node);
 
-		// TODO why are we handling the `<svelte:element this="x" />` case? there is no
-		// reason for someone to use a static value with `<svelte:element>`
-		if (
-			context.state.options.namespace !== 'foreign' &&
-			node.tag.type === 'Literal' &&
-			typeof node.tag.value === 'string'
-		) {
-			if (SVGElements.includes(node.tag.value)) {
-				node.metadata.svg = true;
-				return;
-			}
-
-			if (MathMLElements.includes(node.tag.value)) {
-				node.metadata.mathml = true;
-				return;
-			}
-		}
-
 		for (const attribute of node.attributes) {
 			if (attribute.type === 'Attribute') {
 				if (attribute.name === 'xmlns' && is_text_attribute(attribute)) {

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

@@ -586,22 +586,6 @@ const validation = {
 		) {
 			e.render_tag_invalid_call_expression(node);
 		}
-
-		const is_inside_textarea = context.path.find((n) => {
-			return (
-				n.type === 'SvelteElement' &&
-				n.name === 'svelte:element' &&
-				n.tag.type === 'Literal' &&
-				n.tag.value === 'textarea'
-			);
-		});
-		if (is_inside_textarea) {
-			e.tag_invalid_placement(
-				node,
-				'inside <textarea> or <svelte:element this="textarea">',
-				'render'
-			);
-		}
 	},
 	IfBlock(node, context) {
 		validate_block_not_empty(node.consequent, context);

packages/svelte/src/compiler/warnings.js

@@ -90,7 +90,8 @@ export const codes = [
 	"component_name_lowercase",
 	"element_invalid_self_closing_tag",
 	"event_directive_deprecated",
-	"slot_element_deprecated"
+	"slot_element_deprecated",
+	"svelte_element_invalid_this"
 ];
 
 /**
@@ -712,4 +713,12 @@ export function event_directive_deprecated(node, name) {
  */
 export function slot_element_deprecated(node) {
 	w(node, "slot_element_deprecated", "Using `<slot>` to render parent content is deprecated. Use `{@render ...}` tags instead");
+}
+
+/**
+ * `this` should be an `{expression}`. Using a string attribute value will cause an error in future versions of Svelte
+ * @param {null | NodeLike} node
+ */
+export function svelte_element_invalid_this(node) {
+	w(node, "svelte_element_invalid_this", "`this` should be an `{expression}`. Using a string attribute value will cause an error in future versions of Svelte");
 }

packages/svelte/tests/css/samples/dynamic-element/_config.js

@@ -6,12 +6,12 @@ export default test({
 			code: 'css_unused_selector',
 			message: 'Unused CSS selector ".unused"',
 			start: {
-				character: 79,
+				character: 81,
 				column: 1,
 				line: 7
 			},
 			end: {
-				character: 86,
+				character: 88,
 				column: 8,
 				line: 7
 			}

packages/svelte/tests/css/samples/dynamic-element/input.svelte

@@ -1,4 +1,4 @@
-<svelte:element this="div" class="used" />
+<svelte:element this={"div"} class="used" />
 
 <style>
 	.used {