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>
pull/11659/head
Rich Harris 8 months ago committed by GitHub
parent ade6b6e480
commit d288735fa8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

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

@ -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

@ -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

@ -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");
}
/**

@ -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];
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) {

@ -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)) {

@ -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);

@ -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"
];
/**
@ -713,3 +714,11 @@ 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");
}

@ -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
}

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

@ -1,2 +1,2 @@
<svelte:element this="div"></svelte:element>
<svelte:element this="div" class="foo"></svelte:element>
<svelte:element this={"div"}></svelte:element>
<svelte:element this={"div"} class="foo"></svelte:element>

@ -2,40 +2,40 @@
"html": {
"type": "Fragment",
"start": 0,
"end": 101,
"end": 105,
"children": [
{
"type": "Element",
"name": "svelte:element",
"start": 0,
"end": 44,
"end": 46,
"tag": "div",
"attributes": [],
"children": []
},
{
"type": "Text",
"start": 44,
"end": 45,
"start": 46,
"end": 47,
"raw": "\n",
"data": "\n"
},
{
"type": "Element",
"name": "svelte:element",
"start": 45,
"end": 101,
"start": 47,
"end": 105,
"tag": "div",
"attributes": [
{
"type": "Attribute",
"start": 72,
"end": 83,
"start": 76,
"end": 87,
"name": "class",
"value": [
{
"start": 79,
"end": 82,
"start": 83,
"end": 86,
"type": "Text",
"raw": "foo",
"data": "foo"

@ -2,4 +2,4 @@
export let disabled = false;
</script>
<svelte:element {disabled} this="button">Click me</svelte:element>
<svelte:element {disabled} this={"button"}>Click me</svelte:element>

@ -6,4 +6,4 @@
};
</script>
<svelte:element this="button" {...props}>Click me</svelte:element>
<svelte:element this={"button"} {...props}>Click me</svelte:element>

@ -2,7 +2,7 @@
export let item;
</script>
<svelte:element this="div" class:active={true}>
<svelte:element this={"div"} class:active={true}>
{item.text}
</svelte:element>

@ -1 +1 @@
<svelte:element this="div">Foo</svelte:element>
<svelte:element this={"div"}>Foo</svelte:element>

@ -9,13 +9,13 @@
</svg>
<svg>
<svelte:element this="path">
<svelte:element this={"path"}>
<foreignObject>
<svelte:element this="span">ok</svelte:element>
<svelte:element this={"span"}>ok</svelte:element>
</foreignObject>
<foreignObject>
{#if true}
<svelte:element this="span">ok</svelte:element>
<svelte:element this={"span"}>ok</svelte:element>
{/if}
</foreignObject>
</svelte:element>

@ -1,3 +1,3 @@
<svelte:element this="svg" xmlns="http://www.w3.org/2000/svg">
<svelte:element this="path" xmlns="http://www.w3.org/2000/svg"></svelte:element>
<svelte:element this={"svg"} xmlns="http://www.w3.org/2000/svg">
<svelte:element this={"path"} xmlns="http://www.w3.org/2000/svg"></svelte:element>
</svelte:element>

@ -1 +1 @@
<svelte:element this="textarea"></svelte:element>
<svelte:element this={"textarea"}></svelte:element>

@ -6,7 +6,7 @@
<h1></h1>
<svelte:element this="{'foo'}"></svelte:element>
<svelte:element this="img"></svelte:element>
<svelte:element this={"img"}></svelte:element>
<svelte:element this="{propTag}"></svelte:element>
<svelte:element this="{static_tag}"></svelte:element>
<svelte:element this="{func_tag()}"></svelte:element>

@ -1,3 +1,3 @@
<div>
<svelte:element this="p" />
<svelte:element this={"p"} />
</div>

@ -1 +1 @@
<svelte:element this="div">Foo</svelte:element>
<svelte:element this={"div"}>Foo</svelte:element>

@ -1,3 +1,3 @@
<svelte:element this="title">lorem</svelte:element>
<svelte:element this="style">{'.ipsum { display: block; }'}</svelte:element>
<svelte:element this="script">{'console.log(true);'}</svelte:element>
<svelte:element this={"title"}>lorem</svelte:element>
<svelte:element this={"style"}>{'.ipsum { display: block; }'}</svelte:element>
<svelte:element this={"script"}>{'console.log(true);'}</svelte:element>

@ -1,14 +1,14 @@
[
{
"code": "svelte_element_invalid_this",
"message": "Invalid element definition — must be an `{expression}`",
"code": "svelte_element_missing_this",
"message": "`<svelte:element>` must have a 'this' attribute with a value",
"start": {
"line": 2,
"column": 17
},
"end": {
"line": 2,
"column": 17
"column": 21
}
}
]

@ -1,7 +1,7 @@
[
{
"code": "svelte_element_missing_this",
"message": "`<svelte:element>` must have a 'this' attribute",
"message": "`<svelte:element>` must have a 'this' attribute with a value",
"start": {
"line": 2,
"column": 1

@ -230,3 +230,14 @@ In Svelte 4 you could have content inside a `<svelte:options />` tag. It was ign
### `<slot>` elements in declarative shadow roots are preserved
Svelte 4 replaced the `<slot />` tag in all places with its own version of slots. Svelte 5 preserves them in the case they are a child of a `<template shadowrootmode="...">` element.
### `<svelte:element>` tag must be an expression
In Svelte 4, `<svelte:element this="div">` is valid code. This makes little sense — you should just do `<div>`. In the vanishingly rare case that you _do_ need to use a literal value for some reason, you can do this:
```diff
- <svelte:element this="div">
+ <svelte:element this={"div"}>
```
Note that whereas Svelte 4 would treat `<svelte:element this="input">` (for example) identically to `<input>` for the purposes of determining which `bind:` directives could be applied, Svelte 5 does not.

Loading…
Cancel
Save