From 4365562228870aaa5c7d85cfd27d03d929a5f295 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 15 May 2024 18:50:29 +0200 Subject: [PATCH] fix: deduplicate children prop from default slot (#10800) * feat: provide isSnippet type, deduplicate children prop from default slot fixes #10790 part of #9774 * fix ce bug * remove isSnippet type, adjust test * fix types * revert unrelated changes * remove changeset * enhance test * fix * fix * fix * fix, different approach without needing symbol --------- Co-authored-by: Rich Harris --- .changeset/cold-cheetahs-judge.md | 5 +++++ .../3-transform/client/visitors/template.js | 18 ++++++++++++++++-- .../3-transform/server/transform-server.js | 18 ++++++++++++++++-- .../client/dom/elements/custom-element.js | 3 ++- .../src/internal/client/dom/legacy/misc.js | 12 ++++++++++++ packages/svelte/src/internal/client/index.js | 4 ++-- packages/svelte/src/internal/server/index.js | 7 +++++-- .../samples/slot-children-prop/A.svelte | 6 ++++++ .../samples/slot-children-prop/_config.js | 5 +++++ .../samples/slot-children-prop/main.svelte | 9 +++++++++ .../_expected/client/index.svelte.js | 3 ++- .../_expected/server/index.svelte.js | 3 ++- 12 files changed, 82 insertions(+), 11 deletions(-) create mode 100644 .changeset/cold-cheetahs-judge.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/slot-children-prop/A.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/slot-children-prop/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/slot-children-prop/main.svelte diff --git a/.changeset/cold-cheetahs-judge.md b/.changeset/cold-cheetahs-judge.md new file mode 100644 index 000000000000..63302f29e866 --- /dev/null +++ b/.changeset/cold-cheetahs-judge.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: deduplicate children prop and default slot diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 34ffe66a874b..85fcf7afe30a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -660,6 +660,13 @@ function serialize_inline_component(node, component_name, context) { */ let slot_scope_applies_to_itself = false; + /** + * Components may have a children prop and also have child nodes. In this case, we assume + * that the child component isn't using render tags yet and pass the slot as $$slots.default. + * We're not doing it for spread attributes, as this would result in too many false positives. + */ + let has_children_prop = false; + /** * @param {import('estree').Property} prop */ @@ -709,6 +716,10 @@ function serialize_inline_component(node, component_name, context) { slot_scope_applies_to_itself = true; } + if (attribute.name === 'children') { + has_children_prop = true; + } + const [, value] = serialize_attribute_value(attribute.value, context); if (attribute.metadata.dynamic) { @@ -825,10 +836,13 @@ function serialize_inline_component(node, component_name, context) { b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body]) ); - if (slot_name === 'default') { + if (slot_name === 'default' && !has_children_prop) { push_prop( b.init('children', context.state.options.dev ? b.call('$.wrap_snippet', slot_fn) : slot_fn) ); + // We additionally add the default slot as a boolean, so that the slot render function on the other + // side knows it should get the content to render from $$props.children + serialized_slots.push(b.init(slot_name, b.true)); } else { serialized_slots.push(b.init(slot_name, slot_fn)); } @@ -3057,7 +3071,7 @@ export const template_visitors = { ); const expression = is_default - ? b.member(b.id('$$props'), b.id('children')) + ? b.call('$.default_slot', b.id('$$props')) : b.member(b.member(b.id('$$props'), b.id('$$slots')), name, true, true); const slot = b.call('$.slot', context.state.node, expression, props_expression, fallback); diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index ab362e6a1725..c20cf0ef5dbe 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -978,6 +978,13 @@ function serialize_inline_component(node, component_name, context) { */ let slot_scope_applies_to_itself = false; + /** + * Components may have a children prop and also have child nodes. In this case, we assume + * that the child component isn't using render tags yet and pass the slot as $$slots.default. + * We're not doing it for spread attributes, as this would result in too many false positives. + */ + let has_children_prop = false; + /** * @param {import('estree').Property} prop */ @@ -1006,6 +1013,10 @@ function serialize_inline_component(node, component_name, context) { slot_scope_applies_to_itself = true; } + if (attribute.name === 'children') { + has_children_prop = true; + } + const value = serialize_attribute_value(attribute.value, context, false, true); push_prop(b.prop('init', b.key(attribute.name), value)); } else if (attribute.type === 'BindDirective' && attribute.name !== 'this') { @@ -1080,7 +1091,7 @@ function serialize_inline_component(node, component_name, context) { b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body]) ); - if (slot_name === 'default') { + if (slot_name === 'default' && !has_children_prop) { push_prop( b.prop( 'init', @@ -1088,6 +1099,9 @@ function serialize_inline_component(node, component_name, context) { context.state.options.dev ? b.call('$.add_snippet_symbol', slot_fn) : slot_fn ) ); + // We additionally add the default slot as a boolean, so that the slot render function on the other + // side knows it should get the content to render from $$props.children + serialized_slots.push(b.init('default', b.true)); } else { const slot = b.prop('init', b.literal(slot_name), slot_fn); serialized_slots.push(slot); @@ -1755,7 +1769,7 @@ const template_visitors = { const lets = []; /** @type {import('estree').Expression} */ - let expression = b.member_id('$$props.children'); + let expression = b.call('$.default_slot', b.id('$$props')); for (const attribute of node.attributes) { if (attribute.type === 'SpreadAttribute') { diff --git a/packages/svelte/src/internal/client/dom/elements/custom-element.js b/packages/svelte/src/internal/client/dom/elements/custom-element.js index 36b4ff6afdb0..ae966651e8c9 100644 --- a/packages/svelte/src/internal/client/dom/elements/custom-element.js +++ b/packages/svelte/src/internal/client/dom/elements/custom-element.js @@ -109,8 +109,9 @@ if (typeof HTMLElement === 'function') { const existing_slots = get_custom_elements_slots(this); for (const name of this.$$s) { if (name in existing_slots) { - if (name === 'default') { + if (name === 'default' && !this.$$d.children) { this.$$d.children = create_slot(name); + $$slots.default = true; } else { $$slots[name] = create_slot(name); } diff --git a/packages/svelte/src/internal/client/dom/legacy/misc.js b/packages/svelte/src/internal/client/dom/legacy/misc.js index a4b52efe283c..b9ef9fc53a5b 100644 --- a/packages/svelte/src/internal/client/dom/legacy/misc.js +++ b/packages/svelte/src/internal/client/dom/legacy/misc.js @@ -66,3 +66,15 @@ export function update_legacy_props($$new_props) { } } } + +/** + * @param {Record} $$props + */ +export function default_slot($$props) { + var children = $$props.$$slots?.default; + if (children === true) { + return $$props.children; + } else { + return children; + } +} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 6ea507bf8e83..dd0d759d8bbd 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -72,7 +72,8 @@ export { add_legacy_event_listener, bubble_event, reactive_import, - update_legacy_props + update_legacy_props, + default_slot } from './dom/legacy/misc.js'; export { append, @@ -154,7 +155,6 @@ export { } from './dom/operations.js'; export { noop } from '../shared/utils.js'; export { - add_snippet_symbol, validate_component, validate_dynamic_element_tag, validate_snippet, diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 3d319213644d..f55a72846cc4 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -193,8 +193,9 @@ export function spread_attributes(attrs, lowercase_attributes, is_html, class_ha for (let i = 0; i < attrs.length; i++) { const obj = attrs[i]; for (key in obj) { - // omit functions - if (typeof obj[key] !== 'function') { + // omit functions and internal svelte properties + const prefix = key[0] + key[1]; // this is faster than key.slice(0, 2) + if (typeof obj[key] !== 'function' && prefix !== '$$') { merged_attrs[key] = obj[key]; } } @@ -549,3 +550,5 @@ export { } from '../shared/validate.js'; export { escape_html as escape }; + +export { default_slot } from '../client/dom/legacy/misc.js'; diff --git a/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/A.svelte b/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/A.svelte new file mode 100644 index 000000000000..32735e38d6fa --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/A.svelte @@ -0,0 +1,6 @@ + + +{children} + diff --git a/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/_config.js b/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/_config.js new file mode 100644 index 000000000000..48b7168f308c --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/_config.js @@ -0,0 +1,5 @@ +import { test } from '../../test'; + +export default test({ + html: `foo bar foo` +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/main.svelte b/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/main.svelte new file mode 100644 index 000000000000..63c5cc3aae74 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/slot-children-prop/main.svelte @@ -0,0 +1,9 @@ + + + + bar + + + diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js index a8de35aae118..c44f9f143305 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js @@ -21,7 +21,8 @@ export default function Function_prop_no_getter($$anchor) { $.template_effect(() => $.set_text(text, `clicks: ${$.stringify($.get(count))}`)); $.append($$anchor, text); - } + }, + $$slots: { default: true } }); $.append($$anchor, fragment); diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js index ae468ceb9437..b096c4b6c0d7 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js @@ -19,7 +19,8 @@ export default function Function_prop_no_getter($$payload, $$props) { onmouseenter: () => count = plusOne(count), children: ($$payload, $$slotProps) => { $$payload.out += `clicks: ${$.escape(count)}`; - } + }, + $$slots: { default: true } }); $$payload.out += ``;