diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cdc334868..b0672d790e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/src/compiler/compile/nodes/Text.ts b/src/compiler/compile/nodes/Text.ts index bfd28a5073..6b7432c22f 100644 --- a/src/compiler/compile/nodes/Text.ts +++ b/src/compiler/compile/nodes/Text.ts @@ -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); + } } diff --git a/src/compiler/compile/render_dom/wrappers/Fragment.ts b/src/compiler/compile/render_dom/wrappers/Fragment.ts index 224b17d43f..a0984b69b9 100644 --- a/src/compiler/compile/render_dom/wrappers/Fragment.ts +++ b/src/compiler/compile/render_dom/wrappers/Fragment.ts @@ -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) diff --git a/src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts b/src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts index 8088bbac91..8c8bd70696 100644 --- a/src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts +++ b/src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts @@ -138,11 +138,27 @@ export default class InlineComponentWrapper extends Wrapper { const statements: Array = []; const updates: Array = []; + 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 diff --git a/src/compiler/compile/render_dom/wrappers/Text.ts b/src/compiler/compile/render_dom/wrappers/Text.ts index 1978cba0d7..7ef8aebd70 100644 --- a/src/compiler/compile/render_dom/wrappers/Text.ts +++ b/src/compiler/compile/render_dom/wrappers/Text.ts @@ -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; } diff --git a/src/compiler/compile/render_ssr/handlers/Element.ts b/src/compiler/compile/render_ssr/handlers/Element.ts index 6e6a61974a..ad5c421bc4 100644 --- a/src/compiler/compile/render_ssr/handlers/Element.ts +++ b/src/compiler/compile/render_ssr/handlers/Element.ts @@ -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; }) { + + 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(``); } } else if (slot && nearest_inline_component) { - renderer.render(node.children, options); + renderer.render(children, options); if (!is_void(node.name)) { renderer.add_string(``); @@ -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(``); } } } + diff --git a/src/compiler/compile/render_ssr/handlers/InlineComponent.ts b/src/compiler/compile/render_ssr/handlers/InlineComponent.ts index 5c4d9c73b8..37f05a941c 100644 --- a/src/compiler/compile/render_ssr/handlers/InlineComponent.ts +++ b/src/compiler/compile/render_ssr/handlers/InlineComponent.ts @@ -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 === "" + ); +} \ No newline at end of file diff --git a/src/compiler/compile/render_ssr/handlers/utils/remove_whitespace_children.ts b/src/compiler/compile/render_ssr/handlers/utils/remove_whitespace_children.ts new file mode 100644 index 0000000000..b6c1bf493a --- /dev/null +++ b/src/compiler/compile/render_ssr/handlers/utils/remove_whitespace_children.ts @@ -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' + ); +} diff --git a/src/compiler/utils/link.ts b/src/compiler/utils/link.ts new file mode 100644 index 0000000000..7cdef75d94 --- /dev/null +++ b/src/compiler/utils/link.ts @@ -0,0 +1,4 @@ +export function link(next: T, prev: T) { + prev.next = next; + if (next) next.prev = prev; +} diff --git a/test/runtime/samples/component-slot-fallback-empty/Nested.svelte b/test/runtime/samples/component-slot-fallback-empty/Nested.svelte new file mode 100644 index 0000000000..9e6683feb7 --- /dev/null +++ b/test/runtime/samples/component-slot-fallback-empty/Nested.svelte @@ -0,0 +1,4 @@ +
+

default fallback content

 + bar fallback +
\ No newline at end of file diff --git a/test/runtime/samples/component-slot-fallback-empty/_config.js b/test/runtime/samples/component-slot-fallback-empty/_config.js new file mode 100644 index 0000000000..2e153d24db --- /dev/null +++ b/test/runtime/samples/component-slot-fallback-empty/_config.js @@ -0,0 +1,13 @@ +export default { + html: ` +
+

default fallback content

+ +
+ +
+

default fallback content

+ bar fallback +
+ ` +}; diff --git a/test/runtime/samples/component-slot-fallback-empty/main.svelte b/test/runtime/samples/component-slot-fallback-empty/main.svelte new file mode 100644 index 0000000000..7ae5f4c5d7 --- /dev/null +++ b/test/runtime/samples/component-slot-fallback-empty/main.svelte @@ -0,0 +1,10 @@ + + + + + + + +