Skip to content

Commit

Permalink
fix: deduplicate children prop from default slot (#10800)
Browse files Browse the repository at this point in the history
* 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 <rich.harris@vercel.com>
  • Loading branch information
dummdidumm and Rich-Harris committed May 15, 2024
1 parent cac8630 commit 4365562
Show file tree
Hide file tree
Showing 12 changed files with 82 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/cold-cheetahs-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: deduplicate children prop and default slot
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -1080,14 +1091,17 @@ 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',
b.id('children'),
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);
Expand Down Expand Up @@ -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') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
12 changes: 12 additions & 0 deletions packages/svelte/src/internal/client/dom/legacy/misc.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,15 @@ export function update_legacy_props($$new_props) {
}
}
}

/**
* @param {Record<string, any>} $$props
*/
export function default_slot($$props) {
var children = $$props.$$slots?.default;
if (children === true) {
return $$props.children;
} else {
return children;
}
}
4 changes: 2 additions & 2 deletions packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions packages/svelte/src/internal/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
}
Expand Down Expand Up @@ -549,3 +550,5 @@ export {
} from '../shared/validate.js';

export { escape_html as escape };

export { default_slot } from '../client/dom/legacy/misc.js';
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
export let children;
</script>

{children}
<slot />
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { test } from '../../test';

export default test({
html: `foo bar foo`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<script>
import A from "./A.svelte";
</script>

<A children="foo">
bar
</A>

<A children="foo" />
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 += `<!--]-->`;
Expand Down

0 comments on commit 4365562

Please sign in to comment.