Skip to content

Commit c2bd9d7

Browse files
tanhauhautaylorzane
tanhauhau
authored and
taylorzane
committed
fix render fallback slot content due to whitespace (sveltejs#4500)
1 parent ddee94c commit c2bd9d7

File tree

12 files changed

+178
-53
lines changed

12 files changed

+178
-53
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
* In `vars` array, correctly indicate whether `module` variables are `mutated` or `reassigned` ([#3215](https://github.com/sveltejs/svelte/issues/3215))
66
* Fix spread props not updating in certain situations ([#3521](https://github.com/sveltejs/svelte/issues/3521), [#4480](https://github.com/sveltejs/svelte/issues/4480))
7+
* Use the fallback content for slots if they are passed only whitespace ([#4092](https://github.com/sveltejs/svelte/issues/4092))
78
* In `dev` mode, check for unknown props even if the component has no writable props ([#4323](https://github.com/sveltejs/svelte/issues/4323))
89
* Exclude global variables from `$capture_state` ([#4463](https://github.com/sveltejs/svelte/issues/4463))
910
* Fix bitmask overflow for slots ([#4481](https://github.com/sveltejs/svelte/issues/4481))

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

+29
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,18 @@ import Component from '../Component';
33
import TemplateScope from './shared/TemplateScope';
44
import { INode } from './interfaces';
55

6+
// Whitespace inside one of these elements will not result in
7+
// a whitespace node being created in any circumstances. (This
8+
// list is almost certainly very incomplete)
9+
const elements_without_text = new Set([
10+
'audio',
11+
'datalist',
12+
'dl',
13+
'optgroup',
14+
'select',
15+
'video',
16+
]);
17+
618
export default class Text extends Node {
719
type: 'Text';
820
data: string;
@@ -13,4 +25,21 @@ export default class Text extends Node {
1325
this.data = info.data;
1426
this.synthetic = info.synthetic || false;
1527
}
28+
29+
should_skip() {
30+
if (/\S/.test(this.data)) return false;
31+
32+
const parent_element = this.find_nearest(/(?:Element|InlineComponent|Head)/);
33+
if (!parent_element) return false;
34+
35+
if (parent_element.type === 'Head') return true;
36+
if (parent_element.type === 'InlineComponent') return parent_element.children.length === 1 && this === parent_element.children[0];
37+
38+
// svg namespace exclusions
39+
if (/svg$/.test(parent_element.namespace)) {
40+
if (this.prev && this.prev.type === "Element" && this.prev.name === "tspan") return false;
41+
}
42+
43+
return parent_element.namespace || elements_without_text.has(parent_element.name);
44+
}
1645
}

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

+1-5
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { INode } from '../../nodes/interfaces';
1717
import Renderer from '../Renderer';
1818
import Block from '../Block';
1919
import { trim_start, trim_end } from '../../../utils/trim';
20+
import { link } from '../../../utils/link';
2021
import { Identifier } from 'estree';
2122

2223
const wrappers = {
@@ -38,11 +39,6 @@ const wrappers = {
3839
Window
3940
};
4041

41-
function link(next: Wrapper, prev: Wrapper) {
42-
prev.next = next;
43-
if (next) next.prev = prev;
44-
}
45-
4642
function trimmable_at(child: INode, next_sibling: Wrapper): boolean {
4743
// Whitespace is trimmable if one of the following is true:
4844
// The child and its sibling share a common nearest each block (not at an each block boundary)

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

+16-9
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,27 @@ export default class InlineComponentWrapper extends Wrapper {
138138
const statements: Array<Node | Node[]> = [];
139139
const updates: Array<Node | Node[]> = [];
140140

141+
if (this.fragment) {
142+
this.renderer.add_to_context('$$scope', true);
143+
const default_slot = this.slots.get('default');
144+
145+
this.fragment.nodes.forEach((child) => {
146+
child.render(default_slot.block, null, x`#nodes` as unknown as Identifier);
147+
});
148+
}
149+
141150
let props;
142151
const name_changes = block.get_unique_name(`${name.name}_changes`);
143152

144153
const uses_spread = !!this.node.attributes.find(a => a.is_spread);
145154

155+
// removing empty slot
156+
for (const slot of this.slots.keys()) {
157+
if (!this.slots.get(slot).block.has_content()) {
158+
this.slots.delete(slot);
159+
}
160+
}
161+
146162
const initial_props = this.slots.size > 0
147163
? [
148164
p`$$slots: {
@@ -172,15 +188,6 @@ export default class InlineComponentWrapper extends Wrapper {
172188
}
173189
}
174190

175-
if (this.fragment) {
176-
this.renderer.add_to_context('$$scope', true);
177-
const default_slot = this.slots.get('default');
178-
179-
this.fragment.nodes.forEach((child) => {
180-
child.render(default_slot.block, null, x`#nodes` as unknown as Identifier);
181-
});
182-
}
183-
184191
if (component.compile_options.dev) {
185192
// TODO this is a terrible hack, but without it the component
186193
// will complain that options.target is missing. This would

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

+1-31
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,6 @@ import Wrapper from './shared/Wrapper';
55
import { x } from 'code-red';
66
import { Identifier } from 'estree';
77

8-
// Whitespace inside one of these elements will not result in
9-
// a whitespace node being created in any circumstances. (This
10-
// list is almost certainly very incomplete)
11-
const elements_without_text = new Set([
12-
'audio',
13-
'datalist',
14-
'dl',
15-
'optgroup',
16-
'select',
17-
'video',
18-
]);
19-
20-
// TODO this should probably be in Fragment
21-
function should_skip(node: Text) {
22-
if (/\S/.test(node.data)) return false;
23-
24-
const parent_element = node.find_nearest(/(?:Element|InlineComponent|Head)/);
25-
if (!parent_element) return false;
26-
27-
if (parent_element.type === 'Head') return true;
28-
if (parent_element.type === 'InlineComponent') return parent_element.children.length === 1 && node === parent_element.children[0];
29-
30-
// svg namespace exclusions
31-
if (/svg$/.test(parent_element.namespace)) {
32-
if (node.prev && node.prev.type === "Element" && node.prev.name === "tspan") return false;
33-
}
34-
35-
return parent_element.namespace || elements_without_text.has(parent_element.name);
36-
}
37-
388
export default class TextWrapper extends Wrapper {
399
node: Text;
4010
data: string;
@@ -50,7 +20,7 @@ export default class TextWrapper extends Wrapper {
5020
) {
5121
super(renderer, block, parent, node);
5222

53-
this.skip = should_skip(this.node);
23+
this.skip = this.node.should_skip();
5424
this.data = data;
5525
this.var = (this.skip ? null : x`t`) as unknown as Identifier;
5626
}

‎src/compiler/compile/render_ssr/handlers/Element.ts

+8-3
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@ import Renderer, { RenderOptions } from '../Renderer';
66
import Element from '../../nodes/Element';
77
import { x } from 'code-red';
88
import Expression from '../../nodes/shared/Expression';
9+
import remove_whitespace_children from './utils/remove_whitespace_children';
910

1011
export default function(node: Element, renderer: Renderer, options: RenderOptions & {
1112
slot_scopes: Map<any, any>;
1213
}) {
14+
15+
const children = remove_whitespace_children(node.children, node.next);
16+
1317
// awkward special case
1418
let node_contents;
1519

@@ -133,7 +137,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
133137
if (node_contents !== undefined) {
134138
if (contenteditable) {
135139
renderer.push();
136-
renderer.render(node.children, options);
140+
renderer.render(children, options);
137141
const result = renderer.pop();
138142

139143
renderer.add_expression(x`($$value => $$value === void 0 ? ${result} : $$value)(${node_contents})`);
@@ -145,7 +149,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
145149
renderer.add_string(`</${node.name}>`);
146150
}
147151
} else if (slot && nearest_inline_component) {
148-
renderer.render(node.children, options);
152+
renderer.render(children, options);
149153

150154
if (!is_void(node.name)) {
151155
renderer.add_string(`</${node.name}>`);
@@ -163,10 +167,11 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
163167
output: renderer.pop()
164168
});
165169
} else {
166-
renderer.render(node.children, options);
170+
renderer.render(children, options);
167171

168172
if (!is_void(node.name)) {
169173
renderer.add_string(`</${node.name}>`);
170174
}
171175
}
172176
}
177+

‎src/compiler/compile/render_ssr/handlers/InlineComponent.ts

+18-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { string_literal } from '../../utils/stringify';
22
import Renderer, { RenderOptions } from '../Renderer';
33
import { get_slot_scope } from './shared/get_slot_scope';
44
import InlineComponent from '../../nodes/InlineComponent';
5+
import remove_whitespace_children from './utils/remove_whitespace_children';
56
import { p, x } from 'code-red';
67

78
function get_prop_value(attribute) {
@@ -67,12 +68,14 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
6768

6869
const slot_fns = [];
6970

70-
if (node.children.length) {
71+
const children = remove_whitespace_children(node.children, node.next);
72+
73+
if (children.length) {
7174
const slot_scopes = new Map();
7275

7376
renderer.push();
7477

75-
renderer.render(node.children, Object.assign({}, options, {
78+
renderer.render(children, Object.assign({}, options, {
7679
slot_scopes
7780
}));
7881

@@ -82,9 +85,11 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
8285
});
8386

8487
slot_scopes.forEach(({ input, output }, name) => {
85-
slot_fns.push(
86-
p`${name}: (${input}) => ${output}`
87-
);
88+
if (!is_empty_template_literal(output)) {
89+
slot_fns.push(
90+
p`${name}: (${input}) => ${output}`
91+
);
92+
}
8893
});
8994
}
9095

@@ -94,3 +99,11 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
9499

95100
renderer.add_expression(x`@validate_component(${expression}, "${node.name}").$$render($$result, ${props}, ${bindings}, ${slots})`);
96101
}
102+
103+
function is_empty_template_literal(template_literal) {
104+
return (
105+
template_literal.expressions.length === 0 &&
106+
template_literal.quasis.length === 1 &&
107+
template_literal.quasis[0].value.raw === ""
108+
);
109+
}

‎src/compiler/compile/render_ssr/handlers/utils/remove_whitespace_children.ts

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { INode } from '../../../nodes/interfaces';
2+
import { trim_end, trim_start } from '../../../../utils/trim';
3+
import { link } from '../../../../utils/link';
4+
5+
// similar logic from `compile/render_dom/wrappers/Fragment`
6+
// We want to remove trailing whitespace inside an element/component/block,
7+
// *unless* there is no whitespace between this node and its next sibling
8+
export default function remove_whitespace_children(children: INode[], next?: INode): INode[] {
9+
const nodes: INode[] = [];
10+
let last_child: INode;
11+
let i = children.length;
12+
while (i--) {
13+
const child = children[i];
14+
15+
if (child.type === 'Text') {
16+
if (child.should_skip()) {
17+
continue;
18+
}
19+
20+
let { data } = child;
21+
22+
if (nodes.length === 0) {
23+
const should_trim = next
24+
? next.type === 'Text' &&
25+
/^\s/.test(next.data) &&
26+
trimmable_at(child, next)
27+
: !child.has_ancestor('EachBlock');
28+
29+
if (should_trim) {
30+
data = trim_end(data);
31+
if (!data) continue;
32+
}
33+
}
34+
35+
// glue text nodes (which could e.g. be separated by comments) together
36+
if (last_child && last_child.type === 'Text') {
37+
last_child.data = data + last_child.data;
38+
continue;
39+
}
40+
41+
nodes.unshift(child);
42+
link(last_child, last_child = child);
43+
} else {
44+
nodes.unshift(child);
45+
link(last_child, last_child = child);
46+
}
47+
}
48+
49+
const first = nodes[0];
50+
if (first && first.type === 'Text') {
51+
first.data = trim_start(first.data);
52+
if (!first.data) {
53+
first.var = null;
54+
nodes.shift();
55+
56+
if (nodes[0]) {
57+
nodes[0].prev = null;
58+
}
59+
}
60+
}
61+
62+
return nodes;
63+
}
64+
65+
function trimmable_at(child: INode, next_sibling: INode): boolean {
66+
// Whitespace is trimmable if one of the following is true:
67+
// The child and its sibling share a common nearest each block (not at an each block boundary)
68+
// The next sibling's previous node is an each block
69+
return (
70+
next_sibling.find_nearest(/EachBlock/) ===
71+
child.find_nearest(/EachBlock/) || next_sibling.prev.type === 'EachBlock'
72+
);
73+
}

‎src/compiler/utils/link.ts

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export function link<T extends { next?: T; prev?: T }>(next: T, prev: T) {
2+
prev.next = next;
3+
if (next) next.prev = prev;
4+
}

‎test/runtime/samples/component-slot-fallback-empty/Nested.svelte

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
<div>
2+
<slot><p class='default'>default fallback content</p></slot>
3+
<slot name='bar'>bar fallback</slot>
4+
</div>

‎test/runtime/samples/component-slot-fallback-empty/_config.js

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
export default {
2+
html: `
3+
<div>
4+
<p class="default">default fallback content</p>
5+
<input slot="bar">
6+
</div>
7+
8+
<div>
9+
<p class="default">default fallback content</p>
10+
bar fallback
11+
</div>
12+
`
13+
};

‎test/runtime/samples/component-slot-fallback-empty/main.svelte

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
import Nested from "./Nested.svelte";
3+
</script>
4+
5+
<Nested>
6+
<input slot="bar">
7+
</Nested>
8+
9+
<Nested>
10+
</Nested>

0 commit comments

Comments
 (0)