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 @@ -627,6 +627,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 @@ -676,6 +683,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 @@ -797,13 +808,8 @@ function serialize_inline_component(node, component_name, context) {
b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body])
);

if (slot_name === 'default') {
push_prop(
b.init(
'children',
context.state.options.dev ? b.call('$.add_snippet_symbol', slot_fn) : slot_fn
)
);
if (slot_name === 'default' && !has_children_prop) {
push_prop(b.init('children', b.call('$.add_snippet_symbol', slot_fn)));
} else {
serialized_slots.push(b.init(slot_name, slot_fn));
}
Expand Down Expand Up @@ -2509,9 +2515,7 @@ export const template_visitors = {
} else {
context.state.init.push(b.const(node.expression, b.arrow(args, body)));
}
if (context.state.options.dev) {
context.state.init.push(b.stmt(b.call('$.add_snippet_symbol', node.expression)));
}
context.state.init.push(b.stmt(b.call('$.add_snippet_symbol', node.expression)));
},
FunctionExpression: function_visitor,
ArrowFunctionExpression: function_visitor,
Expand Down Expand Up @@ -2934,7 +2938,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 @@ -962,6 +962,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 @@ -990,6 +997,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 @@ -1064,14 +1075,8 @@ function serialize_inline_component(node, component_name, context) {
b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body])
);

if (slot_name === 'default') {
push_prop(
b.prop(
'init',
b.id('children'),
context.state.options.dev ? b.call('$.add_snippet_symbol', slot_fn) : slot_fn
)
);
if (slot_name === 'default' && !has_children_prop) {
push_prop(b.prop('init', b.id('children'), b.call('$.add_snippet_symbol', slot_fn)));
} else {
const slot = b.prop('init', b.literal(slot_name), slot_fn);
serialized_slots.push(slot);
Expand Down Expand Up @@ -1601,9 +1606,7 @@ const template_visitors = {
)
);

if (context.state.options.dev) {
context.state.init.push(b.stmt(b.call('$.add_snippet_symbol', node.expression)));
}
context.state.init.push(b.stmt(b.call('$.add_snippet_symbol', node.expression)));
},
Component(node, context) {
const state = context.state;
Expand Down Expand Up @@ -1723,7 +1726,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
19 changes: 19 additions & 0 deletions packages/svelte/src/internal/client/dom/blocks/snippet.js
Expand Up @@ -19,3 +19,22 @@ export function snippet(get_snippet, node, ...args) {
}
});
}

const snippet_symbol = Symbol.for('svelte.snippet');

/**
* @param {any} fn
*/
export function add_snippet_symbol(fn) {
fn[snippet_symbol] = true;
return fn;
}

/**
* Returns true if given parameter is a snippet.
* @param {any} maybeSnippet
* @returns {maybeSnippet is import('svelte').Snippet}
*/
export function is_snippet(maybeSnippet) {
return /** @type {any} */ (maybeSnippet)?.[snippet_symbol] === true;
}
@@ -1,5 +1,6 @@
import { createClassComponent } from '../../../../legacy/legacy-client.js';
import { destroy_effect, render_effect } from '../../reactivity/effects.js';
import { add_snippet_symbol } from '../blocks/snippet.js';
import { append } from '../template.js';
import { define_property, object_keys } from '../../utils.js';

Expand Down Expand Up @@ -109,8 +110,8 @@ 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') {
this.$$d.children = create_slot(name);
if (name === 'default' && !this.$$d.children) {
this.$$d.children = add_snippet_symbol(create_slot(name));
} else {
$$slots[name] = create_slot(name);
}
Expand Down
15 changes: 15 additions & 0 deletions packages/svelte/src/internal/client/dom/legacy/misc.js
@@ -1,6 +1,7 @@
import { set, source } from '../../reactivity/sources.js';
import { get } from '../../runtime.js';
import { is_array } from '../../utils.js';
import { is_snippet } from '../blocks/snippet.js';

/**
* Under some circumstances, imports may be reactive in legacy mode. In that case,
Expand Down Expand Up @@ -66,3 +67,17 @@ export function update_legacy_props($$new_props) {
}
}
}

/**
* @param {Record<string, any>} $$props
*/
export function default_slot($$props) {
var children = $$props.$$slots?.default;
if (children) {
return children;
}
children = $$props.children;
if (is_snippet(children)) {
return children;
}
}
6 changes: 3 additions & 3 deletions packages/svelte/src/internal/client/index.js
Expand Up @@ -5,7 +5,7 @@ export { key_block as key } from './dom/blocks/key.js';
export { css_props } from './dom/blocks/css-props.js';
export { each_keyed, each_indexed } from './dom/blocks/each.js';
export { html } from './dom/blocks/html.js';
export { snippet } from './dom/blocks/snippet.js';
export { snippet, add_snippet_symbol } from './dom/blocks/snippet.js';
export { component } from './dom/blocks/svelte-component.js';
export { element } from './dom/blocks/svelte-element.js';
export { head } from './dom/blocks/svelte-head.js';
Expand Down Expand Up @@ -57,7 +57,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 @@ -137,7 +138,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
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/validate.js
@@ -1,3 +1,4 @@
import { is_snippet } from './dom/blocks/snippet.js';
import { untrack } from './runtime.js';
import { get_descriptor, is_array } from './utils.js';

Expand Down
5 changes: 4 additions & 1 deletion packages/svelte/src/internal/server/index.js
Expand Up @@ -8,6 +8,10 @@ import {
is_tag_valid_with_parent
} from '../../constants.js';
import { DEV } from 'esm-env';

export * from '../client/validate.js';
export { add_snippet_symbol } from '../client/dom/blocks/snippet.js';
export { default_slot } from '../client/dom/legacy/misc.js';
import { current_component, pop, push } from './context.js';
import { BLOCK_CLOSE, BLOCK_OPEN } from './hydration.js';

Expand Down Expand Up @@ -660,7 +664,6 @@ export function once(get_value) {
export { push, pop } from './context.js';

export {
add_snippet_symbol,
validate_component,
validate_dynamic_element_tag,
validate_snippet,
Expand Down
15 changes: 3 additions & 12 deletions packages/svelte/src/internal/shared/validate.js
@@ -1,21 +1,12 @@
import { is_void } from '../../compiler/phases/1-parse/utils/names.js';

const snippet_symbol = Symbol.for('svelte.snippet');

/**
* @param {any} fn
*/
export function add_snippet_symbol(fn) {
fn[snippet_symbol] = true;
return fn;
}
import { is_snippet } from '../client/dom/blocks/snippet.js';

/**
* Validate that the function handed to `{@render ...}` is a snippet function, and not some other kind of function.
* @param {any} snippet_fn
*/
export function validate_snippet(snippet_fn) {
if (snippet_fn && snippet_fn[snippet_symbol] !== true) {
if (snippet_fn && !is_snippet(snippet_fn)) {
throw new Error(
'The argument to `{@render ...}` must be a snippet function, not a component or some other kind of function. ' +
'If you want to dynamically render one snippet or another, use `$derived` and pass its result to `{@render ...}`.'
Expand All @@ -29,7 +20,7 @@ export function validate_snippet(snippet_fn) {
* @param {any} component_fn
*/
export function validate_component(component_fn) {
if (component_fn?.[snippet_symbol] === true) {
if (is_snippet(component_fn)) {
throw new Error('A snippet must be rendered with `{@render ...}`');
}
return component_fn;
Expand Down
@@ -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 @@ -20,12 +20,12 @@ export default function Function_prop_no_getter($$anchor, $$props) {
onmousedown: () => $.set(count, $.get(count) + 1),
onmouseup,
onmouseenter: () => $.set(count, $.proxy(plusOne($.get(count)))),
children: ($$anchor, $$slotProps) => {
children: $.add_snippet_symbol(($$anchor, $$slotProps) => {
var text = $.text($$anchor);

$.render_effect(() => $.set_text(text, `clicks: ${$.stringify($.get(count))}`));
$.append($$anchor, text);
}
})
});

$.append($$anchor, fragment);
Expand Down
Expand Up @@ -19,9 +19,9 @@ export default function Function_prop_no_getter($$payload, $$props) {
onmousedown: () => count += 1,
onmouseup,
onmouseenter: () => count = plusOne(count),
children: ($$payload, $$slotProps) => {
children: $.add_snippet_symbol(($$payload, $$slotProps) => {
$$payload.out += `clicks: ${$.escape(count)}`;
}
})
});

$$payload.out += `<!--]-->`;
Expand Down