Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: deduplicate children prop from default slot #10800

Merged
merged 14 commits into from May 15, 2024
5 changes: 5 additions & 0 deletions .changeset/cold-cheetahs-judge.md
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: deduplicate children prop and default slot
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
Expand Up @@ -970,6 +970,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 @@ -998,6 +1005,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 @@ -1072,14 +1083,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 @@ -1736,7 +1750,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
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
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
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
Expand Up @@ -276,8 +276,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 @@ -630,3 +631,5 @@ export {
} from '../shared/validate.js';

export { escape_html as escape };

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

{children}
<slot />
@@ -0,0 +1,5 @@
import { test } from '../../test';

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

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

<A children="foo" />
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
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