Skip to content

Commit 54e1c23

Browse files
author
Alfred Ringstad
committed
Reuse DOM elements if possible. Add dev warnings when passing nullish values as tag. Throw an error when trying to bind other than this. Add more tests.
1 parent 4fcc681 commit 54e1c23

File tree

20 files changed

+163
-89
lines changed

20 files changed

+163
-89
lines changed

‎src/compiler/compile/nodes/DynamicElement.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,19 @@ export default class DynamicElement extends Node {
9797
this.scope = scope;
9898

9999
this.children = map_children(component, this, this.scope, info.children);
100+
101+
this.validate();
102+
}
103+
104+
validate() {
105+
this.bindings.forEach(binding => {
106+
if (binding.name !== 'this') {
107+
this.component.error(binding, {
108+
code: 'invalid-binding',
109+
message: `'${binding.name}' is not a valid binding. svelte:element only supports bind:this`
110+
});
111+
}
112+
});
100113
}
101114

102115
add_css_class() {

‎src/compiler/compile/render_dom/wrappers/DynamicElement.ts‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import Wrapper from './shared/Wrapper';
22
import Renderer from '../Renderer';
33
import Block from '../Block';
4-
import FragmentWrapper from './Fragment';
54
import { b, x } from 'code-red';
65
import { Identifier } from 'estree';
76
import DynamicElement from '../../nodes/DynamicElement';
@@ -10,7 +9,6 @@ import create_debugging_comment from './shared/create_debugging_comment';
109
import Element from '../../nodes/Element';
1110

1211
export default class DynamicElementWrapper extends Wrapper {
13-
fragment: FragmentWrapper;
1412
node: DynamicElement;
1513
elementWrapper: ElementWrapper;
1614
block: Block;
@@ -112,43 +110,45 @@ export default class DynamicElementWrapper extends Wrapper {
112110
);
113111

114112
const anchor = this.get_or_create_anchor(block, parent_node, parent_nodes);
115-
const body = b`
116-
${
117-
has_transitions
118-
? b`
119-
@group_outros();
120-
@transition_out(${this.var}, 1, 1, @noop);
121-
@check_outros();
122-
`
123-
: b`${this.var}.d(1);`
124-
}
125-
${this.var} = ${this.block.name}(#ctx);
126-
${this.var}.c();
127-
${has_transitions && b`@transition_in(${this.var})`}
128-
${this.var}.m(${this.get_update_mount_node(anchor)}, ${anchor});
129-
`;
130113

131-
if (dynamic) {
132-
block.chunks.update.push(b`
133-
if (${condition}) {
134-
${body}
114+
if (has_transitions) {
115+
block.chunks.intro.push(b`@transition_in(${this.var})`);
116+
block.chunks.outro.push(b`@transition_out(${this.var})`);
117+
118+
const body = b`
119+
@group_outros();
120+
@transition_out(${this.var}, 1, 1, @noop);
121+
@check_outros();
122+
${this.var} = ${this.block.name}(#ctx);
123+
${this.var}.c();
124+
@transition_in(${this.var});
125+
${this.var}.m(${this.get_update_mount_node(anchor)}, ${anchor});
126+
`;
127+
128+
if (dynamic) {
129+
block.chunks.update.push(b`
130+
if (${condition}) {
131+
${body}
132+
} else {
133+
${this.var}.p(#ctx, #dirty);
134+
}
135+
`);
135136
} else {
136-
${this.var}.p(#ctx, #dirty);
137+
block.chunks.update.push(b`
138+
if (${condition}) {
139+
${body}
140+
}
141+
`);
137142
}
138-
`);
139-
} else {
143+
} else if (dynamic) {
140144
block.chunks.update.push(b`
145+
${this.var}.p(#ctx, #dirty);
141146
if (${condition}) {
142-
${body}
147+
${this.var}.m(${this.get_update_mount_node(anchor)}, ${anchor});
143148
}
144149
`);
145150
}
146151

147-
if (has_transitions) {
148-
block.chunks.intro.push(b`@transition_in(${this.var})`);
149-
block.chunks.outro.push(b`@transition_out(${this.var})`);
150-
}
151-
152152
block.chunks.destroy.push(b`${this.var}.d(detaching)`);
153153
}
154154
}

‎src/compiler/compile/render_dom/wrappers/Element/index.ts‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,10 @@ export default class ElementWrapper extends Wrapper {
211211
}
212212
});
213213

214+
if (node.dynamic_tag) {
215+
block.add_dependencies(node.dynamic_tag.dependencies);
216+
}
217+
214218
if (this.parent) {
215219
if (node.actions.length > 0 ||
216220
node.animation ||
@@ -244,6 +248,14 @@ export default class ElementWrapper extends Wrapper {
244248
b`${node} = ${render_statement};`
245249
);
246250

251+
if (this.node.dynamic_tag && this.renderer.options.dev) {
252+
block.chunks.create.push(b`@validate_dynamic_element(${this.node.dynamic_tag.manipulate(block)});`);
253+
254+
if (renderer.options.hydratable) {
255+
block.chunks.claim.push(b`@validate_dynamic_element(${this.node.dynamic_tag.manipulate(block)});`);
256+
}
257+
}
258+
247259
if (renderer.options.hydratable) {
248260
if (parent_nodes) {
249261
block.chunks.claim.push(b`
@@ -278,13 +290,14 @@ export default class ElementWrapper extends Wrapper {
278290
block.chunks.destroy.push(b`if (detaching) @detach(${node});`);
279291
}
280292

293+
let staticChildren = null;
294+
281295
// insert static children with textContent or innerHTML
282296
const can_use_textcontent = this.can_use_textcontent();
283297
if (!this.node.namespace && (this.can_use_innerhtml || can_use_textcontent) && this.fragment.nodes.length > 0) {
284298
if (this.fragment.nodes.length === 1 && this.fragment.nodes[0].node.type === 'Text') {
285-
block.chunks.create.push(
286-
b`${node}.textContent = ${string_literal((this.fragment.nodes[0] as TextWrapper).data)};`
287-
);
299+
staticChildren = b`${node}.textContent = ${string_literal((this.fragment.nodes[0] as TextWrapper).data)};`;
300+
block.chunks.create.push(staticChildren);
288301
} else {
289302
const state = {
290303
quasi: {
@@ -303,9 +316,8 @@ export default class ElementWrapper extends Wrapper {
303316
to_html((this.fragment.nodes as unknown as Array<ElementWrapper | TextWrapper>), block, literal, state, can_use_raw_text);
304317
literal.quasis.push(state.quasi);
305318

306-
block.chunks.create.push(
307-
b`${node}.${this.can_use_innerhtml ? 'innerHTML' : 'textContent'} = ${literal};`
308-
);
319+
staticChildren = b`${node}.${this.can_use_innerhtml ? 'innerHTML' : 'textContent'} = ${literal};`;
320+
block.chunks.create.push(staticChildren);
309321
}
310322
} else {
311323
this.fragment.nodes.forEach((child: Wrapper) => {
@@ -334,6 +346,23 @@ export default class ElementWrapper extends Wrapper {
334346
this.add_classes(block);
335347
this.add_manual_style_scoping(block);
336348

349+
if (this.node.dynamic_tag) {
350+
const dependencies = this.node.dynamic_tag.dynamic_dependencies();
351+
if (dependencies.length) {
352+
const condition = block.renderer.dirty(
353+
dependencies
354+
);
355+
356+
block.chunks.update.push(b`
357+
if (${condition}) {
358+
@detach(${node});
359+
${node} = ${render_statement};
360+
${staticChildren}
361+
}
362+
`);
363+
}
364+
}
365+
337366
if (nodes && this.renderer.options.hydratable && !this.void) {
338367
block.chunks.claim.push(
339368
b`${this.node.children.length > 0 ? nodes : children}.forEach(@detach);`

‎src/compiler/parse/state/tag.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ export default function tag(parser: Parser) {
181181

182182
if (name === 'svelte:component') {
183183
const index = element.attributes.findIndex(attr => attr.type === 'Attribute' && attr.name === 'this');
184-
if (!~index) {
184+
if (index === -1) {
185185
parser.error({
186186
code: 'missing-component-definition',
187187
message: "<svelte:component> must have a 'this' attribute"
@@ -201,7 +201,7 @@ export default function tag(parser: Parser) {
201201

202202
if (name === 'svelte:element') {
203203
const index = element.attributes.findIndex(attr => attr.type === 'Attribute' && attr.name === 'tag');
204-
if (!~index) {
204+
if (index === -1) {
205205
parser.error({
206206
code: 'missing-element-definition',
207207
message: '<svelte:element> must have a \'tag\' attribute'

‎src/runtime/internal/dev.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ export function validate_slots(name, slot, keys) {
9797
}
9898
}
9999

100+
export function validate_dynamic_element(tag) {
101+
if (tag == null) {
102+
console.warn('<svelte:element> expects a non-nullish value in attribute "tag"');
103+
}
104+
}
105+
100106
type Props = Record<string, any>;
101107
export interface SvelteComponentDev {
102108
$set(props?: Props): void;

‎test/runtime/samples/dev-warning-dynamic-element-nullish-tag/_config.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export default {
2+
compileOptions: {
3+
dev: true
4+
},
5+
6+
warnings: [
7+
'<svelte:element> expects a non-nullish value in attribute "tag"'
8+
]
9+
};

‎test/runtime/samples/dev-warning-dynamic-element-nullish-tag/main.svelte‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<script>
2+
let tag = null;
3+
</script>
4+
5+
<svelte:element {tag}></svelte:element>

‎test/runtime/samples/dynamic-element-binding-invalid/_config.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default {
2+
error: "'value' is not a valid binding. svelte:element only supports bind:this"
3+
};

‎test/runtime/samples/dynamic-element-binding-invalid/main.svelte‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<script>
2+
const tag = "div";
3+
let value;
4+
</script>
5+
6+
<svelte:element tag={tag} bind:value></svelte:element>

‎test/runtime/samples/dynamic-element-binding-this/_config.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export default {
2+
html: '<div></div>',
3+
4+
test({ assert, component, target }) {
5+
const div = target.querySelector('div');
6+
assert.equal(div, component.foo);
7+
}
8+
};

0 commit comments

Comments
 (0)