Skip to content

Commit

Permalink
feat: provide isSnippet type, deduplicate children prop from default …
Browse files Browse the repository at this point in the history
…slot

fixes #10790
part of #9774
  • Loading branch information
dummdidumm committed Mar 13, 2024
1 parent ffb27f6 commit 378a17e
Show file tree
Hide file tree
Showing 16 changed files with 125 additions and 40 deletions.
5 changes: 5 additions & 0 deletions .changeset/cold-cheetahs-judge.md
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: deduplicate children prop and default slot
5 changes: 5 additions & 0 deletions .changeset/famous-grapes-refuse.md
@@ -0,0 +1,5 @@
---
"svelte": patch
---

feat: provide `isSnippet` function to determine whether a given value is a snippet
Expand Up @@ -774,6 +774,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 @@ -823,6 +830,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 @@ -944,13 +955,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 @@ -2685,9 +2691,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 @@ -3106,7 +3110,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 @@ -947,6 +947,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 @@ -975,6 +982,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 @@ -1049,14 +1060,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 @@ -1614,9 +1619,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 @@ -1744,7 +1747,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 @@ -38,3 +38,22 @@ export function snippet(get_snippet, node, ...args) {
};
}, block);
}

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 isSnippet(maybeSnippet) {
return /** @type {any} */ (maybeSnippet)?.[snippet_symbol] === true;
}
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 { isSnippet } 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 (isSnippet(children)) {
return children;
}
}
15 changes: 3 additions & 12 deletions packages/svelte/src/internal/client/validate.js
@@ -1,3 +1,4 @@
import { isSnippet } from './dom/blocks/snippet.js';
import { untrack } from './runtime.js';
import { is_array } from './utils.js';

Expand Down Expand Up @@ -103,22 +104,12 @@ export function loop_guard(timeout) {
};
}

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

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

/**
* 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 && !isSnippet(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 @@ -132,7 +123,7 @@ export function validate_snippet(snippet_fn) {
* @param {any} component_fn
*/
export function validate_component(component_fn) {
if (component_fn?.[snippet_symbol] === true) {
if (isSnippet(component_fn)) {
throw new Error('A snippet must be rendered with `{@render ...}`');
}
return component_fn;
Expand Down
2 changes: 2 additions & 0 deletions packages/svelte/src/internal/server/index.js
Expand Up @@ -11,6 +11,8 @@ import { DEV } from 'esm-env';
import { UNINITIALIZED } from '../client/constants.js';

export * from '../client/validate.js';
export { add_snippet_symbol } from '../client/dom/blocks/snippet.js';
export { default_slot } from '../client/dom/legacy/misc.js';

/**
* @typedef {{
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/legacy/legacy-client.js
Expand Up @@ -12,7 +12,7 @@ import * as $ from '../internal/index.js';
* @template {Record<string, any>} Slots
*
* @param {import('../main/public.js').ComponentConstructorOptions<Props> & {
* component: import('../main/public.js').SvelteComponent<Props, Events, Slots>;
* component: typeof import('../main/public.js').SvelteComponent<Props, Events, Slots>;
* immutable?: boolean;
* hydrate?: boolean;
* recover?: boolean;
Expand Down
3 changes: 2 additions & 1 deletion packages/svelte/src/main/main-client.js
Expand Up @@ -181,5 +181,6 @@ export {
hasContext,
getContext,
getAllContexts,
setContext
setContext,
isSnippet
} from '../internal/index.js';
3 changes: 2 additions & 1 deletion packages/svelte/src/main/main-server.js
Expand Up @@ -12,7 +12,8 @@ export {
tick,
unmount,
untrack,
createRoot
createRoot,
isSnippet
} from './main-client.js';

/** @returns {void} */
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`
});
@@ -0,0 +1,7 @@
<script>
import A from "./A.svelte";
</script>

<A children="foo">
bar
</A>
7 changes: 6 additions & 1 deletion packages/svelte/tests/types/snippet.ts
@@ -1,4 +1,4 @@
import type { Snippet } from 'svelte';
import { type Snippet, isSnippet } from 'svelte';

const return_type: ReturnType<Snippet> = null as any;

Expand Down Expand Up @@ -38,3 +38,8 @@ const h: Snippet<[{ a: true }]> = (a) => {
const i: Snippet = () => {
return return_type;
};

let j = null as any;
if (isSnippet(j)) {
let x: Snippet = j;
}
18 changes: 17 additions & 1 deletion packages/svelte/types/index.d.ts
Expand Up @@ -291,6 +291,12 @@ declare module 'svelte' {
* Anything except a function
*/
type NotFunction<T> = T extends Function ? never : T;
/**
* Returns true if given parameter is a snippet.
* */
export function isSnippet(maybeSnippet: any): maybeSnippet is (this: void) => unique symbol & {
_: "functions passed to {@render ...} tags must use the `Snippet` type imported from \"svelte\"";
};
/**
* @deprecated Use `mount` or `hydrate` instead
*/
Expand Down Expand Up @@ -1729,7 +1735,17 @@ declare module 'svelte/legacy' {
*
* */
export function createClassComponent<Props extends Record<string, any>, Exports extends Record<string, any>, Events extends Record<string, any>, Slots extends Record<string, any>>(options: ComponentConstructorOptions<Props> & {
component: SvelteComponent<Props, Events, Slots>;
component: {
new (options: ComponentConstructorOptions<Props & (Props extends {
children?: any;
} ? {} : Slots extends {
default: any;
} ? {
children?: ((this: void) => unique symbol & {
_: "functions passed to {@render ...} tags must use the `Snippet` type imported from \"svelte\"";
}) | undefined;
} : {})>): SvelteComponent<Props, Events, Slots>;
};
immutable?: boolean | undefined;
hydrate?: boolean | undefined;
recover?: boolean | undefined;
Expand Down

0 comments on commit 378a17e

Please sign in to comment.