fix render fallback slot content due to whitespace (#4500)

pull/4531/head
Tan Li Hau 5 years ago committed by GitHub
parent addea43e4f
commit 926a2aebd8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

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

@ -3,6 +3,18 @@ import Component from '../Component';
import TemplateScope from './shared/TemplateScope';
import { INode } from './interfaces';
// Whitespace inside one of these elements will not result in
// a whitespace node being created in any circumstances. (This
// list is almost certainly very incomplete)
const elements_without_text = new Set([
'audio',
'datalist',
'dl',
'optgroup',
'select',
'video',
]);
export default class Text extends Node {
type: 'Text';
data: string;
@ -13,4 +25,21 @@ export default class Text extends Node {
this.data = info.data;
this.synthetic = info.synthetic || false;
}
should_skip() {
if (/\S/.test(this.data)) return false;
const parent_element = this.find_nearest(/(?:Element|InlineComponent|Head)/);
if (!parent_element) return false;
if (parent_element.type === 'Head') return true;
if (parent_element.type === 'InlineComponent') return parent_element.children.length === 1 && this === parent_element.children[0];
// svg namespace exclusions
if (/svg$/.test(parent_element.namespace)) {
if (this.prev && this.prev.type === "Element" && this.prev.name === "tspan") return false;
}
return parent_element.namespace || elements_without_text.has(parent_element.name);
}
}

@ -17,6 +17,7 @@ import { INode } from '../../nodes/interfaces';
import Renderer from '../Renderer';
import Block from '../Block';
import { trim_start, trim_end } from '../../../utils/trim';
import { link } from '../../../utils/link';
import { Identifier } from 'estree';
const wrappers = {
@ -38,11 +39,6 @@ const wrappers = {
Window
};
function link(next: Wrapper, prev: Wrapper) {
prev.next = next;
if (next) next.prev = prev;
}
function trimmable_at(child: INode, next_sibling: Wrapper): boolean {
// Whitespace is trimmable if one of the following is true:
// The child and its sibling share a common nearest each block (not at an each block boundary)

@ -138,11 +138,27 @@ export default class InlineComponentWrapper extends Wrapper {
const statements: Array<Node | Node[]> = [];
const updates: Array<Node | Node[]> = [];
if (this.fragment) {
this.renderer.add_to_context('$$scope', true);
const default_slot = this.slots.get('default');
this.fragment.nodes.forEach((child) => {
child.render(default_slot.block, null, x`#nodes` as unknown as Identifier);
});
}
let props;
const name_changes = block.get_unique_name(`${name.name}_changes`);
const uses_spread = !!this.node.attributes.find(a => a.is_spread);
// removing empty slot
for (const slot of this.slots.keys()) {
if (!this.slots.get(slot).block.has_content()) {
this.slots.delete(slot);
}
}
const initial_props = this.slots.size > 0
? [
p`$$slots: {
@ -172,15 +188,6 @@ export default class InlineComponentWrapper extends Wrapper {
}
}
if (this.fragment) {
this.renderer.add_to_context('$$scope', true);
const default_slot = this.slots.get('default');
this.fragment.nodes.forEach((child) => {
child.render(default_slot.block, null, x`#nodes` as unknown as Identifier);
});
}
if (component.compile_options.dev) {
// TODO this is a terrible hack, but without it the component
// will complain that options.target is missing. This would

@ -5,36 +5,6 @@ import Wrapper from './shared/Wrapper';
import { x } from 'code-red';
import { Identifier } from 'estree';
// Whitespace inside one of these elements will not result in
// a whitespace node being created in any circumstances. (This
// list is almost certainly very incomplete)
const elements_without_text = new Set([
'audio',
'datalist',
'dl',
'optgroup',
'select',
'video',
]);
// TODO this should probably be in Fragment
function should_skip(node: Text) {
if (/\S/.test(node.data)) return false;
const parent_element = node.find_nearest(/(?:Element|InlineComponent|Head)/);
if (!parent_element) return false;
if (parent_element.type === 'Head') return true;
if (parent_element.type === 'InlineComponent') return parent_element.children.length === 1 && node === parent_element.children[0];
// svg namespace exclusions
if (/svg$/.test(parent_element.namespace)) {
if (node.prev && node.prev.type === "Element" && node.prev.name === "tspan") return false;
}
return parent_element.namespace || elements_without_text.has(parent_element.name);
}
export default class TextWrapper extends Wrapper {
node: Text;
data: string;
@ -50,7 +20,7 @@ export default class TextWrapper extends Wrapper {
) {
super(renderer, block, parent, node);
this.skip = should_skip(this.node);
this.skip = this.node.should_skip();
this.data = data;
this.var = (this.skip ? null : x`t`) as unknown as Identifier;
}

@ -6,10 +6,14 @@ import Renderer, { RenderOptions } from '../Renderer';
import Element from '../../nodes/Element';
import { x } from 'code-red';
import Expression from '../../nodes/shared/Expression';
import remove_whitespace_children from './utils/remove_whitespace_children';
export default function(node: Element, renderer: Renderer, options: RenderOptions & {
slot_scopes: Map<any, any>;
}) {
const children = remove_whitespace_children(node.children, node.next);
// awkward special case
let node_contents;
@ -133,7 +137,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
if (node_contents !== undefined) {
if (contenteditable) {
renderer.push();
renderer.render(node.children, options);
renderer.render(children, options);
const result = renderer.pop();
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
renderer.add_string(`</${node.name}>`);
}
} else if (slot && nearest_inline_component) {
renderer.render(node.children, options);
renderer.render(children, options);
if (!is_void(node.name)) {
renderer.add_string(`</${node.name}>`);
@ -163,10 +167,11 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
output: renderer.pop()
});
} else {
renderer.render(node.children, options);
renderer.render(children, options);
if (!is_void(node.name)) {
renderer.add_string(`</${node.name}>`);
}
}
}

@ -2,6 +2,7 @@ import { string_literal } from '../../utils/stringify';
import Renderer, { RenderOptions } from '../Renderer';
import { get_slot_scope } from './shared/get_slot_scope';
import InlineComponent from '../../nodes/InlineComponent';
import remove_whitespace_children from './utils/remove_whitespace_children';
import { p, x } from 'code-red';
function get_prop_value(attribute) {
@ -67,12 +68,14 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
const slot_fns = [];
if (node.children.length) {
const children = remove_whitespace_children(node.children, node.next);
if (children.length) {
const slot_scopes = new Map();
renderer.push();
renderer.render(node.children, Object.assign({}, options, {
renderer.render(children, Object.assign({}, options, {
slot_scopes
}));
@ -82,9 +85,11 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
});
slot_scopes.forEach(({ input, output }, name) => {
slot_fns.push(
p`${name}: (${input}) => ${output}`
);
if (!is_empty_template_literal(output)) {
slot_fns.push(
p`${name}: (${input}) => ${output}`
);
}
});
}
@ -94,3 +99,11 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
renderer.add_expression(x`@validate_component(${expression}, "${node.name}").$$render($$result, ${props}, ${bindings}, ${slots})`);
}
function is_empty_template_literal(template_literal) {
return (
template_literal.expressions.length === 0 &&
template_literal.quasis.length === 1 &&
template_literal.quasis[0].value.raw === ""
);
}

@ -0,0 +1,73 @@
import { INode } from '../../../nodes/interfaces';
import { trim_end, trim_start } from '../../../../utils/trim';
import { link } from '../../../../utils/link';
// similar logic from `compile/render_dom/wrappers/Fragment`
// We want to remove trailing whitespace inside an element/component/block,
// *unless* there is no whitespace between this node and its next sibling
export default function remove_whitespace_children(children: INode[], next?: INode): INode[] {
const nodes: INode[] = [];
let last_child: INode;
let i = children.length;
while (i--) {
const child = children[i];
if (child.type === 'Text') {
if (child.should_skip()) {
continue;
}
let { data } = child;
if (nodes.length === 0) {
const should_trim = next
? next.type === 'Text' &&
/^\s/.test(next.data) &&
trimmable_at(child, next)
: !child.has_ancestor('EachBlock');
if (should_trim) {
data = trim_end(data);
if (!data) continue;
}
}
// glue text nodes (which could e.g. be separated by comments) together
if (last_child && last_child.type === 'Text') {
last_child.data = data + last_child.data;
continue;
}
nodes.unshift(child);
link(last_child, last_child = child);
} else {
nodes.unshift(child);
link(last_child, last_child = child);
}
}
const first = nodes[0];
if (first && first.type === 'Text') {
first.data = trim_start(first.data);
if (!first.data) {
first.var = null;
nodes.shift();
if (nodes[0]) {
nodes[0].prev = null;
}
}
}
return nodes;
}
function trimmable_at(child: INode, next_sibling: INode): boolean {
// Whitespace is trimmable if one of the following is true:
// The child and its sibling share a common nearest each block (not at an each block boundary)
// The next sibling's previous node is an each block
return (
next_sibling.find_nearest(/EachBlock/) ===
child.find_nearest(/EachBlock/) || next_sibling.prev.type === 'EachBlock'
);
}

@ -0,0 +1,4 @@
export function link<T extends { next?: T; prev?: T }>(next: T, prev: T) {
prev.next = next;
if (next) next.prev = prev;
}

@ -0,0 +1,4 @@
<div>
<slot><p class='default'>default fallback content</p></slot>
<slot name='bar'>bar fallback</slot>
</div>

@ -0,0 +1,13 @@
export default {
html: `
<div>
<p class="default">default fallback content</p>
<input slot="bar">
</div>
<div>
<p class="default">default fallback content</p>
bar fallback
</div>
`
};

@ -0,0 +1,10 @@
<script>
import Nested from "./Nested.svelte";
</script>
<Nested>
<input slot="bar">
</Nested>
<Nested>
</Nested>
Loading…
Cancel
Save