Skip to content

chore: more slot/snippet interop #11619

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

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -778,6 +789,8 @@ function serialize_inline_component(node, component_name, context) {

/** @type {import('estree').Statement[]} */
const snippet_declarations = [];
/** @type {import('estree').Property[]} */
const serialized_slots = [];

// Group children by slot
for (const child of node.fragment.nodes) {
Expand All @@ -791,6 +804,8 @@ function serialize_inline_component(node, component_name, context) {
});

push_prop(b.prop('init', child.expression, child.expression));
// Back/forward compatibility: allows people to pass snippets when component still uses slots
serialized_slots.push(b.init(child.expression.name, b.true));

continue;
}
Expand All @@ -814,8 +829,6 @@ function serialize_inline_component(node, component_name, context) {
}

// Serialize each slot
/** @type {import('estree').Property[]} */
const serialized_slots = [];
for (const slot_name of Object.keys(children)) {
const body = create_block(node, `${node.name}_${slot_name}`, children[slot_name], context);
if (body.length === 0) continue;
Expand All @@ -825,10 +838,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 @@ -1770,6 +1786,10 @@ export const template_visitors = {
const raw_args = unwrap_optional(node.expression).arguments;
const is_reactive =
callee.type !== 'Identifier' || context.state.scope.get(callee.name)?.kind !== 'normal';
const needs_backwards_compat =
callee.type === 'Identifier' &&
raw_args.length < 2 &&
context.state.scope.get(callee.name)?.kind === 'prop';

/** @type {import('estree').Expression[]} */
const args = [context.state.node];
Expand All @@ -1782,7 +1802,19 @@ export const template_visitors = {
snippet_function = b.call('$.validate_snippet', snippet_function);
}

if (is_reactive) {
if (needs_backwards_compat) {
context.state.init.push(
b.stmt(
b.call(
'$.render_snippet_or_slot',
b.thunk(snippet_function),
b.id('$$props'),
b.literal(callee.name),
...args
)
)
);
} else if (is_reactive) {
context.state.init.push(b.stmt(b.call('$.snippet', b.thunk(snippet_function), ...args)));
} else {
context.state.init.push(
Expand Down Expand Up @@ -3056,11 +3088,14 @@ export const template_visitors = {
b.block(create_block(node, 'fallback', node.fragment.nodes, context))
);

const expression = is_default
? b.member(b.id('$$props'), b.id('children'))
: 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);
const slot = b.call(
'$.slot',
context.state.node,
b.id('$$props'),
name,
props_expression,
fallback
);
context.state.init.push(b.stmt(slot));
},
SvelteHead(node, context) {
Expand Down
Original file line number Diff line number Diff line change
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 @@ -1026,6 +1037,8 @@ function serialize_inline_component(node, component_name, context) {

/** @type {import('estree').Statement[]} */
const snippet_declarations = [];
/** @type {import('estree').Property[]} */
const serialized_slots = [];

// Group children by slot
for (const child of node.fragment.nodes) {
Expand All @@ -1039,6 +1052,8 @@ function serialize_inline_component(node, component_name, context) {
});

push_prop(b.prop('init', child.expression, child.expression));
// Back/forward compatibility: allows people to pass snippets when component still uses slots
serialized_slots.push(b.init(child.expression.name, b.true));

continue;
}
Expand All @@ -1060,9 +1075,6 @@ function serialize_inline_component(node, component_name, context) {
}

// Serialize each slot
/** @type {import('estree').Property[]} */
const serialized_slots = [];

for (const slot_name of Object.keys(children)) {
const body = create_block(node, children[slot_name], context);
if (body.length === 0) continue;
Expand All @@ -1072,14 +1084,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 @@ -1288,6 +1303,10 @@ const template_visitors = {

const callee = unwrap_optional(node.expression).callee;
const raw_args = unwrap_optional(node.expression).arguments;
const needs_backwards_compat =
callee.type === 'Identifier' &&
raw_args.length < 2 &&
context.state.scope.get(callee.name)?.kind === 'prop';

const expression = /** @type {import('estree').Expression} */ (context.visit(callee));
const snippet_function = state.options.dev
Expand All @@ -1298,17 +1317,34 @@ const template_visitors = {
return /** @type {import('estree').Expression} */ (context.visit(arg));
});

state.template.push(
t_statement(
b.stmt(
(node.expression.type === 'CallExpression' ? b.call : b.maybe_call)(
snippet_function,
b.id('$$payload'),
...snippet_args
if (needs_backwards_compat) {
state.template.push(
t_statement(
b.stmt(
b.call(
'$.render_snippet_or_slot',
snippet_function,
b.id('$$props'),
b.literal(callee.name),
b.id('$$payload'),
...snippet_args
)
)
)
)
);
);
} else {
state.template.push(
t_statement(
b.stmt(
(node.expression.type === 'CallExpression' ? b.call : b.maybe_call)(
snippet_function,
b.id('$$payload'),
...snippet_args
)
)
)
);
}

state.template.push(block_close);
},
Expand Down Expand Up @@ -1736,15 +1772,15 @@ const template_visitors = {
const lets = [];

/** @type {import('estree').Expression} */
let expression = b.member_id('$$props.children');
let slot_name = b.literal('default');

for (const attribute of node.attributes) {
if (attribute.type === 'SpreadAttribute') {
spreads.push(/** @type {import('estree').Expression} */ (context.visit(attribute)));
} else if (attribute.type === 'Attribute') {
const value = serialize_attribute_value(attribute.value, context, false, true);
if (attribute.name === 'name') {
expression = b.member(b.member_id('$$props.$$slots'), value, true, true);
slot_name = value;
} else if (attribute.name !== 'slot') {
if (attribute.metadata.dynamic) {
props.push(b.get(attribute.name, [b.return(value)]));
Expand All @@ -1768,7 +1804,14 @@ const template_visitors = {
node.fragment.nodes.length === 0
? b.literal(null)
: b.thunk(b.block(create_block(node, node.fragment.nodes, context)));
const slot = b.call('$.slot', b.id('$$payload'), expression, props_expression, fallback);
const slot = b.call(
'$.slot',
b.id('$$payload'),
b.id('$$props'),
slot_name,
props_expression,
fallback
);

state.template.push(t_statement(b.stmt(slot)));
state.template.push(block_close);
Expand Down
31 changes: 31 additions & 0 deletions packages/svelte/src/internal/client/dom/blocks/snippet.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,34 @@ export function wrap_snippet(fn) {
}
);
}

/**
* Remove this once slots are gone
* @param {any} snippet_fn
* @param {Record<string, any>} $$props
* @param {string} name
* @param {Element} node
* @param {any} slot_props
*/
export function render_snippet_or_slot(snippet_fn, $$props, name, node, slot_props) {
if ($$props.$$slots) {
const slot = $$props.$$slots[name === 'children' ? 'default' : name];
if (typeof slot === 'function') {
let props = undefined;
if (slot_props) {
props = new Proxy(
{},
{
get(_, key) {
return slot_props()?.[key];
}
}
);
}
slot(node, props);
return;
}
}

snippet(snippet_fn, node, slot_props);
}
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
3 changes: 1 addition & 2 deletions packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export { key_block as key } from './dom/blocks/key.js';
export { css_props } from './dom/blocks/css-props.js';
export { index, each } from './dom/blocks/each.js';
export { html } from './dom/blocks/html.js';
export { snippet, wrap_snippet } from './dom/blocks/snippet.js';
export { snippet, wrap_snippet, render_snippet_or_slot } 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 @@ -154,7 +154,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
25 changes: 20 additions & 5 deletions packages/svelte/src/internal/client/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,29 @@ export function set_text(dom, value) {

/**
* @param {Comment} anchor
* @param {void | ((anchor: Comment, slot_props: Record<string, unknown>) => void)} slot_fn
* @param {Record<string, any>} $$props
* @param {string} slot_name
* @param {Record<string, unknown>} slot_props
* @param {null | ((anchor: Comment) => void)} fallback_fn
*/
export function slot(anchor, slot_fn, slot_props, fallback_fn) {
export function slot(anchor, $$props, slot_name, slot_props, fallback_fn) {
var is_snippet = false;
var slot_fn = $$props.$$slots?.[slot_name];
if (slot_fn === true) {
if (slot_name === 'default') {
slot_fn = $$props.children;
} else {
is_snippet = true;
slot_fn = $$props[slot_name];
}
}

if (slot_fn === undefined) {
if (fallback_fn !== null) {
fallback_fn(anchor);
}
} else {
slot_fn(anchor, slot_props);
slot_fn(anchor, is_snippet ? () => slot_props : slot_props);
}
}

Expand Down Expand Up @@ -312,8 +324,11 @@ export function unmount(component) {
* @returns {Record<string, any>}
*/
export function sanitize_slots(props) {
const sanitized = { ...props.$$slots };
if (props.children) sanitized.default = props.children;
/** @type {Record<string, boolean>} */
const sanitized = {};
for (const key in props.$$slots) {
sanitized[key] = true;
}
return sanitized;
}

Expand Down
Loading
Loading