Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] rendering void tag for <svelte:element> #7453

Merged
merged 11 commits into from Apr 30, 2022
2 changes: 2 additions & 0 deletions site/content/docs/02-template-syntax.md
Expand Up @@ -1641,6 +1641,8 @@ The only supported binding is `bind:this`, since the element type specific bindi

If `this` has a nullish value, the element and its children will not be rendered.

If `this` is the name of a void tag (e.g., `br`) and `<svelte:element>` has child elements, a runtime error will be thrown in development mode.

```sv
<script>
let tag = 'div';
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/compile/nodes/Element.ts
@@ -1,4 +1,4 @@
import { is_void } from '../../utils/names';
import { is_void } from '../../../shared/utils/names';
import Node from './shared/Node';
import Attribute from './Attribute';
import Binding from './Binding';
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/compile/render_dom/wrappers/Element/index.ts
Expand Up @@ -2,7 +2,7 @@ import Renderer from '../../Renderer';
import Element from '../../../nodes/Element';
import Wrapper from '../shared/Wrapper';
import Block from '../../Block';
import { is_void } from '../../../../utils/names';
import { is_void } from '../../../../../shared/utils/names';
import FragmentWrapper from '../Fragment';
import { escape_html, string_literal } from '../../../utils/stringify';
import TextWrapper from '../Text';
Expand Down Expand Up @@ -279,6 +279,7 @@ export default class ElementWrapper extends Wrapper {

block.chunks.init.push(b`
${this.renderer.options.dev && b`@validate_dynamic_element(${tag});`}
${this.renderer.options.dev && this.node.children.length > 0 && b`@validate_void_dynamic_element(${tag});`}
let ${this.var} = ${tag} && ${this.child_dynamic_element_block.name}(#ctx);
`);

Expand Down Expand Up @@ -310,6 +311,7 @@ export default class ElementWrapper extends Wrapper {
} else if (${not_equal}(${previous_tag}, ${tag})) {
${this.var}.d(1);
${this.renderer.options.dev && b`@validate_dynamic_element(${tag});`}
${this.renderer.options.dev && this.node.children.length > 0 && b`@validate_void_dynamic_element(${tag});`}
${this.var} = ${this.child_dynamic_element_block.name}(#ctx);
${this.var}.c();
${this.var}.m(${this.get_update_mount_node(anchor)}, ${anchor});
Expand Down
36 changes: 29 additions & 7 deletions src/compiler/compile/render_ssr/handlers/Element.ts
@@ -1,6 +1,6 @@
import { is_void } from '../../../utils/names';
import { is_void } from '../../../../shared/utils/names';
import { get_attribute_expression, get_attribute_value, get_class_attribute_value } from './shared/get_attribute_value';
import { boolean_attributes } from './shared/boolean_attributes';
import { boolean_attributes } from '../../../../shared/boolean_attributes';
import Renderer, { RenderOptions } from '../Renderer';
import Element from '../../nodes/Element';
import { p, x } from 'code-red';
Expand All @@ -9,7 +9,7 @@ import remove_whitespace_children from './utils/remove_whitespace_children';
import fix_attribute_casing from '../../render_dom/wrappers/Element/fix_attribute_casing';
import { namespaces } from '../../../utils/namespaces';
import { start_newline } from '../../../utils/patterns';
import { Expression as ESExpression } from 'estree';
import { Node, Expression as ESExpression } from 'estree';

export default function (node: Element, renderer: Renderer, options: RenderOptions) {

Expand All @@ -24,6 +24,10 @@ export default function (node: Element, renderer: Renderer, options: RenderOptio
node.attributes.some((attribute) => attribute.name === 'contenteditable')
);

if (node.is_dynamic_element) {
renderer.push();
}

renderer.add_string('<');
add_tag_name();

Expand Down Expand Up @@ -192,16 +196,34 @@ export default function (node: Element, renderer: Renderer, options: RenderOptio
renderer.add_string('\n');
}
}
if (node.is_dynamic_element) renderer.push();
renderer.render(children, options);
if (node.is_dynamic_element) {
const children = renderer.pop();
renderer.add_expression(x`@is_void(#tag) ? '' : ${children}`);
}
add_close_tag();
}

if (node.is_dynamic_element) {
let content: Node = renderer.pop();
if (options.dev && node.children.length > 0) content = x`(() => { @validate_void_dynamic_element(#tag); return ${content}; })()`;
renderer.add_expression(x`((#tag) => {
${options.dev && x`@validate_dynamic_element(#tag)`}
return #tag ? ${content} : '';
})(${node.tag_expr.node})`);
}

function add_close_tag() {
if (!is_void(node.name)) {
renderer.add_string('</');
add_tag_name();
renderer.add_string('>');
if (node.tag_expr.node.type === 'Literal') {
if (!is_void(node.tag_expr.node.value as string)) {
renderer.add_string('</');
add_tag_name();
renderer.add_string('>');
}
return;
}
renderer.add_expression(x`@is_void(#tag) ? '' : \`</\${#tag}>\``);
}

function add_tag_name() {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/parse/state/tag.ts
@@ -1,7 +1,7 @@
import { Directive, DirectiveType, TemplateNode, Text } from '../../interfaces';
import { extract_svelte_ignore } from '../../utils/extract_svelte_ignore';
import fuzzymatch from '../../utils/fuzzymatch';
import { is_void } from '../../utils/names';
import { is_void } from '../../../shared/utils/names';
import parser_errors from '../errors';
import { Parser } from '../index';
import read_expression from '../read/expression';
Expand Down
6 changes: 0 additions & 6 deletions src/compiler/utils/names.ts
Expand Up @@ -117,12 +117,6 @@ export const reserved = new Set([
'yield'
]);

const void_element_names = /^(?:area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/;

export function is_void(name: string) {
return void_element_names.test(name) || name.toLowerCase() === '!doctype';
}

tanhauhau marked this conversation as resolved.
Show resolved Hide resolved
export function is_valid(str: string): boolean {
let i = 0;

Expand Down
10 changes: 9 additions & 1 deletion src/runtime/internal/dev.ts
@@ -1,5 +1,6 @@
import { custom_event, append, append_hydration, insert, insert_hydration, detach, listen, attr } from './dom';
import { SvelteComponent } from './Component';
import { is_void } from '../../shared/utils/names';

export function dispatch_dev<T=any>(type: string, detail?: T) {
document.dispatchEvent(custom_event(type, { version: '__VERSION__', ...detail }, { bubbles: true }));
Expand Down Expand Up @@ -108,11 +109,18 @@ export function validate_slots(name, slot, keys) {
}

export function validate_dynamic_element(tag: unknown) {
if (tag && typeof tag !== 'string') {
const is_string = typeof tag === 'string';
if (tag && !is_string) {
throw new Error('<svelte:element> expects "this" attribute to be a string.');
}
}

export function validate_void_dynamic_element(tag: undefined | string) {
if (tag && is_void(tag)) {
throw new Error(`<svelte:element this="${tag}"> is self-closing and cannot have content.`);
}
}

type Props = Record<string, any>;
export interface SvelteComponentDev {
$set(props?: Props): void;
Expand Down
7 changes: 4 additions & 3 deletions src/runtime/internal/ssr.ts
@@ -1,6 +1,7 @@
import { set_current_component, current_component } from './lifecycle';
import { run_all, blank_object } from './utils';
import { boolean_attributes } from '../../compiler/compile/render_ssr/handlers/shared/boolean_attributes';
import { boolean_attributes } from '../../shared/boolean_attributes';
export { is_void } from '../../shared/utils/names';

export const invalid_attribute_name_character = /[\s'">/=\u{FDD0}-\u{FDEF}\u{FFFE}\u{FFFF}\u{1FFFE}\u{1FFFF}\u{2FFFE}\u{2FFFF}\u{3FFFE}\u{3FFFF}\u{4FFFE}\u{4FFFF}\u{5FFFE}\u{5FFFF}\u{6FFFE}\u{6FFFF}\u{7FFFE}\u{7FFFF}\u{8FFFE}\u{8FFFF}\u{9FFFE}\u{9FFFF}\u{AFFFE}\u{AFFFF}\u{BFFFE}\u{BFFFF}\u{CFFFE}\u{CFFFF}\u{DFFFE}\u{DFFFF}\u{EFFFE}\u{EFFFF}\u{FFFFE}\u{FFFFF}\u{10FFFE}\u{10FFFF}]/u;
// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
Expand All @@ -19,7 +20,7 @@ export function spread(args, attrs_to_add) {
attributes.class += ' ' + classes_to_add;
}
}

if (styles_to_add) {
if (attributes.style == null) {
attributes.style = style_object_to_string(styles_to_add);
Expand Down Expand Up @@ -55,7 +56,7 @@ export function merge_ssr_styles(style_attribute, style_directive) {
if (!name) continue;
style_object[name] = value;
}

for (const name in style_directive) {
const value = style_directive[name];
if (value) {
Expand Down
5 changes: 5 additions & 0 deletions src/shared/utils/names.ts
@@ -0,0 +1,5 @@
const void_element_names = /^(?:area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/;

export function is_void(name: string) {
return void_element_names.test(name) || name.toLowerCase() === '!doctype';
}
12 changes: 12 additions & 0 deletions test/runtime/samples/dynamic-element-void-tag/_config.js
@@ -0,0 +1,12 @@
export default {
props: {
propTag: 'hr'
},
html: '<h1></h1><col><img><hr><input><br>',

test({ assert, component, target }) {
assert.htmlEqual(target.innerHTML, '<h1></h1><col><img><hr><input><br>');
component.propTag = 'link';
assert.htmlEqual(target.innerHTML, '<h1></h1><col><img><link><input><br>');
}
};
12 changes: 12 additions & 0 deletions test/runtime/samples/dynamic-element-void-tag/main.svelte
@@ -0,0 +1,12 @@
<script>
export let propTag;
const static_tag = 'input';
const func_tag = () => 'br';
</script>

<h1></h1>
<svelte:element this="{'col'}"></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>
@@ -0,0 +1,9 @@
export default {
compileOptions: {
dev: true
},
props: {
tag: 'br'
},
error: '<svelte:element this="br"> is self-closing and cannot have content.'
};
@@ -0,0 +1,5 @@
<script>
export let tag;
</script>

<svelte:element this='{tag}'>foo</svelte:element>
@@ -0,0 +1,9 @@
export default {
compileOptions: {
dev: true
},
props: {
tag: 'br'
},
error: '<svelte:element this="br"> is self-closing and cannot have content.'
};
@@ -0,0 +1,5 @@
<script>
export let tag;
</script>

<svelte:element this='{tag}'><div>bar</div></svelte:element>
@@ -0,0 +1 @@
<div>This is nested</div>
@@ -0,0 +1,9 @@
export default {
compileOptions: {
dev: true
},
props: {
tag: 'br'
},
error: '<svelte:element this="br"> is self-closing and cannot have content.'
};
@@ -0,0 +1,6 @@
<script>
import Nested from './Nested.svelte';
export let tag;
</script>

<svelte:element this='{tag}'><Nested/></svelte:element>
@@ -0,0 +1 @@
<div>This is nested</div>
@@ -0,0 +1,6 @@
export default {
props: {
tag: 'br'
},
html: '<br>'
};
@@ -0,0 +1,6 @@
<script>
import Nested from './Nested.svelte';
export let tag;
</script>

<svelte:element this='{tag}'><Nested/></svelte:element>